Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

feat: add best effort attempt to revoke oauth access token in external auth provider - #19775

#19775
Merged
pawbana merged 4 commits into
maincoder/coder:mainfrom
oauth-revocation-15575coder/coder:oauth-revocation-15575Copy head branch name to clipboard
Sep 19, 2025
Merged

feat: add best effort attempt to revoke oauth access token in external auth provider#19775
pawbana merged 4 commits into
maincoder/coder:mainfrom
oauth-revocation-15575coder/coder:oauth-revocation-15575Copy head branch name to clipboard

Conversation

@pawbana

@pawbana pawbana commented Sep 11, 2025

Copy link
Copy Markdown
Contributor

Solves #15575
Adds OAuth access token revocation when unlinking external auth provider. Due to revocation not being consistently implemented by providers this is only best effort attempt. Unsuccessful revocation won't influence link removal.

@pawbana pawbana changed the title feat: add best effort attempt to revoke oauth access token in provider #15575 feat: add best effort attempt to revoke oauth access token in provider Sep 11, 2025
Comment thread site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPage.tsx Outdated
@pawbana
pawbana force-pushed the oauth-revocation-15575 branch 3 times, most recently from e0c6a5e to 8b82266 Compare September 16, 2025 07:49
@pawbana pawbana changed the title feat: add best effort attempt to revoke oauth access token in provider feat: add best effort attempt to revoke oauth access token in external auth provider Sep 16, 2025
@pawbana
pawbana force-pushed the oauth-revocation-15575 branch from 8b82266 to 3bdda1f Compare September 16, 2025 09:10

@deansheather deansheather left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Backend review

Comment thread .vscode/settings.json Outdated
Comment thread coderd/coderdtest/oidctest/idp.go Outdated
Comment thread coderd/externalauth/externalauth_test.go Outdated
Comment thread coderd/externalauth/externalauth_test.go Outdated
Comment thread codersdk/externalauth.go Outdated
@pawbana
pawbana force-pushed the oauth-revocation-15575 branch 5 times, most recently from 78554eb to 93f15d9 Compare September 17, 2025 07:33
@pawbana
pawbana force-pushed the oauth-revocation-15575 branch from 93f15d9 to a14d39f Compare September 17, 2025 11:32
Comment on lines +418 to +419
require.Error(t, err)
require.Contains(t, err.Error(), "token validation failed")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can condense require.Error(); require.Contains(t, err.Error(), msg) into require.ErrorContains(t, err, msg)

Comment thread coderd/externalauth/externalauth_test.go
Comment thread coderd/externalauth/externalauth_test.go Outdated
Comment thread coderd/externalauth/externalauth_test.go
Comment thread codersdk/externalauth.go Outdated

@deansheather deansheather left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Backend LGTM

@pawbana
pawbana marked this pull request as ready for review September 17, 2025 14:54
Comment thread site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPage.tsx Outdated
Comment thread site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPage.tsx Outdated
Comment thread site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPage.tsx Outdated
Comment thread site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPage.tsx Outdated

@aslilac aslilac left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

frontend looks much better! thanks

@pawbana
pawbana merged commit 439b041 into main Sep 19, 2025
31 checks passed
@pawbana
pawbana deleted the oauth-revocation-15575 branch September 19, 2025 14:27
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 19, 2025
CODER_EXTERNAL_AUTH_0_TYPE=github
CODER_EXTERNAL_AUTH_0_CLIENT_ID=xxxxxx
CODER_EXTERNAL_AUTH_0_CLIENT_SECRET=xxxxxxx
CODER_EXTERNAL_AUTH_0_REVOKE_URL=https://api.github.com/applications/<CLIENT ID>/grant

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we instead inherit this automatically if type=github.
We use the type to set defaults for known provides.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.