From 272f7532e92d9f65c084d5e91cd0a5de79b72ea3 Mon Sep 17 00:00:00 2001
From: nkomonen-amazon <nkomonen@amazon.com>
Date: Tue, 14 Jan 2025 16:27:27 -0500
Subject: [PATCH 1/4] fix(sso): Sign in did not allow custom start url

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 <nkomonen@amazon.com>
---
 packages/core/src/auth/sso/constants.ts       |  9 +++-
 .../core/src/login/webview/vue/backend.ts     |  5 ++
 packages/core/src/login/webview/vue/login.vue | 54 +++++++++++++++----
 .../core/src/shared/utilities/uriUtils.ts     | 12 +++++
 .../test/shared/utilities/uriUtils.test.ts    | 14 ++++-
 5 files changed, 83 insertions(+), 11 deletions(-)

diff --git a/packages/core/src/auth/sso/constants.ts b/packages/core/src/auth/sso/constants.ts
index 4b0e781ceaa..d810c77ae90 100644
--- a/packages/core/src/auth/sso/constants.ts
+++ b/packages/core/src/auth/sso/constants.ts
@@ -11,8 +11,15 @@
 export const builderIdStartUrl = 'https://view.awsapps.com/start'
 export const internalStartUrl = 'https://amzn.awsapps.com/start'
 
+/**
+ * Doc: https://docs.aws.amazon.com/singlesignon/latest/userguide/howtochangeURL.html
+ */
 export const ssoUrlFormatRegex =
     /^(https?:\/\/(.+)\.awsapps\.com\/start|https?:\/\/identitycenter\.amazonaws\.com\/ssoins-[\da-zA-Z]{16})\/?$/
 
+/**
+ * It is possible for a start url to be a completely custom url that redirects to something that matches the format
+ * below, so this message is only a warning.
+ */
 export const ssoUrlFormatMessage =
