refactor: replace OAuth2 callback_url with RFC 6749 compliant redirect_uris #18810

ThomasK33 · 2025-07-09T12:02:59Z

RFC 6749 Compliance: Replace callback_url with redirect_uris Array

This PR improves OAuth2 provider compliance with RFC 6749 by replacing the single callback_url field with a proper redirect_uris array. This change allows OAuth2 clients to register multiple valid redirect URIs and enforces exact URI matching as required by the specification.

Key changes:

Migrated database schema to remove callback_url column and make redirect_uris the source of truth
Updated API endpoints to accept and validate multiple redirect URIs
Modified OAuth2 authorization flow to perform exact URI matching against registered URIs
Changed device authorization endpoint to accept form-urlencoded data instead of JSON
Updated frontend to support adding/removing multiple redirect URIs
Added database migration with backward compatibility support

This change improves security by enforcing stricter redirect URI validation and provides more flexibility for OAuth2 clients that need multiple callback endpoints.

ThomasK33 · 2025-07-09T12:03:17Z

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

coderd/apidoc/docs.go

coderd/database/dbauthz/dbauthz_test.go

dannykopping · 2025-07-17T13:52:58Z

coderd/database/migrations/000351_remove_oauth2_callback_url.up.sql

+-- being an array with a single NULL element. This is preferable to an empty
+-- array as it will pass a CHECK for array length > 0, enforcing that all
+-- apps have at least one URI entry, even if it's null.


Can't we make the CHECK check for the cardinality OR the "nullness"?

I think I'm doing that in the client_credentials PR up in the stack, but I would need to double-check.

I'm sorry. I just reread the comment. I'd rather not introduce a possible null here and perform a clean migration from callback_url to redirect_uris, then make sure that all of them get carried over.

The constraint for redirect_uris will be loosened in the PR introducing client_credentials. We'll then allow setting an empty redirect_uris array but not a null one.

coderd/oauth2provider/authorize.go

dannykopping · 2025-07-17T14:02:07Z

coderd/oauth2provider/middleware.go

 				site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
 					Status:       http.StatusInternalServerError,
 					HideStatus:   false,
 					Title:        "Internal Server Error",
-					Description:  err.Error(),
+					Description:  "OAuth2 app has no registered redirect URIs",


Ditto; bad request rather than internal server error.

coderd/oauth2provider/tokens.go

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

johnstcn · 2025-07-21T20:22:25Z

codersdk/oauth2.go

-	Icon        string    `json:"icon"`
+	ID           uuid.UUID `json:"id" format:"uuid"`
+	Name         string    `json:"name"`
+	RedirectURIs []string  `json:"redirect_uris"`


As I understand, you could only get this response if the OAuth2 provider was enabled, which was only available on development builds and not release builds. So not a breaking change, correct?

Yes, either a dev build or if one actively enables the oauth2 experiment

johnstcn · 2025-07-21T20:24:12Z

coderd/oauth2provider/tokens.go

+	// RFC 6749: Exact match against registered redirect URIs
+	if slices.Contains(registeredRedirectURIs, redirectURIParam) {
+		return redirectURL
+	}


suggestion: add a link to the spec https://datatracker.ietf.org/doc/html/rfc6749#section-3.1.2.3

johnstcn · 2025-07-21T20:27:06Z

coderd/oauth2.go

+// @Accept application/x-www-form-urlencoded
 // @Produce json


I found it hard to believe, but here it is https://datatracker.ietf.org/doc/html/rfc8628#section-3.2

…pliance Change-Id: I4823e475777ebdf75e3a80e47ff6bef1a556cd55 Signed-off-by: Thomas Kosiewski <tk@coder.com>

aslilac · 2025-07-23T15:24:34Z

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

+					<div css={{ display: "flex", alignItems: "center", gap: 8 }}>
+						<span css={{ fontWeight: 500, fontSize: 14 }}>Redirect URIs</span>


could you please use tailwind classes? we're currently trying to move away from emotion

aslilac · 2025-07-23T15:25:42Z

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

+						<IconButton
+							size="small"
+							onClick={addRedirectUri}
+							css={{ padding: 4 }}


Suggested change

css={{ padding: 4 }}

className="p-1"

same here, and for reference one tailwind "spacing" is basically 4px

aslilac · 2025-07-23T15:28:56Z

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

