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

ThomasK33 · 2025-07-04T13:32:44Z

Problem

Users were being automatically logged out when deleting OAuth2 applications.

Root Cause

User deletes OAuth2 app successfully
React Query automatically refetches the app data
Management API incorrectly returned 401 Unauthorized for the missing app
Frontend axios interceptor sees 401 and calls signOut()
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

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]>

johnstcn · 2025-07-07T12:49:32Z

.claude/scripts/format.sh

+			_CANONICALIZE_CMD="python3"
+		elif command -v python >/dev/null 2>&1; then
+			_CANONICALIZE_CMD="python"
+		elif command -v perl >/dev/null 2>&1; then
+			_CANONICALIZE_CMD="perl"


Why do we need to run python or perl interpreters at all here?

Those are fallbacks in case realpath or readlink are not available.

While working with CC hooks, I noticed that their file paths are wonky. Sometimes, they are relative, and sometimes, they are absolute, often a misformulation of both.

So this is just in case someone develops coder on an env that doesn’t have realpath or readlink.

So this is just in case someone develops coder on an env that doesn’t have realpath or readlink.

Can you give an example? I see these available in Nix and they're also available on darwin via Brew / coreutils.

Unless we don't care about Windows development, then I'm happy to zap this

Actually, this is a CC hook, and CC only works on Linux/macOS.

I'll yeet the fallbacks later.

johnstcn

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

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

BrunoQuaresma · 2025-07-07T13:01:32Z

site/src/pages/DeploymentSettingsPage/OAuth2AppsSettingsPage/EditOAuth2AppPageView.tsx

@@ -141,6 +141,7 @@ export const EditOAuth2AppPageView: FC<EditOAuth2AppProps> = ({
 							confirmLoading={mutatingResource.deleteApp}
 							name={app.name}
 							entity="OAuth2 application"
+							info="⚠️ Warning: Deleting this OAuth2 application will immediately invalidate all active sessions and API keys associated with it. Users currently authenticated through this application will be logged out and need to re-authenticate."


Since this message is displayed inside of a DeleteDialog, which already has a style to warn users, we don't need to prefix it with "⚠️ Warning:".

BrunoQuaresma · 2025-07-07T13:11:51Z

site/src/pages/WorkspacePage/WorkspacePage.test.tsx

@@ -554,6 +558,7 @@ describe("WorkspacePage", () => {
 						appearance: MockAppearanceConfig,
 						entitlements: MockEntitlements,
 						experiments: [],
+						buildInfo: { version: "v0.0.0-test" } as BuildInfoResponse,


Nit: You can use a more complete build info object, from our existent mocks, to avoid casting partial values using as. The advantage on that, IMHO, is to have a more reliable value. Time to time, I face some bugs in tests because the value types are correct, but the value is missing some properties.

To "fix" that you could do:

import { MockBuildInfo } from "testHelpers/entities" return <DashboardContext.Provider value={{ buildInfo: { ..MockBuildInfo, version: "v0.0.0-test" } }} />

BrunoQuaresma · 2025-07-07T13:13:45Z

site/src/testHelpers/storybook.tsx

@@ -56,6 +56,7 @@ export const withDashboardProvider = (
 				entitlements,
 				experiments,
 				appearance: MockAppearanceConfig,
+				buildInfo: { version: "v0.0.0-test" } as BuildInfoResponse,


Same as before. However, If having the version as v0.0.0-test is what we want to use by default, you could just use the MockBuildInfo directly, and update the value prop to use the new value.

ThomasK33 · 2025-07-07T13:16:52Z

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 · 2025-07-07T13:22:14Z

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.

github-actions bot assigned ThomasK33 Jul 4, 2025

ThomasK33 requested a review from BrunoQuaresma July 4, 2025 13:33

ThomasK33 force-pushed the fix/oauth2-app-deletion-logout branch from 7fac49d to 932c04f Compare July 4, 2025 13:51

ThomasK33 force-pushed the fix/oauth2-app-deletion-logout branch from 932c04f to eb425dd Compare July 4, 2025 13:52

ThomasK33 requested a review from johnstcn July 7, 2025 12:14

johnstcn reviewed Jul 7, 2025

View reviewed changes

BrunoQuaresma approved these changes Jul 7, 2025

View reviewed changes

johnstcn approved these changes Jul 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

ThomasK33 commented Jul 4, 2025

Uh oh!

johnstcn Jul 7, 2025

Uh oh!

ThomasK33 Jul 7, 2025

Uh oh!

johnstcn Jul 7, 2025

Uh oh!

ThomasK33 Jul 7, 2025

Uh oh!

ThomasK33 Jul 7, 2025

Uh oh!

ThomasK33 Jul 7, 2025

Uh oh!

johnstcn left a comment

Uh oh!

BrunoQuaresma left a comment

Uh oh!

BrunoQuaresma Jul 7, 2025

Uh oh!

BrunoQuaresma Jul 7, 2025

Uh oh!

BrunoQuaresma Jul 7, 2025

Uh oh!

ThomasK33 commented Jul 7, 2025

Uh oh!

johnstcn commented Jul 7, 2025

Uh oh!

Uh oh!

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

Are you sure you want to change the base?

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

Conversation

ThomasK33 commented Jul 4, 2025

Problem

Root Cause

Solution

Additional Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johnstcn left a comment

Choose a reason for hiding this comment

Uh oh!

BrunoQuaresma left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThomasK33 commented Jul 7, 2025

Uh oh!

johnstcn commented Jul 7, 2025

Uh oh!

Uh oh!