Skip to content

Commit b14d8b1

Browse files
nkomonen-amazonkaranA-aws
authored andcommitted
fix(sso): login with custom startUrl not allowed (aws#6368)
## 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]>
1 parent 7b6cf26 commit b14d8b1

File tree

7 files changed

+93
-13
lines changed

7 files changed

+93
-13
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Bug Fix",
3+
"description": "Auth: Valid StartURL not accepted at login"
4+
}

packages/core/src/auth/sso/constants.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,15 @@
1111
export const builderIdStartUrl = 'https://view.awsapps.com/start'
1212
export const internalStartUrl = 'https://amzn.awsapps.com/start'
1313

14+
/**
15+
* Doc: https://docs.aws.amazon.com/singlesignon/latest/userguide/howtochangeURL.html
16+
*/
1417
export const ssoUrlFormatRegex =
1518
/^(https?:\/\/(.+)\.awsapps\.com\/start|https?:\/\/identitycenter\.amazonaws\.com\/ssoins-[\da-zA-Z]{16})\/?$/
1619

17-
export const ssoUrlFormatMessage =
18-
'URLs must start with http:// or https://. Example: https://d-xxxxxxxxxx.awsapps.com/start'
20+
/**
21+
* It is possible for a start url to be a completely custom url that redirects to something that matches the format
22+
* below, so this message is only a warning.
23+
*/
24+
export const ssoUrlFormatMessage = 'URL possibly invalid. Expected format: https://xxxxxxxxxx.awsapps.com/start'
25+
export const urlInvalidFormatMessage = 'URL format invalid. Expected format: https://xxxxxxxxxx.com/yyyy'

