Skip to content

Commit daf7a23

Browse files
authored
fix(auth): add extra notification to SSO login flow (#3148)
## Problem Logging in via the browser is currently too easy. Section 5.4 of RFC 8628 suggests that implementations should make it clear to users which device is requesting access at all times. ## Solution Add an additional notification containing the user verification code before opening the browser. Users must now enter in this code in the browser whereas before it was done automatically. The user code is automatically copied to the clipboard to make things slightly easier. ## Relevant changes * Add notification to SSO portal login flow * Globally fake `vscode.window` for unit tests
1 parent d34c35c commit daf7a23

30 files changed

+307
-142
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": "Authenticating through the browser now requires users to manually enter a user verification code. Previously this was done automatically."
4+
}

docs/TESTPLAN.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ The test suite has two categories of tests:
2020
- Unit Tests: **fast** tests
2121
- Live in `src/test/`
2222
- The `vscode` API is available.
23+
- Use `getTestWindow()` to inspect or manipulate `vscode.window`
2324
- The Toolkit code is invoked as a library, not as an extension activated in VSCode's typical lifecycle.
2425
- Call functions and create objects directly.
2526
- May mock state where needed, though this is discouraged in favor of "fake" data/objects/files.
@@ -65,7 +66,7 @@ modifications/workarounds in `src/test/testRunner.ts`.
6566
- Many failure modes (as opposed to the "happy path") are not tested.
6667
- No performance/benchmark regression tests.
6768
- No UI tests (to exercise webviews).
68-
- https://github.com/redhat-developer/vscode-extension-tester
69+
- https://github.com/redhat-developer/vscode-extension-tester
6970
- Missing acceptance tests:
7071
- Connect to AWS
7172
- Fixed credentials and fixed credentials with assume roles

package-lock.json

