Skip to content

fix(sso): login with custom startUrl not allowed #6368

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jan 15, 2025
Merged

fix(sso): login with custom startUrl not allowed #6368

merged 4 commits into from
Jan 15, 2025

Conversation

nkomonen-amazon
Copy link
Contributor

@nkomonen-amazon nkomonen-amazon commented Jan 14, 2025 •
edited by justinmk3
Loading

Problem:

A user reported that a non-standard start url is technically
valid. This is because it can redirect to the underlying valid
start url that matches the pattern: https://xxxxxxxx.awsapps.com/start

#6341

Solution:

Allow any URL, but warn users if they are using a non-standard one.
We will show a yellow warning message in this case.
The red error message is still shown when the input does not match a
URL in general.

Examples

Invalid URL

Screenshot 2025-01-14 at 4 33 58 PM

Possibly valid since it may redirect to a valid URL

Screenshot 2025-01-14 at 4 34 13 PM

Missing the trailing /start

Screenshot 2025-01-14 at 4 34 29 PM

URL that also matches expected pattern

Screenshot 2025-01-14 at 4 34 35 PM
  • Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
  • Your code changes must meet the guidelines in CONTRIBUTING.md.
  • License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@nkomonen-amazon
fix(sso): Sign in did not allow custom start url
272f753 
Problem:

A user reported that a non-standard start url is technically
valid. This is because it can redirect to the underlying valid
start url that matches the pattern: https://xxxxxxxx.awsapps.com/start

Solution:

Allow any URL, but warn users if they are using a non-standard one.
We will show a yellow warning message in this case.
The red error message is still shown when the input does not match a
URL in general.

Signed-off-by: nkomonen-amazon <[email protected]>
@nkomonen-amazon
changelog items
bbba601 
Signed-off-by: nkomonen-amazon <[email protected]>
@nkomonen-amazon nkomonen-amazon requested a review from a team as a code owner January 14, 2025 21:36
@nkomonen-amazon nkomonen-amazon changed the title Start url fix fix(sso): login with custom startUrl not allowed Jan 14, 2025
justinmk3
justinmk3 reviewed Jan 14, 2025
View reviewed changes
packages/core/src/shared/utilities/uriUtils.ts
@@ -58,3 +58,15 @@ export function toUri(path: string | vscode.Uri): vscode.Uri {
}
return vscode.Uri.file(path)
}

export function isValidUrl(string: string): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is vscode.Uri.parse useful here? docstring may want to mention why not

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would add additional LOC since we'd have to check every part manually. Also there may be nuances with a valid URL that we could miss