packages/core/src/login/webview/vue/backend.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import { AuthEnabledFeatures, AuthError, AuthFlowState, AuthUiClick, userCancell
3131
import { DevSettings } from '../../../shared/settings'
3232
import { AuthSSOServer } from '../../../auth/sso/server'
3333
import { getLogger } from '../../../shared/logger/logger'
34+
import { isValidUrl } from '../../../shared/utilities/uriUtils'
3435

3536
export abstract class CommonAuthWebview extends VueWebview {
3637
private readonly className = 'CommonAuthWebview'
@@ -276,4 +277,8 @@ export abstract class CommonAuthWebview extends VueWebview {
276277
cancelAuthFlow() {
277278
AuthSSOServer.lastInstance?.cancelCurrentFlow()
278279
}
280+
281+
validateUrl(url: string) {
282+
return isValidUrl(url)
283+
}
279284
}

packages/core/src/login/webview/vue/login.vue

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@
193193
@keydown.enter="handleContinueClick()"
194194
/>
195195
<h4 class="start-url-error">{{ startUrlError }}</h4>
196+
<h4 class="start-url-warning">{{ startUrlWarning }}</h4>
196197
<div class="title topMargin">Region</div>
197198
<div class="hint">AWS Region that hosts identity directory</div>
198199
<select
@@ -278,7 +279,7 @@ import { LoginOption } from './types'
278279
import { CommonAuthWebview } from './backend'
279280
import { WebviewClientFactory } from '../../../webviews/client'
280281
import { Region } from '../../../shared/regions/endpoints'
281-
import { ssoUrlFormatRegex, ssoUrlFormatMessage } from '../../../auth/sso/constants'
282+
import { ssoUrlFormatRegex, ssoUrlFormatMessage, urlInvalidFormatMessage } from '../../../auth/sso/constants'
282283
283284
const client = WebviewClientFactory.create<CommonAuthWebview>()
284285
@@ -340,6 +341,7 @@ export default defineComponent({
340341
stage: 'START' as Stage,
341342
regions: [] as Region[],
342343
startUrlError: '',
344+
startUrlWarning: '',
343345
selectedRegion: 'us-east-1',
344346
startUrl: '',
345347
app: this.app,
@@ -365,7 +367,7 @@ export default defineComponent({
365367
366368
// Pre-select the first available login option
367369
await this.preselectLoginOption()
368-
this.handleUrlInput() // validate the default startUrl
370+
await this.handleUrlInput() // validate the default startUrl
369371
},
370372
methods: {
371373
toggleItemSelection(itemId: number) {
@@ -475,18 +477,48 @@ export default defineComponent({
475477
this.stage = 'CONNECTED'
476478
}
477479
},
478-
handleUrlInput() {
479-
if (this.startUrl && !ssoUrlFormatRegex.test(this.startUrl)) {
480-
this.startUrlError = ssoUrlFormatMessage
481-
} else if (this.startUrl && this.existingStartUrls.some((url) => url === this.startUrl)) {
482-
this.startUrlError =
483-
'A connection for this start URL already exists. Sign out before creating a new one.'
484-
} else {
485-
this.startUrlError = ''
480+
async handleUrlInput() {
481+
const messages = await resolveStartUrlMessages(this.startUrl, this.existingStartUrls)
482+
this.startUrlError = messages.error
483+
this.startUrlWarning = messages.warning
484+
485+
if (!messages.error && !messages.warning) {
486486
void client.storeMetricMetadata({
487487
credentialStartUrl: this.startUrl,
488488
})
489489
}
490+
491+
async function resolveStartUrlMessages(
492+
startUrl: string | undefined,
493+
existingStartUrls: string[]
494+
): Promise<{ warning: string; error: string }> {
495+
// No URL
496+
if (!startUrl) {
497+
return { error: '', warning: '' }
498+
}
499+
500+
// Validate URL format
501+
if (!ssoUrlFormatRegex.test(startUrl)) {
502+
console.log('Before Validate')
503+
if (await client.validateUrl(startUrl)) {
504+
console.log('After Validate')
505+
return { error: '', warning: ssoUrlFormatMessage }
506+
} else {
507+
return { error: urlInvalidFormatMessage, warning: '' }
508+
}
509+
}
510+
511+
// Ensure that URL does not exist yet
512+
if (existingStartUrls.some((url) => url === startUrl)) {
513+
return {
514+
error: 'A connection for this start URL already exists. Sign out before creating a new one.',
515+
warning: '',
516+
}
517+
}
518+
519+
// URL is valid
520+
return { error: '', warning: '' }
521+
}
490522
},
491523
handleRegionInput(event: any) {
492524
void client.storeMetricMetadata({
@@ -743,6 +775,10 @@ body.vscode-high-contrast:not(body.vscode-high-contrast-light) .regionSelect {
743775
color: #ff0000;
744776
font-size: var(--font-size-sm);
745777
}
778+
.start-url-warning {
779+
color: #dfe216;
780+
font-size: var(--font-size-sm);
781+
}
746782
#logo {
747783
fill: var(--vscode-button-foreground);
748784
}

packages/core/src/shared/utilities/uriUtils.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,15 @@ export function toUri(path: string | vscode.Uri): vscode.Uri {
5858
}
5959
return vscode.Uri.file(path)
6060
}
61+
62+
export function isValidUrl(string: string): boolean {
63+
try {
64+
const url = new URL(string)
65+
// handle case where, eg: 'https://test', would be considered valid. At a minimum
66+
// we'll require a top-level domain, eg: 'https://test.com'.
67+
const hostParts = url.hostname.split('.').filter((part) => !!part)
68+
return hostParts.length > 1
69+
} catch (err) {
70+
return false
71+
}
72+
}

packages/core/src/test/shared/utilities/uriUtils.test.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*/
55

66
import assert from 'assert'
7-
import { fromQueryToParameters } from '../../../shared/utilities/uriUtils'
7+
import { fromQueryToParameters, isValidUrl } from '../../../shared/utilities/uriUtils'
88

99
describe('uriUtils', function () {
1010
it('returns empty when no query parameters are present', function () {
@@ -33,4 +33,16 @@ describe('uriUtils', function () {
3333
])
3434
)
3535
})
36+
37+
it('isValidUrl()', function () {
38+
assert.strictEqual(isValidUrl(''), false)
39+
assert.strictEqual(isValidUrl('test.com'), false)
40+
assert.strictEqual(isValidUrl('https://test'), false)
41+
assert.strictEqual(isValidUrl('http://test.'), false)
42+
assert.strictEqual(isValidUrl('https://test.com http://test2.com'), false)
43+
44+
assert.strictEqual(isValidUrl('https://test.com'), true)
45+
assert.strictEqual(isValidUrl('http://test.com'), true)
46+
assert.strictEqual(isValidUrl('http://test.com/path'), true)
47+
})
3648
})
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Bug Fix",
3+
"description": "Auth: Valid StartURL not accepted at login"
4+
}

0 commit comments

Comments
 (0)