Skip to content

fix: return 404 instead of 401 for missing OAuth2 apps #18755

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 7, 2025
Merged

fix: return 404 instead of 401 for missing OAuth2 apps #18755

merged 1 commit into from
Jul 7, 2025
+174 −47

Conversation

ThomasK33
Copy link
Member

Problem

Users were being automatically logged out when deleting OAuth2 applications.

Root Cause

  1. User deletes OAuth2 app successfully
  2. React Query automatically refetches the app data
  3. Management API incorrectly returned 401 Unauthorized for the missing app
  4. Frontend axios interceptor sees 401 and calls signOut()
  5. User gets logged out unexpectedly

Solution

  • Change management API to return 404 Not Found for missing OAuth2 apps
  • OAuth2 protocol endpoints continue returning 401 per RFC 6749
  • Rename writeInvalidClient to writeClientNotFound for clarity

Additional Changes

  • Add conditional OAuth2 navigation when experiment is enabled or in dev builds
  • Add isDevBuild() utility and buildInfo to dashboard context
  • Minor improvements to format script and warning dialogs

@ThomasK33 ThomasK33 requested a review from BrunoQuaresma July 4, 2025 13:33
@ThomasK33 ThomasK33 force-pushed the fix/oauth2-app-deletion-logout branch 2 times, most recently from 932c04f to eb425dd Compare July 4, 2025 13:52
@ThomasK33 ThomasK33 requested a review from johnstcn July 7, 2025 12:14
johnstcn
johnstcn reviewed Jul 7, 2025
View reviewed changes
johnstcn
johnstcn reviewed Jul 7, 2025
View reviewed changes
Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

Instead of deviating further from the OAuth2 spec, what do you think about instead modifying the interceptor to ignore this as a special case?

If the user's token has indeed expired, there will almost certainly be another 401 to trigger this logic and sign the user out.

BrunoQuaresma
BrunoQuaresma approved these changes Jul 7, 2025
View reviewed changes
Copy link
Collaborator

@BrunoQuaresma BrunoQuaresma left a comment

Choose a reason for hiding this comment

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

I left a few comments related to the FE code, but I don't think they are blockers. 👍

@ThomasK33 Graphite App
Copy link
Member Author

Instead of deviating further from the OAuth2 spec, what do you think about instead modifying the interceptor to ignore this as a special case?

If the user's token has indeed expired, there will almost certainly be another 401 to trigger this logic and sign the user out.

We're not deviating from the RFC here.

These changes do not address the revocation endpoint (which will be part of a future PR), but a custom endpoint to delete OAuth2 apps in coder. Thus we're not constrained and free to return the codes we want or need.

@johnstcn
Copy link
Member

johnstcn commented Jul 7, 2025

We're not deviating from the RFC here.

These changes do not address the revocation endpoint (which will be part of a future PR), but a custom endpoint to delete OAuth2 apps in coder. Thus we're not constrained and free to return the codes we want or need.

Sorry, misread the scope. That sounds fine then.

@ThomasK33
fix: return 404 instead of 401 for missing OAuth2 apps in management API
9997b55 
This prevents users from being logged out when deleting OAuth2 apps.
The frontend interceptor triggers logout on 401 responses, but React Query
refetches deleted apps and should get 404, not 401.

Also adds conditional OAuth2 navigation when experiment is enabled.

Change-Id: I48886144883539b7c51307f2a500f95be31dd383
Signed-off-by: Thomas Kosiewski <[email protected]>
@ThomasK33 ThomasK33 force-pushed the fix/oauth2-app-deletion-logout branch from eb425dd to 9997b55 Compare July 7, 2025 17:42
@ThomasK33 ThomasK33 merged commit 3dcd2ac into main Jul 7, 2025
37 checks passed
@ThomasK33 ThomasK33 deleted the fix/oauth2-app-deletion-logout branch July 7, 2025 17:57
@github-actions github-actions bot locked and limited conversation to collaborators Jul 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@BrunoQuaresma BrunoQuaresma BrunoQuaresma approved these changes

@johnstcn johnstcn johnstcn approved these changes

Assignees

@ThomasK33 ThomasK33

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ThomasK33 @johnstcn @BrunoQuaresma