@nkomonen-amazon
fix comments
112c333 
Signed-off-by: nkomonen-amazon <[email protected]>
@nkomonen-amazon
refactor message
77be23b 
Signed-off-by: nkomonen-amazon <[email protected]>
@nkomonen-amazon nkomonen-amazon merged commit d862a21 into aws:master Jan 15, 2025
25 of 26 checks passed
@nkomonen-amazon nkomonen-amazon deleted the startUrlFix branch January 15, 2025 18:57
karanA-aws pushed a commit to karanA-aws/aws-toolkit-vscode that referenced this pull request Jan 17, 2025
@nkomonen-amazon @karanA-aws
fix(sso): login with custom startUrl not allowed (aws#6368)
b14d8b1 
## Problem:

A user reported that a non-standard start url is technically
valid. This is because it can redirect to the underlying valid
start url that matches the pattern: https://xxxxxxxx.awsapps.com/start

## Solution:

Allow any URL, but warn users if they are using a non-standard one.
We will show a yellow warning message in this case.
The red error message is still shown when the input does not match a
URL in general.

## Examples 

### Invalid URL
<img width="315" alt="Screenshot 2025-01-14 at 4 33 58 PM"
src="https://github.com/user-attachments/assets/a5b2cb8a-c4fc-4678-a711-2f3f00bbe084"
/>

### Possibly valid since it may redirect to a valid URL
<img width="302" alt="Screenshot 2025-01-14 at 4 34 13 PM"
src="https://github.com/user-attachments/assets/0690f818-f4ba-4eae-b037-f856f5a2b2a0"
/>

### Missing the trailing `/start`
<img width="295" alt="Screenshot 2025-01-14 at 4 34 29 PM"
src="https://github.com/user-attachments/assets/8bcf3a4b-eba3-4bd8-8c68-24b709ee854d"
/>

### URL that also matches expected pattern
<img width="286" alt="Screenshot 2025-01-14 at 4 34 35 PM"
src="https://github.com/user-attachments/assets/eea2f2cb-6500-469c-9836-96ffc9cb5794"
/>

---

- Treat all work as PUBLIC. Private `feature/x` branches will not be
squash-merged at release time.
- Your code changes must meet the guidelines in
[CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines).
- License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.

---------

Signed-off-by: nkomonen-amazon <[email protected]>
kevluu-aws pushed a commit to kevluu-aws/aws-toolkit-vscode that referenced this pull request Jan 23, 2025
@nkomonen-amazon @kevluu-aws
fix(sso): login with custom startUrl not allowed (aws#6368)
5f3709a 
## Problem:

A user reported that a non-standard start url is technically
valid. This is because it can redirect to the underlying valid
start url that matches the pattern: https://xxxxxxxx.awsapps.com/start

## Solution:

Allow any URL, but warn users if they are using a non-standard one.
We will show a yellow warning message in this case.
The red error message is still shown when the input does not match a
URL in general.

## Examples 

### Invalid URL
<img width="315" alt="Screenshot 2025-01-14 at 4 33 58 PM"
src="https://github.com/user-attachments/assets/a5b2cb8a-c4fc-4678-a711-2f3f00bbe084"
/>

### Possibly valid since it may redirect to a valid URL
<img width="302" alt="Screenshot 2025-01-14 at 4 34 13 PM"
src="https://github.com/user-attachments/assets/0690f818-f4ba-4eae-b037-f856f5a2b2a0"
/>

### Missing the trailing `/start`
<img width="295" alt="Screenshot 2025-01-14 at 4 34 29 PM"
src="https://github.com/user-attachments/assets/8bcf3a4b-eba3-4bd8-8c68-24b709ee854d"
/>

### URL that also matches expected pattern
<img width="286" alt="Screenshot 2025-01-14 at 4 34 35 PM"
src="https://github.com/user-attachments/assets/eea2f2cb-6500-469c-9836-96ffc9cb5794"
/>

---

- Treat all work as PUBLIC. Private `feature/x` branches will not be
squash-merged at release time.
- Your code changes must meet the guidelines in
[CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines).
- License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.

---------

Signed-off-by: nkomonen-amazon <[email protected]>
s7ab059789 pushed a commit to s7ab059789/aws-toolkit-vscode that referenced this pull request Feb 19, 2025
@nkomonen-amazon @s7ab059789
fix(sso): login with custom startUrl not allowed (aws#6368)
a2ad424 
## Problem:

A user reported that a non-standard start url is technically
valid. This is because it can redirect to the underlying valid
start url that matches the pattern: https://xxxxxxxx.awsapps.com/start

## Solution:

Allow any URL, but warn users if they are using a non-standard one.
We will show a yellow warning message in this case.
The red error message is still shown when the input does not match a
URL in general.

## Examples 

### Invalid URL
<img width="315" alt="Screenshot 2025-01-14 at 4 33 58 PM"
src="https://github.com/user-attachments/assets/a5b2cb8a-c4fc-4678-a711-2f3f00bbe084"
/>

### Possibly valid since it may redirect to a valid URL
<img width="302" alt="Screenshot 2025-01-14 at 4 34 13 PM"
src="https://github.com/user-attachments/assets/0690f818-f4ba-4eae-b037-f856f5a2b2a0"
/>

### Missing the trailing `/start`
<img width="295" alt="Screenshot 2025-01-14 at 4 34 29 PM"
src="https://github.com/user-attachments/assets/8bcf3a4b-eba3-4bd8-8c68-24b709ee854d"
/>

### URL that also matches expected pattern
<img width="286" alt="Screenshot 2025-01-14 at 4 34 35 PM"
src="https://github.com/user-attachments/assets/eea2f2cb-6500-469c-9836-96ffc9cb5794"
/>

---

- Treat all work as PUBLIC. Private `feature/x` branches will not be
squash-merged at release time.
- Your code changes must meet the guidelines in
[CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines).
- License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.

---------

Signed-off-by: nkomonen-amazon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinmk3 justinmk3 justinmk3 approved these changes

@jpinkney-aws jpinkney-aws jpinkney-aws approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nkomonen-amazon @justinmk3 @jpinkney-aws