-    'URLs must start with http:// or https://. Example: https://d-xxxxxxxxxx.awsapps.com/start'
+    'URL possibly invalid. It typically follows the pattern: https://xxxxxxxxxx.awsapps.com/start'
diff --git a/packages/core/src/login/webview/vue/backend.ts b/packages/core/src/login/webview/vue/backend.ts
index 0c1cbdaebc7..ed467175334 100644
--- a/packages/core/src/login/webview/vue/backend.ts
+++ b/packages/core/src/login/webview/vue/backend.ts
@@ -31,6 +31,7 @@ import { AuthEnabledFeatures, AuthError, AuthFlowState, AuthUiClick, userCancell
 import { DevSettings } from '../../../shared/settings'
 import { AuthSSOServer } from '../../../auth/sso/server'
 import { getLogger } from '../../../shared/logger/logger'
+import { isValidUrl } from '../../../shared/utilities/uriUtils'
 
 export abstract class CommonAuthWebview extends VueWebview {
     private readonly className = 'CommonAuthWebview'
@@ -276,4 +277,8 @@ export abstract class CommonAuthWebview extends VueWebview {
     cancelAuthFlow() {
         AuthSSOServer.lastInstance?.cancelCurrentFlow()
     }
+
+    validateUrl(url: string) {
+        return isValidUrl(url)
+    }
 }
diff --git a/packages/core/src/login/webview/vue/login.vue b/packages/core/src/login/webview/vue/login.vue
index f15848a9069..55979922d71 100644
--- a/packages/core/src/login/webview/vue/login.vue
+++ b/packages/core/src/login/webview/vue/login.vue
@@ -193,6 +193,7 @@
                     @keydown.enter="handleContinueClick()"
                 />
                 <h4 class="start-url-error">{{ startUrlError }}</h4>
+                <h4 class="start-url-warning">{{ startUrlWarning }}</h4>
                 <div class="title topMargin">Region</div>
                 <div class="hint">AWS Region that hosts identity directory</div>
                 <select
@@ -340,6 +341,7 @@ export default defineComponent({
             stage: 'START' as Stage,
             regions: [] as Region[],
             startUrlError: '',
+            startUrlWarning: '',
             selectedRegion: 'us-east-1',
             startUrl: '',
             app: this.app,
@@ -365,7 +367,7 @@ export default defineComponent({
 
         // Pre-select the first available login option
         await this.preselectLoginOption()
-        this.handleUrlInput() // validate the default startUrl
+        await this.handleUrlInput() // validate the default startUrl
     },
     methods: {
         toggleItemSelection(itemId: number) {
@@ -475,18 +477,48 @@ export default defineComponent({
                 this.stage = 'CONNECTED'
             }
         },
-        handleUrlInput() {
-            if (this.startUrl && !ssoUrlFormatRegex.test(this.startUrl)) {
-                this.startUrlError = ssoUrlFormatMessage
-            } else if (this.startUrl && this.existingStartUrls.some((url) => url === this.startUrl)) {
-                this.startUrlError =
-                    'A connection for this start URL already exists. Sign out before creating a new one.'
-            } else {
-                this.startUrlError = ''
+        async handleUrlInput() {
+            const messages = await resolveStartUrlMessages(this.startUrl, this.existingStartUrls)
+            this.startUrlError = messages.error
+            this.startUrlWarning = messages.warning
+
+            if (!messages.error && !messages.warning) {
                 void client.storeMetricMetadata({
                     credentialStartUrl: this.startUrl,
                 })
             }
+
+            async function resolveStartUrlMessages(
+                startUrl: string | undefined,
+                existingStartUrls: string[]
+            ): Promise<{ warning: string; error: string }> {
+                // No URL
+                if (!startUrl) {
+                    return { error: '', warning: '' }
+                }
+
+                // Validate URL format
+                if (!ssoUrlFormatRegex.test(startUrl)) {
+                    console.log('Before Validate')
+                    if (await client.validateUrl(startUrl)) {
+                        console.log('After Validate')
+                        return { error: '', warning: ssoUrlFormatMessage }
+                    } else {
+                        return { error: 'URL format invalid. Must follow eg: https://xxxxx.com/yyyy', warning: '' }
+                    }
+                }
+
+                // Ensure that URL does not exist yet
+                if (existingStartUrls.some((url) => url === startUrl)) {
+                    return {
+                        error: 'A connection for this start URL already exists. Sign out before creating a new one.',
+                        warning: '',
+                    }
+                }
+
+                // URL is valid
+                return { error: '', warning: '' }
+            }
         },
         handleRegionInput(event: any) {
             void client.storeMetricMetadata({
@@ -743,6 +775,10 @@ body.vscode-high-contrast:not(body.vscode-high-contrast-light) .regionSelect {
     color: #ff0000;
     font-size: var(--font-size-sm);
 }
+.start-url-warning {
+    color: #dfe216;
+    font-size: var(--font-size-sm);
+}
 #logo {
     fill: var(--vscode-button-foreground);
 }
diff --git a/packages/core/src/shared/utilities/uriUtils.ts b/packages/core/src/shared/utilities/uriUtils.ts
index deca450570b..41d43f9a0ec 100644
--- a/packages/core/src/shared/utilities/uriUtils.ts
+++ b/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 {
+    try {
+        const url = new URL(string)
+        // handle case where, eg: 'https://test', would be considered valid. At a minimum
+        // we'll require a top-level domain, eg: 'https://test.com'.
+        const hostParts = url.hostname.split('.').filter((part) => !!part)
+        return hostParts.length > 1
+    } catch (err) {
+        return false
+    }
+}
diff --git a/packages/core/src/test/shared/utilities/uriUtils.test.ts b/packages/core/src/test/shared/utilities/uriUtils.test.ts
index 11daa57644a..d8c03ad0cf8 100644
--- a/packages/core/src/test/shared/utilities/uriUtils.test.ts
+++ b/packages/core/src/test/shared/utilities/uriUtils.test.ts
@@ -4,7 +4,7 @@
  */
 
 import assert from 'assert'
-import { fromQueryToParameters } from '../../../shared/utilities/uriUtils'
+import { fromQueryToParameters, isValidUrl } from '../../../shared/utilities/uriUtils'
 
 describe('uriUtils', function () {
     it('returns empty when no query parameters are present', function () {
@@ -33,4 +33,16 @@ describe('uriUtils', function () {
             ])
         )
     })
+
+    it('isValidUrl()', function () {
+        assert.strictEqual(isValidUrl(''), false)
+        assert.strictEqual(isValidUrl('test.com'), false)
+        assert.strictEqual(isValidUrl('https://test'), false)
+        assert.strictEqual(isValidUrl('http://test.'), false)
+        assert.strictEqual(isValidUrl('https://test.com http://test2.com'), false)
+
+        assert.strictEqual(isValidUrl('https://test.com'), true)
+        assert.strictEqual(isValidUrl('http://test.com'), true)
+        assert.strictEqual(isValidUrl('http://test.com/path'), true)
+    })
 })

From bbba6013d2484cb9250d4513d1c5090a907f174c Mon Sep 17 00:00:00 2001
From: nkomonen-amazon <nkomonen@amazon.com>
Date: Tue, 14 Jan 2025 16:30:47 -0500
Subject: [PATCH 2/4] changelog items

Signed-off-by: nkomonen-amazon <nkomonen@amazon.com>
---
 .../Bug Fix-71c6bbc1-67ae-4318-a7f0-c594e097ebc4.json         | 4 ++++
 .../Bug Fix-29e6ef4c-536b-47bb-ae27-26b802ccdb65.json         | 4 ++++
 2 files changed, 8 insertions(+)
 create mode 100644 packages/amazonq/.changes/next-release/Bug Fix-71c6bbc1-67ae-4318-a7f0-c594e097ebc4.json
 create mode 100644 packages/toolkit/.changes/next-release/Bug Fix-29e6ef4c-536b-47bb-ae27-26b802ccdb65.json

diff --git a/packages/amazonq/.changes/next-release/Bug Fix-71c6bbc1-67ae-4318-a7f0-c594e097ebc4.json b/packages/amazonq/.changes/next-release/Bug Fix-71c6bbc1-67ae-4318-a7f0-c594e097ebc4.json
new file mode 100644
index 00000000000..e0c15b7f2dc
--- /dev/null
+++ b/packages/amazonq/.changes/next-release/Bug Fix-71c6bbc1-67ae-4318-a7f0-c594e097ebc4.json	
@@ -0,0 +1,4 @@
+{
+	"type": "Bug Fix",
+	"description": "Auth: Valid StartURL not accepted at login"
+}
diff --git a/packages/toolkit/.changes/next-release/Bug Fix-29e6ef4c-536b-47bb-ae27-26b802ccdb65.json b/packages/toolkit/.changes/next-release/Bug Fix-29e6ef4c-536b-47bb-ae27-26b802ccdb65.json
new file mode 100644
index 00000000000..e0c15b7f2dc
--- /dev/null
+++ b/packages/toolkit/.changes/next-release/Bug Fix-29e6ef4c-536b-47bb-ae27-26b802ccdb65.json	
@@ -0,0 +1,4 @@
+{
+	"type": "Bug Fix",
+	"description": "Auth: Valid StartURL not accepted at login"
+}

From 112c333a3d407a0ac61e8770f161c040e8aed917 Mon Sep 17 00:00:00 2001
From: nkomonen-amazon <nkomonen@amazon.com>
Date: Wed, 15 Jan 2025 12:49:06 -0500
Subject: [PATCH 3/4] fix comments

Signed-off-by: nkomonen-amazon <nkomonen@amazon.com>
---
 packages/core/src/auth/sso/constants.ts | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/packages/core/src/auth/sso/constants.ts b/packages/core/src/auth/sso/constants.ts
index d810c77ae90..ca6fb132b04 100644
--- a/packages/core/src/auth/sso/constants.ts
+++ b/packages/core/src/auth/sso/constants.ts
@@ -21,5 +21,4 @@ export const ssoUrlFormatRegex =
  * It is possible for a start url to be a completely custom url that redirects to something that matches the format
  * below, so this message is only a warning.
  */
-export const ssoUrlFormatMessage =
-    'URL possibly invalid. It typically follows the pattern: https://xxxxxxxxxx.awsapps.com/start'
+export const ssoUrlFormatMessage = 'URL possibly invalid. Expected format: https://xxxxxxxxxx.awsapps.com/start'

From 77be23ba063a0179d9a476eeb0608e72ee02a221 Mon Sep 17 00:00:00 2001
From: nkomonen-amazon <nkomonen@amazon.com>
Date: Wed, 15 Jan 2025 13:06:47 -0500
Subject: [PATCH 4/4] refactor message

Signed-off-by: nkomonen-amazon <nkomonen@amazon.com>
---
 packages/core/src/auth/sso/constants.ts       | 1 +
 packages/core/src/login/webview/vue/login.vue | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/packages/core/src/auth/sso/constants.ts b/packages/core/src/auth/sso/constants.ts
index ca6fb132b04..0e6bb082d7e 100644
--- a/packages/core/src/auth/sso/constants.ts
+++ b/packages/core/src/auth/sso/constants.ts
@@ -22,3 +22,4 @@ export const ssoUrlFormatRegex =
  * below, so this message is only a warning.
  */
 export const ssoUrlFormatMessage = 'URL possibly invalid. Expected format: https://xxxxxxxxxx.awsapps.com/start'
+export const urlInvalidFormatMessage = 'URL format invalid. Expected format: https://xxxxxxxxxx.com/yyyy'
diff --git a/packages/core/src/login/webview/vue/login.vue b/packages/core/src/login/webview/vue/login.vue
index 55979922d71..4c9f65a2f6a 100644
--- a/packages/core/src/login/webview/vue/login.vue
+++ b/packages/core/src/login/webview/vue/login.vue
@@ -279,7 +279,7 @@ import { LoginOption } from './types'
 import { CommonAuthWebview } from './backend'
 import { WebviewClientFactory } from '../../../webviews/client'
 import { Region } from '../../../shared/regions/endpoints'
-import { ssoUrlFormatRegex, ssoUrlFormatMessage } from '../../../auth/sso/constants'
+import { ssoUrlFormatRegex, ssoUrlFormatMessage, urlInvalidFormatMessage } from '../../../auth/sso/constants'
 
 const client = WebviewClientFactory.create<CommonAuthWebview>()
 
@@ -504,7 +504,7 @@ export default defineComponent({
                         console.log('After Validate')
                         return { error: '', warning: ssoUrlFormatMessage }
                     } else {
-                        return { error: 'URL format invalid. Must follow eg: https://xxxxx.com/yyyy', warning: '' }
+                        return { error: urlInvalidFormatMessage, warning: '' }
                     }
                 }