Lines changed: 7 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3517,7 +3517,7 @@
35173517
"report": "nyc report --reporter=html --reporter=json"
35183518
},
35193519
"devDependencies": {
3520-
"@aws-toolkits/telemetry": "^1.0.87",
3520+
"@aws-toolkits/telemetry": "^1.0.100",
35213521
"@cspotcode/source-map-support": "^0.8.1",
35223522
"@sinonjs/fake-timers": "^8.1.0",
35233523
"@types/adm-zip": "^0.4.34",

src/credentials/auth.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { CancellationError } from '../shared/utilities/timeoutUtils'
2020
import { ToolkitError, UnknownError } from '../shared/errors'
2121
import { getCache } from './sso/cache'
2222
import { createFactoryFunction, Mutable } from '../shared/utilities/tsUtils'
23-
import { SsoToken } from './sso/model'
23+
import { builderIdStartUrl, SsoToken } from './sso/model'
2424
import { SsoClient } from './sso/clients'
2525
import { getLogger } from '../shared/logger'
2626
import { CredentialsProviderManager } from './providers/credentialsProviderManager'
@@ -38,7 +38,6 @@ import { getCodeCatalystDevEnvId } from '../shared/vscode/env'
3838
import { getConfigFilename } from './sharedCredentials'
3939
import { authHelpUrl } from '../shared/constants'
4040

41-
export const builderIdStartUrl = 'https://view.awsapps.com/start'
4241
export const ssoScope = 'sso:account:access'
4342
export const codecatalystScopes = ['codecatalyst:read_write']
4443
export const ssoAccountAccessScopes = ['sso:account:access']
@@ -581,7 +580,9 @@ export class Auth implements AuthService, ConnectionManager {
581580
const provider = this.getTokenProvider(id, profile)
582581
const truncatedUrl = profile.startUrl.match(/https?:\/\/(.*)\.awsapps\.com\/start/)?.[1] ?? profile.startUrl
583582
const label =
584-
profile.startUrl === builderIdStartUrl ? 'AWS Builder ID' : `IAM Identity Center (${truncatedUrl})`
583+
profile.startUrl === builderIdStartUrl
584+
? localizedText.builderId()
585+
: `${localizedText.iamIdentityCenter} (${truncatedUrl})`
585586

586587
return {
587588
id,
@@ -832,23 +833,25 @@ export const createBuilderIdItem = () =>
832833
({
833834
label: codicon`${getIcon('vscode-person')} ${localize(
834835
'aws.auth.builderIdItem.label',
835-
'Use a personal email to sign up and sign in with AWS Builder ID'
836+
'Use a personal email to sign up and sign in with {0}',
837+
localizedText.builderId()
836838
)}`,
837839
data: 'builderId',
838840
onClick: () => telemetry.ui_click.emit({ elementId: 'connection_optionBuilderID' }),
839-
detail: 'AWS Builder ID is a new, personal profile for builders.', // TODO: need a "Learn more" button ?
841+
detail: `${localizedText.builderId()} is a new, personal profile for builders.`, // TODO: need a "Learn more" button ?
840842
} as DataQuickPickItem<'builderId'>)
841843

842844
export const createSsoItem = () =>
843845
({
844846
label: codicon`${getIcon('vscode-organization')} ${localize(
845847
'aws.auth.ssoItem.label',
846-
'Connect using {0} IAM Identity Center',
847-
getIdeProperties().company
848+
'Connect using {0} {1}',
849+
getIdeProperties().company,
850+
localizedText.iamIdentityCenter
848851
)}`,
849852
data: 'sso',
850853
onClick: () => telemetry.ui_click.emit({ elementId: 'connection_optionSSO' }),
851-
detail: "Sign in to your company's IAM Identity Center access portal login page.",
854+
detail: `Sign in to your company's ${localizedText.iamIdentityCenter} access portal login page.`,
852855
} as DataQuickPickItem<'sso'>)
853856

854857
export const createIamItem = () =>
@@ -889,7 +892,7 @@ export async function createStartUrlPrompter(title: string, ignoreScopes = true)
889892

890893
return createInputBox({
891894
title: `${title}: Enter Start URL`,
892-
placeholder: "Enter start URL for your organization's IAM Identity Center",
895+
placeholder: `Enter start URL for your organization's IAM Identity Center`,
893896
buttons: [createHelpButton(), createExitButton()],
894897
validateInput: validateSsoUrl,
895898
})

src/credentials/sso/clients.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,10 @@ export class OidcClient {
5656

5757
public async startDeviceAuthorization(request: StartDeviceAuthorizationRequest) {
5858
const response = await this.client.startDeviceAuthorization(request)
59-
assertHasProps(response, 'expiresIn', 'deviceCode', 'verificationUriComplete')
59+
assertHasProps(response, 'expiresIn', 'deviceCode', 'userCode', 'verificationUri')
6060

6161
return {
62-
...selectFrom(response, 'deviceCode', 'verificationUriComplete'),
62+
...selectFrom(response, 'deviceCode', 'userCode', 'verificationUri'),
6363
expiresAt: new this.clock.Date(response.expiresIn * 1000 + this.clock.Date.now()),
6464
interval: response.interval ? response.interval * 1000 : undefined,
6565
}

src/credentials/sso/model.ts

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,15 @@
55

66
import globals from '../../shared/extensionGlobals'
77

8+
import * as nls from 'vscode-nls'
9+
const localize = nls.loadMessageBundle()
10+
811
import * as vscode from 'vscode'
12+
import * as localizedText from '../../shared/localizedText'
13+
import { getLogger } from '../../shared/logger/logger'
14+
import { telemetry } from '../../shared/telemetry/telemetry'
15+
import { CancellationError } from '../../shared/utilities/timeoutUtils'
16+
import { ssoAuthHelpUrl } from '../../shared/constants'
917

1018
export interface SsoToken {
1119
/**
@@ -66,8 +74,59 @@ export interface SsoProfile {
6674
readonly identifier?: string
6775
}
6876

69-
export async function openSsoPortalLink(authorization: { readonly verificationUriComplete: string }): Promise<boolean> {
70-
return vscode.env.openExternal(vscode.Uri.parse(authorization.verificationUriComplete))
77+
export const builderIdStartUrl = 'https://view.awsapps.com/start'
78+
79+
const tryOpenHelpUrl = (url: string) =>
80+
telemetry.aws_openUrl
81+
.run(async span => {
82+
span.record({ url })
83+
const didOpen = await vscode.env.openExternal(vscode.Uri.parse(url))
84+
if (!didOpen) {
85+
throw new CancellationError('user')
86+
}
87+
})
88+
.catch(e => getLogger().verbose('auth: failed to open help URL: %s', e))
89+
90+
export async function openSsoPortalLink(
91+
startUrl: string,
92+
authorization: { readonly verificationUri: string; readonly userCode: string }
93+
): Promise<boolean> {
94+
async function copyCodeAndOpenLink() {
95+
await vscode.env.clipboard.writeText(authorization.userCode).then(undefined, err => {
96+
getLogger().warn(`auth: failed to copy user code "${authorization.userCode}" to clipboard: %s`, err)
97+
})
98+
99+
return vscode.env.openExternal(vscode.Uri.parse(authorization.verificationUri))
100+
}
101+
102+
async function showLoginNotification() {
103+
const name = startUrl === builderIdStartUrl ? localizedText.builderId() : localizedText.iamIdentityCenterFull()
104+
const title = localize('AWS.auth.loginWithBrowser.messageTitle', 'Copy Code for {0}', name)
105+
const detail = localize(
106+
'AWS.auth.loginWithBrowser.messageDetail',
107+
'To proceed, open the login page and provide this code to confirm the access request: {0}',
108+
authorization.userCode
109+
)
110+
const copyCode = localize('AWS.auth.loginWithBrowser.copyCodeAction', 'Copy Code and Proceed')
111+
const options = { modal: true, detail } as vscode.MessageOptions
112+
113+
while (true) {
114+
// TODO: add the 'Help' item back once we have a suitable URL
115+
// const resp = await vscode.window.showInformationMessage(title, options, copyCode, localizedText.help)
116+
const resp = await vscode.window.showInformationMessage(title, options, copyCode)
117+
switch (resp) {
118+
case copyCode:
119+
return copyCodeAndOpenLink()
120+
case localizedText.help:
121+
await tryOpenHelpUrl(ssoAuthHelpUrl)
122+
continue
123+
default:
124+
throw new CancellationError('user')
125+
}
126+
}
127+
}
128+
129+
return telemetry.aws_loginWithBrowser.run(() => showLoginNotification())
71130
}
72131

73132
// Most SSO 'expirables' are fairly long lived, so a one minute buffer is plenty.

src/credentials/sso/ssoAccessTokenProvider.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ export class SsoAccessTokenProvider {
145145
clientSecret: registration.clientSecret,
146146
})
147147

148-
if (!(await openSsoPortalLink(authorization))) {
148+
if (!(await openSsoPortalLink(this.profile.startUrl, authorization))) {
149149
throw new CancellationError('user')
150150
}
151151

src/integrationTest/stepFunctions/visualizeStateMachine.test.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { MessageObject } from '../../stepFunctions/commands/visualizeStateMachin
1212
import { makeTemporaryToolkitFolder } from '../../shared/filesystemUtilities'
1313
import { closeAllEditors } from '../../test/testUtil'
1414
import { previewStateMachineCommand } from '../../stepFunctions/activation'
15-
import { createTestWindow } from '../../test/shared/vscode/window'
15+
import { getTestWindow } from '../../test/globalSetup.test'
1616

1717
const sampleStateMachine = `
1818
{
@@ -212,9 +212,7 @@ describe('visualizeStateMachine', async function () {
212212
await closeAllEditors()
213213
assert.strictEqual(vscode.window.activeTextEditor, undefined)
214214

215-
const testWindow = createTestWindow()
216-
sinon.replace(vscode, 'window', testWindow)
217-
const errorMessage = testWindow.waitForMessage(/no active text editor/i)
215+
const errorMessage = getTestWindow().waitForMessage(/no active text editor/i)
218216

219217
await Promise.all([previewStateMachineCommand.execute(), errorMessage.then(dialog => dialog.close())])
220218
})

src/shared/constants.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export const documentationUrl: string = isCloud9()
2828
* - alternative?: codecatalyst/latest/userguide/sign-up-create-resources.html
2929
*/
3030
export const authHelpUrl = 'https://docs.aws.amazon.com/general/latest/gr/differences-aws_builder_id.html'
31+
export const ssoAuthHelpUrl = 'https://docs.aws.amazon.com/singlesignon/latest/userguide/howtosignin.html'
3132
export const credentialHelpUrl: string =
3233
'https://docs.aws.amazon.com/toolkit-for-vscode/latest/userguide/setup-credentials.html'
3334
export const ssoCredentialsHelpUrl: string =

0 commit comments

Comments
 (0)