feat: register public clients without a secret (#28046)

Third in the stack, split out of #27873. This is where dynamic client
registration first produces a public client.

- A registration requesting token_endpoint_auth_method `none` issues no
secret and persists the client_type derived in #28043. The response
omits client_secret entirely, per RFC 7591 §3.2.1.
- Discovery still advertises only the two secret-based methods. `none`
is withheld until the token endpoint honors it, so a conforming client
is not told to attempt an exchange that would be rejected.
- The app and its secret are written in one transaction. They were two
independent inserts, so a failed second one left a committed app that
could never authenticate while still holding a registration access
token. Pre-existing, but a public client's missing secret row makes the
orphaned case unspottable by inspection.
- registration_client_uri uses url.JoinPath, fixing a trailing-slash
access URL producing //oauth2/clients/{id}.
- Docs state both limits above, so the page does not describe a flow
that returns 400 until #28047 lands.

A client registered here cannot obtain a token yet; #28047 adds that.
Dynamic client registration is off by default, so none of this is
user-visible until then.

Refs
https://linear.app/codercom/issue/ENG-3029/oauth2-support-public-client
This commit is contained in:
Bobby Ho
2026-08-20 12:49:47 -07:00
committed by GitHub
parent edd8bf8b9b
commit 6348c83142
9 changed files with 489 additions and 88 deletions
+11 -9
View File
@@ -28,15 +28,17 @@ func GetAuthorizationServerMetadata(db database.Store, accessURL *url.URL) http.
}
metadata := codersdk.OAuth2AuthorizationServerMetadata{
Issuer: accessURL.String(),
AuthorizationEndpoint: accessURL.JoinPath("/oauth2/authorize").String(),
TokenEndpoint: accessURL.JoinPath("/oauth2/tokens").String(),
RevocationEndpoint: accessURL.JoinPath("/oauth2/revoke").String(), // RFC 7009
ResponseTypesSupported: []codersdk.OAuth2ProviderResponseType{codersdk.OAuth2ProviderResponseTypeCode},
GrantTypesSupported: []codersdk.OAuth2ProviderGrantType{codersdk.OAuth2ProviderGrantTypeAuthorizationCode, codersdk.OAuth2ProviderGrantTypeRefreshToken},
CodeChallengeMethodsSupported: []codersdk.OAuth2PKCECodeChallengeMethod{codersdk.OAuth2PKCECodeChallengeMethodS256},
ScopesSupported: rbac.ExternalScopeNames(),
TokenEndpointAuthMethodsSupported: []codersdk.OAuth2TokenEndpointAuthMethod{codersdk.OAuth2TokenEndpointAuthMethodClientSecretBasic, codersdk.OAuth2TokenEndpointAuthMethodClientSecretPost},
Issuer: accessURL.String(),
AuthorizationEndpoint: accessURL.JoinPath("/oauth2/authorize").String(),
TokenEndpoint: accessURL.JoinPath("/oauth2/tokens").String(),
RevocationEndpoint: accessURL.JoinPath("/oauth2/revoke").String(), // RFC 7009
ResponseTypesSupported: []codersdk.OAuth2ProviderResponseType{codersdk.OAuth2ProviderResponseTypeCode},
GrantTypesSupported: []codersdk.OAuth2ProviderGrantType{codersdk.OAuth2ProviderGrantTypeAuthorizationCode, codersdk.OAuth2ProviderGrantTypeRefreshToken},
CodeChallengeMethodsSupported: []codersdk.OAuth2PKCECodeChallengeMethod{codersdk.OAuth2PKCECodeChallengeMethodS256},
ScopesSupported: rbac.ExternalScopeNames(),
// Not gated on dcrEnabled: existing clients still need to
// exchange tokens when new registrations are turned off.
TokenEndpointAuthMethodsSupported: codersdk.AdvertisedOAuth2TokenEndpointAuthMethods(),
}
if dcrEnabled {
metadata.RegistrationEndpoint = accessURL.JoinPath("/oauth2/register").String() // RFC 7591
+5
View File
@@ -42,6 +42,11 @@ func TestOAuth2AuthorizationServerMetadata(t *testing.T) {
require.Contains(t, metadata.GrantTypesSupported, codersdk.OAuth2ProviderGrantTypeAuthorizationCode)
require.Contains(t, metadata.GrantTypesSupported, codersdk.OAuth2ProviderGrantTypeRefreshToken)
require.Contains(t, metadata.CodeChallengeMethodsSupported, codersdk.OAuth2PKCECodeChallengeMethodS256)
// Pins the exact advertised set, not just that it contains something
// expected: a hardcoded list that dropped an accepted method or kept an
// unhonored one ("none": the token endpoint doesn't accept it yet) would
// still pass a Contains-only check.
require.ElementsMatch(t, codersdk.AdvertisedOAuth2TokenEndpointAuthMethods(), metadata.TokenEndpointAuthMethodsSupported)
// Supported scopes are published from the curated catalog
require.Equal(t, rbac.ExternalScopeNames(), metadata.ScopesSupported)
}
@@ -77,6 +77,26 @@ func CreateTestOAuth2App(t *testing.T, client *codersdk.Client) (*codersdk.OAuth
return &app, secret.ClientSecretFull
}
// RegisterPublicClient registers a public (secretless, PKCE-only) OAuth2 client
// via RFC 7591 dynamic registration, the only way to create one. This is the
// public counterpart to CreateTestOAuth2App. The caller must call EnableDCR
// first, and needs owner-level permissions to do so.
func RegisterPublicClient(t *testing.T, client *codersdk.Client, name, redirectURI string) codersdk.OAuth2ClientRegistrationResponse {
t.Helper()
ctx := testutil.Context(t, testutil.WaitLong)
resp, err := client.PostOAuth2ClientRegistration(ctx, codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{redirectURI},
ClientName: fmt.Sprintf("%s-%s", name, testutil.MustRandString(t, 10)),
TokenEndpointAuthMethod: codersdk.OAuth2TokenEndpointAuthMethodNone,
})
require.NoError(t, err, "failed to register public OAuth2 client")
// A public client is issued no secret. Asserting it here means every caller
// inherits the check rather than restating it.
require.Empty(t, resp.ClientSecret, "public client must not be issued a secret")
return resp
}
// EnableDCR turns on dynamic client registration for the deployment.
// DCR defaults to disabled, so any test that registers a client via
// POST /oauth2/register must call this first. The caller-provided client
@@ -634,3 +634,21 @@ func TestOAuth2ErrorResponses(t *testing.T) {
)
})
}
// TestOAuth2RegisterPublicClient exercises the RegisterPublicClient helper
// end-to-end against a real server: registering with token_endpoint_auth_
// method "none" issues no client secret. A bug in the helper's request
// shape or assertions would otherwise ride uncaught until a later PR's test
// happened to call it.
func TestOAuth2RegisterPublicClient(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{
IncludeProvisionerDaemon: false,
})
_ = coderdtest.CreateFirstUser(t, client)
oauth2providertest.EnableDCR(t, client)
resp := oauth2providertest.RegisterPublicClient(t, client, "test-public-client", "https://example.com/callback")
require.NotEmpty(t, resp.ClientID)
}
+87 -64
View File
@@ -72,13 +72,22 @@ func CreateDynamicClientRegistration(db database.Store, accessURL *url.URL, audi
// Apply defaults
req = req.ApplyDefaults()
// Generate client credentials
clientType := req.DetermineClientType()
isPublic := clientType == codersdk.OAuth2ClientTypePublic
// Public clients authenticate with PKCE alone and never receive a
// secret (RFC 7591 §2, OAuth 2.1 §2.1).
clientID := uuid.New()
clientSecret, hashedSecret, err := generateClientCredentials()
if err != nil {
writeOAuth2RegistrationError(ctx, rw, http.StatusInternalServerError,
"server_error", "Failed to generate client credentials")
return
var clientSecret string
var hashedSecret []byte
if !isPublic {
var err error
clientSecret, hashedSecret, err = generateClientCredentials()
if err != nil {
writeOAuth2RegistrationError(ctx, rw, http.StatusInternalServerError,
"server_error", "Failed to generate client credentials")
return
}
}
// Generate registration access token for RFC 7592 management
@@ -92,35 +101,72 @@ func CreateDynamicClientRegistration(db database.Store, accessURL *url.URL, audi
// Store in database - use system context since this is a public endpoint
now := dbtime.Now()
clientName := req.GenerateClientName()
//nolint:gocritic // OAuth2 system context — dynamic registration is a public endpoint
app, err := db.InsertOAuth2ProviderApp(dbauthz.AsSystemOAuth2(ctx), database.InsertOAuth2ProviderAppParams{
ID: clientID,
CreatedAt: now,
UpdatedAt: now,
Name: clientName,
Icon: req.LogoURI,
CallbackURL: req.RedirectURIs[0], // Primary redirect URI
RedirectUris: req.RedirectURIs,
ClientType: string(req.DetermineClientType()),
DynamicallyRegistered: sql.NullBool{Bool: true, Valid: true},
ClientIDIssuedAt: sql.NullTime{Time: now, Valid: true},
ClientSecretExpiresAt: sql.NullTime{}, // No expiration for now
GrantTypes: slice.ToStrings(req.GrantTypes),
ResponseTypes: slice.ToStrings(req.ResponseTypes),
TokenEndpointAuthMethod: sql.NullString{String: string(req.TokenEndpointAuthMethod), Valid: true},
Scope: sql.NullString{String: req.Scope, Valid: true},
Contacts: req.Contacts,
ClientUri: sql.NullString{String: req.ClientURI, Valid: req.ClientURI != ""},
LogoUri: sql.NullString{String: req.LogoURI, Valid: req.LogoURI != ""},
TosUri: sql.NullString{String: req.TOSURI, Valid: req.TOSURI != ""},
PolicyUri: sql.NullString{String: req.PolicyURI, Valid: req.PolicyURI != ""},
JwksUri: sql.NullString{String: req.JWKSURI, Valid: req.JWKSURI != ""},
Jwks: pqtype.NullRawMessage{RawMessage: req.JWKS, Valid: len(req.JWKS) > 0},
SoftwareID: sql.NullString{String: req.SoftwareID, Valid: req.SoftwareID != ""},
SoftwareVersion: sql.NullString{String: req.SoftwareVersion, Valid: req.SoftwareVersion != ""},
RegistrationAccessToken: hashedRegToken,
RegistrationClientUri: sql.NullString{String: fmt.Sprintf("%s/oauth2/clients/%s", accessURL.String(), clientID), Valid: true},
})
// The app and its secret are written in one transaction. A partial
// write would commit an app that can never authenticate, and which
// still holds a registration access token.
var app database.OAuth2ProviderApp
err = db.InTx(func(tx database.Store) error {
var err error
//nolint:gocritic // OAuth2 system context, dynamic registration is a public endpoint
app, err = tx.InsertOAuth2ProviderApp(dbauthz.AsSystemOAuth2(ctx), database.InsertOAuth2ProviderAppParams{
ID: clientID,
CreatedAt: now,
UpdatedAt: now,
Name: clientName,
Icon: req.LogoURI,
CallbackURL: req.RedirectURIs[0], // Primary redirect URI
RedirectUris: req.RedirectURIs,
ClientType: string(clientType),
DynamicallyRegistered: sql.NullBool{Bool: true, Valid: true},
ClientIDIssuedAt: sql.NullTime{Time: now, Valid: true},
ClientSecretExpiresAt: sql.NullTime{}, // No expiration for now
GrantTypes: slice.ToStrings(req.GrantTypes),
ResponseTypes: slice.ToStrings(req.ResponseTypes),
TokenEndpointAuthMethod: sql.NullString{String: string(req.TokenEndpointAuthMethod), Valid: true},
Scope: sql.NullString{String: req.Scope, Valid: true},
Contacts: req.Contacts,
ClientUri: sql.NullString{String: req.ClientURI, Valid: req.ClientURI != ""},
LogoUri: sql.NullString{String: req.LogoURI, Valid: req.LogoURI != ""},
TosUri: sql.NullString{String: req.TOSURI, Valid: req.TOSURI != ""},
PolicyUri: sql.NullString{String: req.PolicyURI, Valid: req.PolicyURI != ""},
JwksUri: sql.NullString{String: req.JWKSURI, Valid: req.JWKSURI != ""},
Jwks: pqtype.NullRawMessage{RawMessage: req.JWKS, Valid: len(req.JWKS) > 0},
SoftwareID: sql.NullString{String: req.SoftwareID, Valid: req.SoftwareID != ""},
SoftwareVersion: sql.NullString{String: req.SoftwareVersion, Valid: req.SoftwareVersion != ""},
RegistrationAccessToken: hashedRegToken,
// JoinPath, not Sprintf: an access URL configured with a
// trailing slash would otherwise mint "//oauth2/clients/{id}"
// and hand it to the client as its management endpoint.
RegistrationClientUri: sql.NullString{String: accessURL.JoinPath("/oauth2/clients", clientID.String()).String(), Valid: true},
})
if err != nil {
return xerrors.Errorf("insert oauth2 provider app: %w", err)
}
if isPublic {
return nil
}
// Extract the prefix for the secret row below.
parsedSecret, err := ParseFormattedSecret(clientSecret)
if err != nil {
return xerrors.Errorf("parse generated secret: %w", err)
}
//nolint:gocritic // OAuth2 system context, dynamic registration is a public endpoint
_, err = tx.InsertOAuth2ProviderAppSecret(dbauthz.AsSystemOAuth2(ctx), database.InsertOAuth2ProviderAppSecretParams{
ID: uuid.New(),
CreatedAt: now,
SecretPrefix: []byte(parsedSecret.Prefix),
HashedSecret: hashedSecret,
DisplaySecret: createDisplaySecret(clientSecret),
AppID: clientID,
})
if err != nil {
return xerrors.Errorf("insert oauth2 provider app secret: %w", err)
}
return nil
}, nil)
if err != nil {
logger.Error(ctx, "failed to store oauth2 client registration",
slog.Error(err),
@@ -132,29 +178,6 @@ func CreateDynamicClientRegistration(db database.Store, accessURL *url.URL, audi
return
}
// Create client secret - parse the formatted secret to get components
parsedSecret, err := ParseFormattedSecret(clientSecret)
if err != nil {
writeOAuth2RegistrationError(ctx, rw, http.StatusInternalServerError,
"server_error", "Failed to parse generated secret")
return
}
//nolint:gocritic // OAuth2 system context — dynamic registration is a public endpoint
_, err = db.InsertOAuth2ProviderAppSecret(dbauthz.AsSystemOAuth2(ctx), database.InsertOAuth2ProviderAppSecretParams{
ID: uuid.New(),
CreatedAt: now,
SecretPrefix: []byte(parsedSecret.Prefix),
HashedSecret: hashedSecret,
DisplaySecret: createDisplaySecret(clientSecret),
AppID: clientID,
})
if err != nil {
writeOAuth2RegistrationError(ctx, rw, http.StatusInternalServerError,
"server_error", "Failed to store client secret")
return
}
// Set audit log data
aReq.New = app
@@ -202,7 +225,7 @@ func GetClientConfiguration(db database.Store) http.HandlerFunc {
}
// Get app by client ID
//nolint:gocritic // OAuth2 system context — RFC 7592 client configuration endpoint
//nolint:gocritic // OAuth2 system context, RFC 7592 client configuration endpoint
app, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), clientID)
if err != nil {
if xerrors.Is(err, sql.ErrNoRows) {
@@ -288,7 +311,7 @@ func UpdateClientConfiguration(db database.Store, auditor *audit.Auditor, logger
req = req.ApplyDefaults()
// Get existing app to verify it exists and is dynamically registered
//nolint:gocritic // OAuth2 system context — RFC 7592 client configuration endpoint
//nolint:gocritic // OAuth2 system context, RFC 7592 client configuration endpoint
existingApp, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), clientID)
if err == nil {
aReq.Old = existingApp
@@ -340,7 +363,7 @@ func UpdateClientConfiguration(db database.Store, auditor *audit.Auditor, logger
// Update app in database
now := dbtime.Now()
//nolint:gocritic // OAuth2 system context — RFC 7592 client configuration endpoint
//nolint:gocritic // OAuth2 system context, RFC 7592 client configuration endpoint
updatedApp, err := db.UpdateOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), database.UpdateOAuth2ProviderAppByClientIDParams{
ID: clientID,
UpdatedAt: now,
@@ -428,7 +451,7 @@ func DeleteClientConfiguration(db database.Store, auditor *audit.Auditor, logger
}
// Get existing app to verify it exists and is dynamically registered
//nolint:gocritic // OAuth2 system context — RFC 7592 client configuration endpoint
//nolint:gocritic // OAuth2 system context, RFC 7592 client configuration endpoint
existingApp, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), clientID)
if err == nil {
aReq.Old = existingApp
@@ -452,7 +475,7 @@ func DeleteClientConfiguration(db database.Store, auditor *audit.Auditor, logger
}
// Delete the client and all associated data (tokens, secrets, etc.)
//nolint:gocritic // OAuth2 system context — RFC 7592 client configuration endpoint
//nolint:gocritic // OAuth2 system context, RFC 7592 client configuration endpoint
err = db.DeleteOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), clientID)
if err != nil {
writeOAuth2RegistrationError(ctx, rw, http.StatusInternalServerError,
@@ -504,7 +527,7 @@ func RequireRegistrationAccessToken(db database.Store) func(http.Handler) http.H
}
// Get the client and verify the registration access token
//nolint:gocritic // OAuth2 system context — RFC 7592 registration access token validation
//nolint:gocritic // OAuth2 system context, RFC 7592 registration access token validation
app, err := db.GetOAuth2ProviderAppByClientID(dbauthz.AsSystemOAuth2(ctx), clientID)
if err != nil {
if xerrors.Is(err, sql.ErrNoRows) {
+298
View File
@@ -13,11 +13,14 @@ import (
"github.com/go-chi/chi/v5"
"github.com/google/uuid"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"
"golang.org/x/xerrors"
"cdr.dev/slog/v3/sloggers/slogtest"
"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbgen"
"github.com/coder/coder/v2/coderd/database/dbmock"
"github.com/coder/coder/v2/coderd/database/dbtestutil"
"github.com/coder/coder/v2/coderd/oauth2provider"
"github.com/coder/coder/v2/coderd/tracing"
@@ -104,6 +107,301 @@ func TestCreateDynamicClientRegistration_DCREnabled(t *testing.T) {
}
}
func TestCreateDynamicClientRegistration_ClientType(t *testing.T) {
t.Parallel()
accessURL, err := url.Parse("https://oauth2-registration-client-type-test.example.com")
require.NoError(t, err)
tests := []struct {
name string
req codersdk.OAuth2ClientRegistrationRequest
wantClientType string
wantSecret bool
}{
{
name: "DefaultAuthMethodIsConfidential",
req: codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
},
wantClientType: "confidential",
wantSecret: true,
},
{
name: "ClientSecretPostIsConfidential",
req: codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
TokenEndpointAuthMethod: codersdk.OAuth2TokenEndpointAuthMethodClientSecretPost,
},
wantClientType: "confidential",
wantSecret: true,
},
{
name: "NoneIsPublicWithNoSecret",
req: codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
TokenEndpointAuthMethod: codersdk.OAuth2TokenEndpointAuthMethodNone,
},
wantClientType: "public",
wantSecret: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
db, _ := dbtestutil.NewDB(t)
require.NoError(t, db.UpsertOAuth2DCREnabled(ctx, true))
logger := slogtest.Make(t, nil)
auditor := audit.NewNop()
handler := tracing.StatusWriterMiddleware(oauth2provider.CreateDynamicClientRegistration(db, accessURL, &auditor, logger))
body, err := json.Marshal(tt.req)
require.NoError(t, err)
r := httptest.NewRequest(http.MethodPost, "/oauth2/register", bytes.NewReader(body)).WithContext(ctx)
r.Header.Set("Content-Type", "application/json")
rw := httptest.NewRecorder()
handler.ServeHTTP(rw, r)
require.Equal(t, http.StatusCreated, rw.Code)
var resp codersdk.OAuth2ClientRegistrationResponse
require.NoError(t, json.Unmarshal(rw.Body.Bytes(), &resp))
// RFC 7591 §3.2.1: client_secret is omitted entirely for a client
// that was not issued one. Assert against the raw body, because a
// decoded struct cannot tell an absent key from a present empty
// one, and key presence is exactly what a client branches on.
var rawBody map[string]json.RawMessage
require.NoError(t, json.Unmarshal(rw.Body.Bytes(), &rawBody))
if tt.wantSecret {
require.Contains(t, rawBody, "client_secret")
require.NotEmpty(t, resp.ClientSecret)
} else {
require.NotContains(t, rawBody, "client_secret")
}
clientID, err := uuid.Parse(resp.ClientID)
require.NoError(t, err)
app, err := db.GetOAuth2ProviderAppByClientID(ctx, clientID)
require.NoError(t, err)
require.Equal(t, tt.wantClientType, app.ClientType)
secrets, err := db.GetOAuth2ProviderAppSecretsByAppID(ctx, clientID)
require.NoError(t, err)
if tt.wantSecret {
require.Len(t, secrets, 1)
} else {
require.Empty(t, secrets)
}
})
}
}
// TestCreateDynamicClientRegistration_RegistrationClientURI pins the
// accessURL.JoinPath fix: a trailing slash on the configured access URL
// used to mint "//oauth2/clients/{id}" via fmt.Sprintf. Without this test,
// reverting to fmt.Sprintf would pass every other test in this file, since
// none of them configures a trailing-slash accessURL.
func TestCreateDynamicClientRegistration_RegistrationClientURI(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
accessURL, err := url.Parse("https://oauth2-registration-client-uri-test.example.com/")
require.NoError(t, err)
db, _ := dbtestutil.NewDB(t)
require.NoError(t, db.UpsertOAuth2DCREnabled(ctx, true))
logger := slogtest.Make(t, nil)
auditor := audit.NewNop()
handler := tracing.StatusWriterMiddleware(oauth2provider.CreateDynamicClientRegistration(db, accessURL, &auditor, logger))
req := codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
}
body, err := json.Marshal(req)
require.NoError(t, err)
r := httptest.NewRequest(http.MethodPost, "/oauth2/register", bytes.NewReader(body)).WithContext(ctx)
r.Header.Set("Content-Type", "application/json")
rw := httptest.NewRecorder()
handler.ServeHTTP(rw, r)
require.Equal(t, http.StatusCreated, rw.Code)
var resp codersdk.OAuth2ClientRegistrationResponse
require.NoError(t, json.Unmarshal(rw.Body.Bytes(), &resp))
wantURI := "https://oauth2-registration-client-uri-test.example.com/oauth2/clients/" + resp.ClientID
require.Equal(t, wantURI, resp.RegistrationClientURI)
}
// TestCreateDynamicClientRegistration_Transaction verifies that the app insert
// and the secret insert share a single database transaction, so a failure
// partway through can't leave a permanently committed, orphaned app row with
// no matching secret.
//
// A mock store is used because the failure needs to be injected between the
// two inserts, which isn't reachable through the public registration API
// against a real database (the failing insert's unique secret_prefix is
// generated internally and can't be forced to collide from the outside).
// mDB.EXPECT().InTx(...).Times(1) is the key assertion: it fails the test if
// the two inserts are ever changed back to being independent, unwrapped
// db.InsertX calls, since that path never calls InTx at all.
func TestCreateDynamicClientRegistration_Transaction(t *testing.T) {
t.Parallel()
accessURL, err := url.Parse("https://oauth2-registration-tx-test.example.com")
require.NoError(t, err)
tests := []struct {
name string
secretInsertErr error
wantStatus int
}{
{
name: "BothInsertsShareOneTransaction",
wantStatus: http.StatusCreated,
},
{
name: "SecretInsertFailureFailsTheWholeRegistration",
secretInsertErr: xerrors.New("simulated secret insert failure"),
wantStatus: http.StatusInternalServerError,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
ctrl := gomock.NewController(t)
mDB := dbmock.NewMockStore(ctrl)
// A separate handle for the transaction, so a call made on the
// outer store is distinguishable from one made on tx.
mTx := dbmock.NewMockStore(ctrl)
mDB.EXPECT().GetOAuth2DCREnabled(gomock.Any()).Return(true, nil).Times(1)
mDB.EXPECT().InTx(gomock.Any(), gomock.Any()).DoAndReturn(
func(f func(database.Store) error, _ *database.TxOptions) error {
return f(mTx)
},
).Times(1)
// Both expectations live on mTx, not mDB, and that is the
// assertion. An insert issued on the outer store, whether after
// InTx returns or from inside the closure against the wrong
// handle, lands on mDB, which has no expectation for it, so
// gomock fails the unexpected call. Registering both on a single
// mock makes inside and outside the transaction indistinguishable
// and the test then passes either way, which is the trap the
// project's own InTx rule exists to catch
// (.claude/docs/DATABASE.md).
// Echoes params.ClientType rather than hardcoding it, so the mock
// stays honest about what the handler asked for; a hardcoded
// value would keep passing if a later change reused this
// scaffolding for a public request without updating it.
appCall := mTx.EXPECT().InsertOAuth2ProviderApp(gomock.Any(), gomock.Any()).
DoAndReturn(func(_ context.Context, params database.InsertOAuth2ProviderAppParams) (database.OAuth2ProviderApp, error) {
return database.OAuth2ProviderApp{
ID: uuid.New(),
ClientType: params.ClientType,
}, nil
}).
Times(1)
secretCall := mTx.EXPECT().InsertOAuth2ProviderAppSecret(gomock.Any(), gomock.Any()).
Return(database.OAuth2ProviderAppSecret{}, tt.secretInsertErr).
Times(1)
// The secret insert can only run after the app insert, matching
// registration.go's literal ordering inside the InTx closure.
gomock.InOrder(appCall, secretCall)
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: tt.secretInsertErr != nil})
auditor := audit.NewNop()
handler := tracing.StatusWriterMiddleware(oauth2provider.CreateDynamicClientRegistration(mDB, accessURL, &auditor, logger))
req := codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
}
body, err := json.Marshal(req)
require.NoError(t, err)
r := httptest.NewRequest(http.MethodPost, "/oauth2/register", bytes.NewReader(body)).WithContext(ctx)
r.Header.Set("Content-Type", "application/json")
rw := httptest.NewRecorder()
handler.ServeHTTP(rw, r)
require.Equal(t, tt.wantStatus, rw.Code)
})
}
}
// TestCreateDynamicClientRegistration_PublicClientSkipsSecretInsert verifies
// that a public client's registration issues no InsertOAuth2ProviderAppSecret
// call at all, rather than inserting a row with an empty secret. This pins
// the mechanism (no call made); TestCreateDynamicClientRegistration_ClientType
// /NoneIsPublicWithNoSecret pins the outcome (no row exists) against a real
// database. Both are kept: a handler that inserts an empty-secret row would
// pass the outcome test's require.Empty only if the read side also filters
// it out, so the mechanism test is the one that fails at the actual call
// site if that regresses.
func TestCreateDynamicClientRegistration_PublicClientSkipsSecretInsert(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
accessURL, err := url.Parse("https://oauth2-registration-public-tx-test.example.com")
require.NoError(t, err)
ctrl := gomock.NewController(t)
mDB := dbmock.NewMockStore(ctrl)
mTx := dbmock.NewMockStore(ctrl)
mDB.EXPECT().GetOAuth2DCREnabled(gomock.Any()).Return(true, nil).Times(1)
mDB.EXPECT().InTx(gomock.Any(), gomock.Any()).DoAndReturn(
func(f func(database.Store) error, _ *database.TxOptions) error {
return f(mTx)
},
).Times(1)
mTx.EXPECT().InsertOAuth2ProviderApp(gomock.Any(), gomock.Any()).
Return(database.OAuth2ProviderApp{
ID: uuid.New(),
ClientType: database.OAuth2ProviderAppClientTypePublic,
}, nil).
Times(1)
// The absence of an InsertOAuth2ProviderAppSecret expectation on either
// handle is the assertion: gomock fails on an unexpected call. This only
// holds because the two handles are distinct, so a secret insert issued
// anywhere is unexpected rather than absorbed by a shared expectation.
logger := slogtest.Make(t, nil)
auditor := audit.NewNop()
handler := tracing.StatusWriterMiddleware(oauth2provider.CreateDynamicClientRegistration(mDB, accessURL, &auditor, logger))
req := codersdk.OAuth2ClientRegistrationRequest{
RedirectURIs: []string{"https://example.com/callback"},
TokenEndpointAuthMethod: codersdk.OAuth2TokenEndpointAuthMethodNone,
}
body, err := json.Marshal(req)
require.NoError(t, err)
r := httptest.NewRequest(http.MethodPost, "/oauth2/register", bytes.NewReader(body)).WithContext(ctx)
r.Header.Set("Content-Type", "application/json")
rw := httptest.NewRecorder()
handler.ServeHTTP(rw, r)
require.Equal(t, http.StatusCreated, rw.Code)
}
// TestUpdateClientConfiguration_ClientTypeIsImmutable verifies that an
// RFC 7592 update cannot move a registered client between public and
// confidential. Allowing it would either drop the secret requirement for a
+4 -4
View File
@@ -271,7 +271,7 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
if err != nil {
return codersdk.OAuth2TokenResponse{}, errBadSecret
}
//nolint:gocritic // OAuth2 system context — users cannot read secrets
//nolint:gocritic // OAuth2 system context, users cannot read secrets
dbSecret, err := db.GetOAuth2ProviderAppSecretByPrefix(dbauthz.AsSystemOAuth2(ctx), []byte(secret.Prefix))
if errors.Is(err, sql.ErrNoRows) {
return codersdk.OAuth2TokenResponse{}, errBadSecret
@@ -299,7 +299,7 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
if err != nil {
return codersdk.OAuth2TokenResponse{}, errBadCode
}
//nolint:gocritic // OAuth2 system context — no authenticated user during token exchange
//nolint:gocritic // OAuth2 system context, no authenticated user during token exchange
dbCode, err := db.GetOAuth2ProviderAppCodeByPrefix(dbauthz.AsSystemOAuth2(ctx), []byte(code.Prefix))
if errors.Is(err, sql.ErrNoRows) {
return codersdk.OAuth2TokenResponse{}, errBadCode
@@ -462,7 +462,7 @@ func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAut
if err != nil {
return codersdk.OAuth2TokenResponse{}, errBadToken
}
//nolint:gocritic // OAuth2 system context — no authenticated user during refresh
//nolint:gocritic // OAuth2 system context, no authenticated user during refresh
dbToken, err := db.GetOAuth2ProviderAppTokenByPrefix(dbauthz.AsSystemOAuth2(ctx), []byte(token.Prefix))
if errors.Is(err, sql.ErrNoRows) {
return codersdk.OAuth2TokenResponse{}, errBadToken
@@ -499,7 +499,7 @@ func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAut
}
// Grab the user roles so we can perform the refresh as the user.
//nolint:gocritic // OAuth2 system context — need to read the previous API key
//nolint:gocritic // OAuth2 system context, need to read the previous API key
prevKey, err := db.GetAPIKeyByID(dbauthz.AsSystemOAuth2(ctx), dbToken.APIKeyID)
if err != nil {
return codersdk.OAuth2TokenResponse{}, err
+20 -9
View File
@@ -270,16 +270,12 @@ const (
OAuth2TokenEndpointAuthMethodNone OAuth2TokenEndpointAuthMethod = "none"
)
// AllOAuth2TokenEndpointAuthMethods returns every accepted token endpoint auth
// method. Valid() is defined in terms of it, so what registration accepts
// cannot drift from what this function reports.
// AllOAuth2TokenEndpointAuthMethods returns every token endpoint auth method
// registration accepts. Valid() is defined in terms of it, so what
// registration accepts cannot drift from what this function reports.
//
// Discovery metadata does not yet derive from it:
// coderd/oauth2provider/metadata.go's TokenEndpointAuthMethodsSupported is
// hardcoded to {client_secret_basic, client_secret_post} and does not
// advertise "none", even though "none" is accepted here. A follow-up PR
// wires the token endpoint to honor "none"; only once that lands should
// discovery advertise it too.
// Discovery does not advertise this list verbatim; see
// AdvertisedOAuth2TokenEndpointAuthMethods for why.
func AllOAuth2TokenEndpointAuthMethods() []OAuth2TokenEndpointAuthMethod {
return []OAuth2TokenEndpointAuthMethod{
OAuth2TokenEndpointAuthMethodClientSecretBasic,
@@ -288,6 +284,21 @@ func AllOAuth2TokenEndpointAuthMethods() []OAuth2TokenEndpointAuthMethod {
}
}
// AdvertisedOAuth2TokenEndpointAuthMethods returns the token endpoint auth
// methods safe to advertise in discovery metadata (RFC 8414
// token_endpoint_auth_methods_supported). It excludes "none": registration
// accepts "none" (see AllOAuth2TokenEndpointAuthMethods), but the token
// endpoint still requires a client secret for every authorization_code
// exchange, so advertising "none" would tell a conforming client the server
// accepts an exchange it will reject. Once the token endpoint honors "none",
// this should return the same set as AllOAuth2TokenEndpointAuthMethods.
func AdvertisedOAuth2TokenEndpointAuthMethods() []OAuth2TokenEndpointAuthMethod {
return []OAuth2TokenEndpointAuthMethod{
OAuth2TokenEndpointAuthMethodClientSecretBasic,
OAuth2TokenEndpointAuthMethodClientSecretPost,
}
}
func (m OAuth2TokenEndpointAuthMethod) Valid() bool {
return slices.Contains(AllOAuth2TokenEndpointAuthMethods(), m)
}
+26 -2
View File
@@ -118,10 +118,32 @@ Coder supports the following OAuth2 client authentication methods at the token e
- `client_secret_basic` (recommended): HTTP Basic authentication (RFC 6749 §2.3.1). The username is `client_id` and the password is `client_secret`.
- `client_secret_post`: Form-based authentication where `client_id` and `client_secret` are sent in the request body.
- `none`: No client secret. The client is a public client and authenticates with PKCE alone (RFC 7591 §2, OAuth 2.1 §2.1). Available only through [Dynamic Client Registration](#dynamic-client-registration), which is disabled by default, since a client's type is set when it registers and apps created through the admin UI or API are always confidential.
Coder supports both methods for compatibility; existing integrations using `client_secret_post` do not need to change.
> [!NOTE]
> Registration accepts `none` today, but the token endpoint does not yet
> honor it: an `authorization_code` exchange still requires
> `client_secret`, so a public client cannot obtain a token yet. Discovery
> omits `none` from `token_endpoint_auth_methods_supported` for the same
> reason, so a conforming client is not told to attempt an exchange that
> would be rejected.
If you use Dynamic Client Registration (RFC 7591) and omit `token_endpoint_auth_method`, clients default to `client_secret_basic`. To request `client_secret_post`, set `token_endpoint_auth_method` to `client_secret_post` in the registration request.
Coder supports both secret-based methods for compatibility; existing integrations using `client_secret_post` do not need to change.
Public clients suit native, mobile, and CLI applications that cannot keep a secret confidential. Note the redirect URI restrictions below before choosing one.
If you use Dynamic Client Registration (RFC 7591) and omit `token_endpoint_auth_method`, clients default to `client_secret_basic`. To request `client_secret_post`, set `token_endpoint_auth_method` to `client_secret_post` in the registration request. To register a public client, set it to `none`: Coder issues no `client_secret`, and the registration response omits that field entirely.
> [!IMPORTANT]
> A public client may use `http://` only with a loopback host
> (`localhost`, `127.0.0.1`, `[::1]`). An `http://` redirect URI to any
> other host is rejected, so use `https://` instead. A confidential
> client has the same restriction but also accepts `.localhost`
> subdomains over `http://`.
>
> Which schemes a redirect URI may use is a separate restriction that
> also differs by client type. See
> [Callback URL schemes](#callback-url-schemes).
If client authentication fails, the token endpoint returns **HTTP 401** with an OAuth2 `invalid_client` error and a `WWW-Authenticate: Basic realm="coder"` response header.
@@ -348,6 +370,8 @@ scheme (`myapp://callback`) or a loopback HTTP address.
Custom URI schemes (`myapp://`, `vscode://`, `jetbrains://`, etc.) are fully supported for native and desktop applications. The OS routes the redirect back to the registered application without requiring a running HTTP server.
The out-of-band URN `urn:ietf:wg:oauth:2.0:oob` is accepted from either client type, for clients that display the authorization code for the user to copy rather than receiving it on a redirect. No other URN is accepted.
The following schemes are blocked for security reasons: `javascript:`, `data:`, `file:`, `ftp:`.
Public clients (`token_endpoint_auth_method: none`) additionally cannot register `mailto:`, `tel:`, or `sms:` redirect URIs, since those schemes hand off to another app rather than returning an authorization code to the client. Confidential clients are not subject to this restriction.