+					{redirectUris.map((uri, index) => (
+						<div
+							key={index}
+							css={{ display: "flex", gap: 8, alignItems: "flex-start" }}


Suggested change

css={{ display: "flex", gap: 8, alignItems: "flex-start" }}

className="flex gap-2 items-start"

aslilac · 2025-07-23T15:35:35Z

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

+								<IconButton
+									size="small"
+									onClick={() => removeRedirectUri(index)}
+									css={{ padding: 4, marginTop: index === 0 ? 8 : 0 }}


is the marginTop here to "center" the button? if so you could use flex items-center.

also we probably want to add a story for this component, I went to look at it locally and it doesn't seem accessible from the existing OAuth2 stories.

github-actions bot assigned ThomasK33 Jul 9, 2025

ThomasK33 mentioned this pull request Jul 9, 2025

feat: standardize OAuth2 endpoints and implement token revocation #18809

Open

ThomasK33 force-pushed the thomask33/07-07-feat_standardize_oauth2_endpoints_and_add_token_revocation branch from dd9cb2f to ae754f4 Compare July 9, 2025 12:16

ThomasK33 force-pushed the thomask33/07-08-feat_replace_callback_url_with_redirect_uris_for_oauth2_rfc_6749_compliance branch from f9e6552 to 1444efe Compare July 9, 2025 12:16

ThomasK33 changed the base branch from thomask33/07-07-feat_standardize_oauth2_endpoints_and_add_token_revocation to graphite-base/18810 July 9, 2025 15:19

ThomasK33 force-pushed the thomask33/07-08-feat_replace_callback_url_with_redirect_uris_for_oauth2_rfc_6749_compliance branch from 1444efe to ef4a6c8 Compare July 9, 2025 22:21

ThomasK33 force-pushed the graphite-base/18810 branch from ae754f4 to ab73979 Compare July 9, 2025 22:21

ThomasK33 changed the base branch from graphite-base/18810 to thomask33/07-07-feat_standardize_oauth2_endpoints_and_add_token_revocation July 9, 2025 22:21

ThomasK33 force-pushed the thomask33/07-08-feat_replace_callback_url_with_redirect_uris_for_oauth2_rfc_6749_compliance branch from ef4a6c8 to e24c4b5 Compare July 10, 2025 09:15

ThomasK33 requested a review from johnstcn July 10, 2025 09:25

ThomasK33 marked this pull request as ready for review July 10, 2025 09:25

ThomasK33 mentioned this pull request Jul 10, 2025

feat: add cleanup for expired OAuth2 provider app codes and tokens #18825

Open

ThomasK33 requested review from Emyrk and mafredri July 10, 2025 11:10

ThomasK33 force-pushed the thomask33/07-08-feat_replace_callback_url_with_redirect_uris_for_oauth2_rfc_6749_compliance branch from e24c4b5 to 98e7f95 Compare July 12, 2025 12:55

ThomasK33 force-pushed the thomask33/07-07-feat_standardize_oauth2_endpoints_and_add_token_revocation branch from ab73979 to b89c367 Compare July 12, 2025 12:55

ThomasK33 mentioned this pull request Jul 12, 2025

feat: add OAuth2 client credentials grant type and user ownership #18841

Open

ThomasK33 force-pushed the thomask33/07-07-feat_standardize_oauth2_endpoints_and_add_token_revocation branch from b89c367 to b73b71d Compare July 14, 2025 12:43

ThomasK33 force-pushed the thomask33/07-08-feat_replace_callback_url_with_redirect_uris_for_oauth2_rfc_6749_compliance branch from 98e7f95 to 2792051 Compare July 14, 2025 12:43

This was referenced Jul 14, 2025

feat: add client credentials OAuth2 applications for API access #18846

Open

feat: add OAuth2 token bulk revocation endpoint #18847

Open

ThomasK33 changed the base branch from thomask33/07-07-feat_standardize_oauth2_endpoints_and_add_token_revocation to graphite-base/18810 July 14, 2025 15:16

ThomasK33 force-pushed the thomask33/07-08-feat_replace_callback_url_with_redirect_uris_for_oauth2_rfc_6749_compliance branch from 2792051 to 3dd0c37 Compare July 14, 2025 16:22

ThomasK33 force-pushed the graphite-base/18810 branch from b73b71d to 386d77d Compare July 14, 2025 16:22

ThomasK33 changed the base branch from graphite-base/18810 to thomask33/07-07-feat_standardize_oauth2_endpoints_and_add_token_revocation July 14, 2025 16:22

ThomasK33 mentioned this pull request Jul 14, 2025

docs: remove dbmem references from documentation files #18861

Merged

ThomasK33 force-pushed the thomask33/07-07-feat_standardize_oauth2_endpoints_and_add_token_revocation branch from 386d77d to b58eed8 Compare July 14, 2025 17:18

ThomasK33 force-pushed the thomask33/07-08-feat_replace_callback_url_with_redirect_uris_for_oauth2_rfc_6749_compliance branch from 3dd0c37 to 962c22c Compare July 14, 2025 17:18

ThomasK33 changed the base branch from thomask33/07-07-feat_standardize_oauth2_endpoints_and_add_token_revocation to graphite-base/18810 July 15, 2025 16:03

ThomasK33 force-pushed the thomask33/07-08-feat_replace_callback_url_with_redirect_uris_for_oauth2_rfc_6749_compliance branch from 962c22c to bed62ad Compare July 15, 2025 17:24

ThomasK33 force-pushed the graphite-base/18810 branch from b58eed8 to 9393060 Compare July 15, 2025 17:24

ThomasK33 changed the base branch from graphite-base/18810 to thomask33/07-07-feat_standardize_oauth2_endpoints_and_add_token_revocation July 15, 2025 17:24

ThomasK33 mentioned this pull request Jul 15, 2025

fix: allow non-HTTP URIs in OAuth2 provider redirect URIs #18880

Draft

ThomasK33 requested a review from dannykopping July 17, 2025 12:53