Skip to content

Commit 5a59571

Browse files
authored
feat(auth): request necessary scopes only (#4916)
* feat(auth): request AWS account scopes only when necessary Applies to both IdC and BuilderID - Don't request if we are signing into Amazon Q - Request if we are in toolkit re-using a connection from Amazon Q - via login page - via quick pick menu - Don't invalidate the connection in Amazon Q if we cancel adding scopes in toolkit. TODO: - Cancelling adding scopes in Q still invalidates the connection in toolkit. * remove invalidate option from addScopes, fix telemetry * restore original scopes if re-using a connection in Q fails * allow reauth without invalidation * fix tests * re-add and update test * docstrings
1 parent e90eae8 commit 5a59571

File tree

8 files changed

+126
-55
lines changed

8 files changed

+126
-55
lines changed

packages/core/src/auth/auth.ts

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,6 @@ interface AuthService {
9292
/**
9393
* Replaces the profile for a connection with a new one.
9494
*
95-
* This will invalidate the connection, potentially requiring a re-authentication.
96-
*
9795
* **IAM connections are not implemented**
9896
*/
9997
updateConnection(connection: Pick<Connection, 'id'>, profile: Profile): Promise<Connection>
@@ -166,9 +164,10 @@ export class Auth implements AuthService, ConnectionManager {
166164
}
167165
}
168166

169-
public async reauthenticate({ id }: Pick<SsoConnection, 'id'>): Promise<SsoConnection>
170-
public async reauthenticate({ id }: Pick<IamConnection, 'id'>): Promise<IamConnection>
171-
public async reauthenticate({ id }: Pick<Connection, 'id'>): Promise<Connection> {
167+
public async reauthenticate({ id }: Pick<SsoConnection, 'id'>, invalidate?: boolean): Promise<SsoConnection>
168+
public async reauthenticate({ id }: Pick<IamConnection, 'id'>, invalidate?: boolean): Promise<IamConnection>
169+
public async reauthenticate({ id }: Pick<Connection, 'id'>, invalidate?: boolean): Promise<Connection> {
170+
const shouldInvalidate = invalidate ?? true
172171
const profile = this.store.getProfileOrThrow(id)
173172
if (profile.type === 'sso') {
174173
const provider = this.getSsoTokenProvider(id, profile)
@@ -177,13 +176,13 @@ export class Auth implements AuthService, ConnectionManager {
177176
// so we need to set it here.
178177
await telemetry.aws_loginWithBrowser.run(async span => {
179178
span.record({ isReAuth: true, credentialStartUrl: profile.startUrl })
180-
await this.authenticate(id, () => provider.createToken())
179+
await this.authenticate(id, () => provider.createToken(), shouldInvalidate)
181180
})
182181

183182
return this.getSsoConnection(id, profile)
184183
} else {
185184
const provider = await this.getCredentialsProvider(id, profile)
186-
await this.authenticate(id, () => this.createCachedCredentials(provider))
185+
await this.authenticate(id, () => this.createCachedCredentials(provider), shouldInvalidate)
187186

188187
return this.getIamConnection(id, profile)
189188
}
@@ -383,14 +382,24 @@ export class Auth implements AuthService, ConnectionManager {
383382
await this.validateConnection(connection.id, profile)
384383
}
385384

386-
public async updateConnection(connection: Pick<SsoConnection, 'id'>, profile: SsoProfile): Promise<SsoConnection>
387-
public async updateConnection(connection: Pick<Connection, 'id'>, profile: Profile): Promise<Connection> {
385+
public async updateConnection(
386+
connection: Pick<SsoConnection, 'id'>,
387+
profile: SsoProfile,
388+
invalidate?: boolean
389+
): Promise<SsoConnection>
390+
public async updateConnection(
391+
connection: Pick<Connection, 'id'>,
392+
profile: Profile,
393+
invalidate?: boolean
394+
): Promise<Connection> {
388395
getLogger().info(`auth: Updating connection ${connection.id}`)
389396
if (profile.type === 'iam') {
390397
throw new Error('Updating IAM connections is not supported')
391398
}
392399

393-
await this.invalidateConnection(connection.id, { skipGlobalLogout: true })
400+
if (invalidate ?? false) {
401+
await this.invalidateConnection(connection.id, { skipGlobalLogout: true })
402+
}
394403

395404
const newProfile = await this.store.updateProfile(connection.id, profile)
396405
const updatedConn = this.getSsoConnection(connection.id, newProfile as StoredProfile<SsoProfile>)
@@ -554,15 +563,17 @@ export class Auth implements AuthService, ConnectionManager {
554563
}
555564
}
556565

557-
return runCheck().catch(err => this.handleSsoTokenError(id, err))
566+
return runCheck().catch(async err => {
567+
await this.handleSsoTokenError(id, err) // may throw without setting state to invalid - this is intended.
568+
await this.updateConnectionState(id, 'invalid')
569+
})
558570
}
559571

560572
private async handleSsoTokenError(id: Connection['id'], err: unknown) {
561573
this.throwOnNetworkError(err)
562574

563575
this.#validationErrors.set(id, UnknownError.cast(err))
564576
getLogger().info(`auth: Handling validation error of connection: ${id}`)
565-
return this.updateConnectionState(id, 'invalid')
566577
}
567578

568579
private async getConnectionFromStoreEntry([id, profile]: readonly [Connection['id'], StoredProfile<Profile>]) {
@@ -699,7 +710,8 @@ export class Auth implements AuthService, ConnectionManager {
699710
}
700711

701712
private readonly authenticate = keyedDebounce(this._authenticate.bind(this))
702-
private async _authenticate<T>(id: Connection['id'], callback: () => Promise<T>): Promise<T> {
713+
private async _authenticate<T>(id: Connection['id'], callback: () => Promise<T>, invalidate?: boolean): Promise<T> {
714+
const originalState = this.getConnectionState({ id }) ?? 'unauthenticated'
703715
await this.updateConnectionState(id, 'authenticating')
704716

705717
try {
@@ -709,6 +721,7 @@ export class Auth implements AuthService, ConnectionManager {
709721
return result
710722
} catch (err) {
711723
await this.handleSsoTokenError(id, err)
724+
await this.updateConnectionState(id, invalidate ? 'invalid' : originalState)
712725
throw err
713726
}
714727
}

packages/core/src/auth/secondaryAuth.ts

Lines changed: 41 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -218,30 +218,7 @@ export class SecondaryAuth<T extends Connection = Connection> {
218218
}
219219

220220
public async addScopes(conn: T & SsoConnection, extraScopes: string[]) {
221-
const oldScopes = conn.scopes ?? []
222-
const newScopes = Array.from(new Set([...oldScopes, ...extraScopes]))
223-
224-
const updateConnectionScopes = (scopes: string[]) => {
225-
return this.auth.updateConnection(conn, {
226-
type: 'sso',
227-
scopes,
228-
startUrl: conn.startUrl,
229-
ssoRegion: conn.ssoRegion,
230-
})
231-
}
232-
233-
const updatedConn = await updateConnectionScopes(newScopes)
234-
235-
try {
236-
return await this.auth.reauthenticate(updatedConn)
237-
} catch (e) {
238-
// We updated the connection scopes pre-emptively, but if there is some issue (e.g. user cancels,
239-
// InvalidGrantException, etc), then we need to revert to the old connection scopes. Otherwise,
240-
// this could soft-lock users into a broken connection that cannot be re-authenticated without
241-
// first deleting the connection.
242-
await updateConnectionScopes(oldScopes)
243-
throw e
244-
}
221+
return await addScopes(conn, extraScopes, this.auth)
245222
}
246223

247224
// Used to lazily restore persisted connections.
@@ -288,3 +265,43 @@ export class SecondaryAuth<T extends Connection = Connection> {
288265
}
289266
}
290267
}
268+
269+
/**
270+
* Used to add scopes to a connection, usually when re-using a connection across extensions.
271+
* It does not invalidate or otherwise change the state of the connection, but it does
272+
* trigger listeners for connection updates.
273+
*
274+
* How connections and scopes currently work for both this quickpick and the common Login page:
275+
* - Don't request AWS scopes if we are signing into Amazon Q
276+
* - Request AWS scopes for explorer sign in. Request AWS + CodeCatalyst scopes for CC sign in.
277+
* - Request scope difference if re-using a connection. Cancelling or otherwise failing to get the new scopes does NOT invalidate the connection.
278+
* - Adding scopes updates the connection profile, but does not change its state.
279+
*
280+
* Note: This should exist in connection.ts or utils.ts, but due to circular dependencies, it must go here.
281+
*/
282+
export async function addScopes(conn: SsoConnection, extraScopes: string[], auth = Auth.instance) {
283+
const oldScopes = conn.scopes ?? []
284+
const newScopes = Array.from(new Set([...oldScopes, ...extraScopes]))
285+
286+
const updateConnectionScopes = (scopes: string[]) => {
287+
return auth.updateConnection(conn, {
288+
type: 'sso',
289+
scopes,
290+
startUrl: conn.startUrl,
291+
ssoRegion: conn.ssoRegion,
292+
})
293+
}
294+
295+
const updatedConn = await updateConnectionScopes(newScopes)
296+
297+
try {
298+
return await auth.reauthenticate(updatedConn, false)
299+
} catch (e) {
300+
// We updated the connection scopes pre-emptively, but if there is some issue (e.g. user cancels,
301+
// InvalidGrantException, etc), then we need to revert to the old connection scopes. Otherwise,
302+
// this could soft-lock users into a broken connection that cannot be re-authenticated without
303+
// first deleting the connection.
304+
await updateConnectionScopes(oldScopes)
305+
throw e
306+
}
307+
}

packages/core/src/auth/utils.ts

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ import { TreeNode } from '../shared/treeview/resourceTreeDataProvider'
2020
import { createInputBox } from '../shared/ui/inputPrompter'
2121
import { telemetry } from '../shared/telemetry/telemetry'
2222
import { createCommonButtons, createExitButton, createHelpButton, createRefreshButton } from '../shared/ui/buttons'
23-
import { getIdeProperties, isCloud9 } from '../shared/extensionUtilities'
24-
import { getDependentAuths } from './secondaryAuth'
23+
import { getIdeProperties, isAmazonQ, isCloud9 } from '../shared/extensionUtilities'
24+
import { addScopes, getDependentAuths } from './secondaryAuth'
2525
import { DevSettings } from '../shared/settings'
2626
import { createRegionPrompter } from '../shared/ui/common/region'
2727
import { saveProfileToCredentials } from './credentials/sharedCredentials'
@@ -40,6 +40,7 @@ import {
4040
createSsoProfile,
4141
hasScopes,
4242
scopesSsoAccountAccess,
43+
isSsoConnection,
4344
} from './connection'
4445
import { Commands, placeholder, RegisteredCommand, vscodeComponent } from '../shared/vscode/commands2'
4546
import { Auth } from './auth'
@@ -91,14 +92,29 @@ export async function promptForConnection(auth: Auth, type?: 'iam' | 'iam-only'
9192
return resp
9293
}
9394

95+
/**
96+
* Creates the 'Switch Connection' quickpick for the Toolkit.
97+
* See {@link addScopes} for details about how scopes are requested for new and existing connections.
98+
*/
9499
export async function promptAndUseConnection(...[auth, type]: Parameters<typeof promptForConnection>) {
95100
return telemetry.aws_setCredentials.run(async span => {
96-
const resp = await promptForConnection(auth, type)
97-
if (!resp) {
101+
let conn = await promptForConnection(auth, type)
102+
if (!conn) {
98103
throw new CancellationError('user')
99104
}
100105

101-
await auth.useConnection(resp)
106+
// HACK: We assume that if we are toolkit we want AWS account scopes.
107+
// TODO: Although, Q shouldn't enter this codepath anyways.
108+
// We should deprecate any codepath in Q that may enter this.
109+
if (!isAmazonQ() && isSsoConnection(conn) && !hasScopes(conn, scopesSsoAccountAccess)) {
110+
try {
111+
conn = await addScopes(conn, scopesSsoAccountAccess)
112+
} catch (e: any) {
113+
throw new ToolkitError(`Failed to use connection${conn.label ? `: ${conn.label}` : '.'}`, e)
114+
}
115+
}
116+
117+
await auth.useConnection(conn)
102118
})
103119
}
104120

packages/core/src/codecatalyst/auth.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,8 @@ export class CodeCatalystAuthenticationProvider {
356356
await this.promptOnboarding()
357357
}
358358

359-
return this.secondaryAuth.useNewConnection(conn)
359+
await this.secondaryAuth.useNewConnection(conn)
360+
return conn
360361
}
361362

362363
/**

packages/core/src/codewhisperer/util/authUtil.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import {
2020
isIamConnection,
2121
isSsoConnection,
2222
isBuilderIdConnection,
23-
scopesSsoAccountAccess,
2423
scopesCodeWhispererChat,
2524
scopesFeatureDev,
2625
scopesGumby,
@@ -36,7 +35,7 @@ import { showReauthenticateMessage } from '../../shared/utilities/messages'
3635
import { showAmazonQWalkthroughOnce } from '../../amazonq/onboardingPage/walkthrough'
3736

3837
/** Backwards compatibility for connections w pre-chat scopes */
39-
export const codeWhispererCoreScopes = [...scopesSsoAccountAccess, ...scopesCodeWhispererCore]
38+
export const codeWhispererCoreScopes = [...scopesCodeWhispererCore]
4039
export const codeWhispererChatScopes = [...codeWhispererCoreScopes, ...scopesCodeWhispererChat]
4140
export const amazonQScopes = [...codeWhispererChatScopes, ...scopesGumby, ...scopesFeatureDev]
4241

packages/core/src/login/webview/vue/amazonq/backend_amazonq.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,16 +100,23 @@ export class AmazonQLoginWebview extends CommonAuthWebview {
100100
// if failed, connection is set to invalid
101101
const oldScopes = conn?.scopes ? conn.scopes : []
102102
const newScopes = Array.from(new Set([...oldScopes, ...amazonQScopes]))
103-
newConn = await Auth.instance.createConnectionFromApi({
103+
const payload: AwsConnection = {
104104
type: conn.type,
105105
ssoRegion: conn.ssoRegion,
106106
scopes: newScopes,
107107
startUrl: conn.startUrl,
108108
state: conn.state,
109109
id: conn.id,
110110
label: conn.label,
111-
})
112-
await Auth.instance.reauthenticate(newConn)
111+
}
112+
newConn = await Auth.instance.createConnectionFromApi(payload)
113+
try {
114+
await Auth.instance.reauthenticate(newConn, false)
115+
} catch (e) {
116+
// Restore original scopes as to not soft-lock connections.
117+
await Auth.instance.createConnectionFromApi({ ...payload, scopes: oldScopes })
118+
throw e
119+
}
113120
await AuthUtil.instance.secondaryAuth.useNewConnection(newConn)
114121
}
115122
if (!auto) {

packages/core/src/login/webview/vue/toolkit/backend_toolkit.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,18 @@ import * as vscode from 'vscode'
77
import { tryAddCredentials } from '../../../../auth/utils'
88
import { getLogger } from '../../../../shared/logger'
99
import { CommonAuthWebview } from '../backend'
10-
import { AwsConnection, Connection, createSsoProfile } from '../../../../auth/connection'
10+
import {
11+
AwsConnection,
12+
Connection,
13+
createSsoProfile,
14+
hasScopes,
15+
isIdcSsoConnection,
16+
scopesSsoAccountAccess,
17+
} from '../../../../auth/connection'
1118
import { Auth } from '../../../../auth/auth'
1219
import { CodeCatalystAuthenticationProvider } from '../../../../codecatalyst/auth'
1320
import { AuthError, AuthFlowState, TelemetryMetadata } from '../types'
21+
import { addScopes } from '../../../../auth/secondaryAuth'
1422

1523
export class ToolkitLoginWebview extends CommonAuthWebview {
1624
public override id: string = 'aws.toolkit.AmazonCommonAuth'
@@ -36,20 +44,26 @@ export class ToolkitLoginWebview extends CommonAuthWebview {
3644

3745
if (this.isCodeCatalystLogin) {
3846
return this.ssoSetup('startCodeCatalystSSOSetup', async () => {
39-
this.storeMetricMetadata({ ...metadata, authEnabledFeatures: 'codecatalyst' })
47+
this.storeMetricMetadata({ ...metadata })
48+
49+
const conn = await this.codeCatalystAuth.connectToEnterpriseSso(startUrl, region)
50+
51+
this.storeMetricMetadata({ authEnabledFeatures: this.getAuthEnabledFeatures(conn) })
4052

41-
await this.codeCatalystAuth.connectToEnterpriseSso(startUrl, region)
4253
await vscode.commands.executeCommand('setContext', 'aws.explorer.showAuthView', false)
4354
await this.showResourceExplorer()
4455
})
4556
}
4657

4758
return this.ssoSetup('createIdentityCenterConnection', async () => {
48-
this.storeMetricMetadata({ ...metadata, authEnabledFeatures: 'awsExplorer' })
59+
this.storeMetricMetadata({ ...metadata })
4960

5061
const ssoProfile = createSsoProfile(startUrl, region)
5162
const conn = await Auth.instance.createConnection(ssoProfile)
5263
await Auth.instance.useConnection(conn)
64+
65+
this.storeMetricMetadata({ authEnabledFeatures: this.getAuthEnabledFeatures(conn) })
66+
5367
await vscode.commands.executeCommand('setContext', 'aws.explorer.showAuthView', false)
5468
void vscode.window.showInformationMessage('Toolkit: Successfully connected to AWS IAM Identity Center')
5569
void this.showResourceExplorer()
@@ -126,7 +140,7 @@ export class ToolkitLoginWebview extends CommonAuthWebview {
126140
*/
127141
async useConnection(connectionId: string, auto: boolean): Promise<AuthError | undefined> {
128142
return this.ssoSetup('useConnection', async () => {
129-
const conn = await Auth.instance.getConnection({ id: connectionId })
143+
let conn = await Auth.instance.getConnection({ id: connectionId })
130144
if (conn === undefined || conn.type !== 'sso') {
131145
return
132146
}
@@ -136,6 +150,9 @@ export class ToolkitLoginWebview extends CommonAuthWebview {
136150
if (this.isCodeCatalystLogin) {
137151
await this.codeCatalystAuth.tryUseConnection(conn)
138152
} else {
153+
if (isIdcSsoConnection(conn) && !hasScopes(conn, scopesSsoAccountAccess)) {
154+
conn = await addScopes(conn, scopesSsoAccountAccess)
155+
}
139156
await Auth.instance.useConnection({ id: connectionId })
140157
}
141158

packages/core/src/test/credentials/auth.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,11 +154,12 @@ describe('Auth', function () {
154154
assert.deepStrictEqual(updated.scopes, updatedProfile.scopes)
155155
})
156156

157-
it('invalidates the connection', async function () {
157+
it('does not change the connection state', async function () {
158158
const conn = await auth.createConnection(ssoProfile)
159+
const originalState = auth.getConnectionState(conn)
159160
const updated = await auth.updateConnection(conn, updatedProfile)
160161

161-
assert.strictEqual(auth.getConnectionState(updated), 'invalid')
162+
assert.strictEqual(auth.getConnectionState(updated), originalState)
162163
})
163164

164165
it('fires an event when updating', async function () {

0 commit comments

Comments
 (0)