Skip to content

Commit

Permalink
revert(toolkit): Q <--> Toolkit auth separation notification
Browse files Browse the repository at this point in the history
Removes the prompt shown in Toolkit that Amazon Q no longer shares connections with it. Some time has passed, active users should have been informed by now. We are still getting telemetry hits indicating that this is being used though.

NOTE: Does not revert or remove any separation logic itself. We continue to have separate sessions and also remove any connections in either extension that are extra or don't match the required scopes for that extension. That logic helps us catch auth edge cases.
  • Loading branch information
hayemaxi committed Jan 3, 2025
1 parent 7ae5405 commit 4c747af
Show file tree
Hide file tree
Showing 6 changed files with 5 additions and 93 deletions.
73 changes: 0 additions & 73 deletions packages/core/src/auth/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,11 +339,6 @@ export class Auth implements AuthService, ConnectionManager {
metadata: { connectionState: 'unauthenticated' },
})

// Remove the split session logout prompt, if it exists.
if (!isAmazonQ()) {
await globals.globalState.update('aws.toolkit.separationPromptDismissed', true)
}

try {
;(await tokenProvider.getToken()) ?? (await tokenProvider.createToken())
const storedProfile = await this.store.addProfile(id, profile)
Expand Down Expand Up @@ -1136,71 +1131,3 @@ export function hasVendedIamCredentials(isC9?: boolean, isSM?: boolean) {
isSM ??= isSageMaker()
return isSM || isC9
}

type LoginCommand = 'aws.toolkit.auth.manageConnections' | 'aws.codecatalyst.manageConnections'
/**
* Temporary class that handles notifiting users who were logged out as part of
* splitting auth sessions between extensions.
*
* TODO: Remove after some time.
*/
export class SessionSeparationPrompt {
// Local variable handles per session displays, e.g. we forgot a CodeCatalyst connection AND
// an Explorer only connection. We only want to display once in this case.
// However, we don't want to set this at the global state level until a user interacts with the
// notification in case they miss it the first time.
#separationPromptDisplayed = false

/**
* Open a prompt for that last used command name (or do nothing if no command name has ever been passed),
* which is useful to redisplay the prompt after reloads in case a user misses it.
*/
public async showAnyPreviousPrompt() {
const cmd = globals.globalState.tryGet('aws.toolkit.separationPromptCommand', String)
return cmd ? await this.showForCommand(cmd as LoginCommand) : undefined
}

/**
* Displays a sign in prompt to the user if they have been logged out of the Toolkit as part of
* separating auth sessions between extensions. It will executed the passed command for sign in,
* (e.g. codecatalyst sign in vs explorer)
*/
public async showForCommand(cmd: LoginCommand) {
if (
this.#separationPromptDisplayed ||
globals.globalState.get<boolean>('aws.toolkit.separationPromptDismissed')
) {
return
}

await globals.globalState.update('aws.toolkit.separationPromptCommand', cmd)

await telemetry.toolkit_showNotification.run(async () => {
telemetry.record({ id: 'sessionSeparation' })
this.#separationPromptDisplayed = true
void vscode.window
.showWarningMessage(
'Amazon Q and AWS Toolkit no longer share connections. Please sign in again to use AWS Toolkit.',
'Sign In'
)
.then(async (resp) => {
await telemetry.toolkit_invokeAction.run(async () => {
telemetry.record({ source: 'sessionSeparationNotification' })
if (resp === 'Sign In') {
telemetry.record({ action: 'signIn' })
await vscode.commands.executeCommand(cmd)
} else {
telemetry.record({ action: 'dismiss' })
}

await globals.globalState.update('aws.toolkit.separationPromptDismissed', true)
})
})
})
}

static #instance: SessionSeparationPrompt
public static get instance() {
return (this.#instance ??= new SessionSeparationPrompt())
}
}
2 changes: 0 additions & 2 deletions packages/core/src/codecatalyst/activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import { DevEnvActivityStarter } from './devEnv'
import { learnMoreCommand, onboardCommand, reauth } from './explorer'
import { isInDevEnv } from '../shared/vscode/env'
import { hasScopes, scopesCodeWhispererCore, getTelemetryMetadataForConn } from '../auth/connection'
import { SessionSeparationPrompt } from '../auth/auth'
import { telemetry } from '../shared/telemetry/telemetry'
import { asStringifiedStack } from '../shared/telemetry/spans'

Expand Down Expand Up @@ -64,7 +63,6 @@ export async function activate(ctx: ExtContext): Promise<void> {
})

await authProvider.secondaryAuth.forgetConnection()
await SessionSeparationPrompt.instance.showForCommand('aws.codecatalyst.manageConnections')
})
},
{ emit: false, functionId: { name: 'activate', class: 'CodeCatalyst' } }
Expand Down
7 changes: 1 addition & 6 deletions packages/core/src/extensionNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import globals from './shared/extensionGlobals'
import { Experiments, Settings, showSettingsFailedMsg } from './shared/settings'
import { isReleaseVersion } from './shared/vscode/env'
import { AuthStatus, AuthUserState, telemetry } from './shared/telemetry/telemetry'
import { Auth, SessionSeparationPrompt } from './auth/auth'
import { Auth } from './auth/auth'
import { getTelemetryMetadataForConn } from './auth/connection'
import { registerSubmitFeedback } from './feedback/vue/submitFeedback'
import { activateCommon, deactivateCommon } from './extension'
Expand Down Expand Up @@ -139,14 +139,9 @@ export async function activate(context: vscode.ExtensionContext) {
conn.scopes
)
await Auth.instance.forgetConnection(conn)
await SessionSeparationPrompt.instance.showForCommand('aws.toolkit.auth.manageConnections')
}
}

// Display last prompt if connections were forgotten in prior sessions
// but the user did not interact or sign in again. Useful in case the user misses it the first time.
await SessionSeparationPrompt.instance.showAnyPreviousPrompt()

// MUST restore CW/Q auth so that we can see if this user is already a Q user.
await AuthUtil.instance.restore()
},
Expand Down
2 changes: 0 additions & 2 deletions packages/core/src/shared/globalState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ export type globalKey =
| 'aws.toolkit.amazonq.dismissed'
| 'aws.toolkit.amazonqInstall.dismissed'
| 'aws.amazonq.workspaceIndexToggleOn'
| 'aws.toolkit.separationPromptCommand'
| 'aws.toolkit.separationPromptDismissed'
// Deprecated/legacy names. New keys should start with "aws.".
| '#sessionCreationDates' // Legacy name from `ssoAccessTokenProvider.ts`.
| 'CODECATALYST_RECONNECT'
Expand Down
10 changes: 0 additions & 10 deletions packages/core/src/test/techdebt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,4 @@ describe('tech debt', function () {
// This is relevant for the use of `fs.cpSync` in the copyFiles scripts.
assert.ok(semver.lt(minNodejs, '18.0.0'), 'with node18+, we can remove the dependency on @types/node@18')
})

it('remove separate sessions login edge cases', async function () {
// src/auth/auth.ts:SessionSeparationPrompt
// forgetConnection() function and calls

// Monitor telemtry to determine removal or snooze
// toolkit_showNotification.id = sessionSeparation
// auth_modifyConnection.action = deleteProfile OR auth_modifyConnection.source contains CodeCatalyst
fixByDate('2025-01-06', 'Remove the edge case code from the commit that this test is a part of.')
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "Removal",
"description": "Auth: No longer inform users that Amazon Q and Toolkit extensions have separate auth sessions."
}

0 comments on commit 4c747af

Please sign in to comment.