Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wip] Revoke access token #5613

Closed
wants to merge 1 commit into from
Closed

[wip] Revoke access token #5613

wants to merge 1 commit into from

Conversation

arafatkatze
Copy link
Contributor

@arafatkatze arafatkatze commented Sep 18, 2024

Draft to look at Linear issue

Problem Statement

When users sign in and out of Cody (VS Code extension) multiple times with the same account (more than 20 times), the system does not revoke old tokens, leading to an accumulation of active tokens. Eventually, this triggers the error shown in the login flow as users exceed the limit of active access tokens.

Solution

  • Implemented logic to revoke the existing access tokens when users log out of Cody.
  • Added a cleanup process that automatically removes older access tokens upon sign-out, ensuring users do not hit the access token limit when signing in multiple times.

See video Proof

Test plan

Tested locally on both a local sourcegraph instance and with dotcom instance using Vscode cody.

Changelog

@arafatkatze arafatkatze changed the title Revoke access token [wip] Revoke access token Sep 18, 2024
Copy link
Contributor

@abeatrix abeatrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we will need to implement this for Cody Web since Cody Web doesn't need manual sign in with token (Vova please correct me if I'm wrong 😀 ). However I don't think the current implementation would work as intended because all the tokens that were imported manually before this change will now be treated as "AUTO" source and therefore be removed. This also will not treat the token input through the UI as "MANUAL".

It might make more sense to perform the deletion for all tokens regardless of their source in this case, Wdyt?

@@ -177,7 +178,7 @@ async function showAccessTokenInputBox(endpoint: string): Promise<string | undef
})

if (typeof result === 'string') {
return result.trim()
return 'MANUAL_' + result.trim()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this mean token that were input manually before this change would be treated as "automatically" imported and will be removed on sign out?

We will remove the quick pick for sign in and move into a unified webview for auth. When we do, this solution will probably not going to work?

@abeatrix
Copy link
Contributor

abeatrix commented Sep 18, 2024

not sure if possible, but maybe you can try to match the note of the token to see if it is generated by IDE redirections to identify the ones that are auto generated:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants