Files
coder/coderd/oauth2provider/apps.go
T
Bobby HoandClaude Opus 5 16c58770f8 feat: constrain the OAuth2 client type column (#27931)
Extracted from #27873 so the schema change can be reviewed for migration
safety on its own. #27873 will rebase onto this.

`client_type` decides whether the token endpoint validates a client
secret at all, and the column accepts any text: nullable, no `CHECK`, no
enum. No Go path can write a bad value today, and `IsPublic` fails
closed on anything unrecognized, so the read side is safe. What the
schema still permits is the problem: a future migration writing
`'public'` onto a row that holds a secret turns off client
authentication for that app with nothing to catch it, no constraint, no
log, no audit entry, no test.

`000565` adds `CHECK (client_type IN ('confidential', 'public'))` and
`NOT NULL`. The `UPDATE` ahead of it should touch zero rows, since
migration `000344` added the column with a default of `'confidential'`
and backfilled with `COALESCE`; it is there so `SET NOT NULL` cannot
fail on an unexpected row. Both `ALTER`s take `ACCESS EXCLUSIVE` and
scan a table holding one row per registered OAuth2 client, so the lock
is brief.

## The second migration, and why it aligns the way it does

Two columns describe the same fact and can currently contradict each
other.

`token_endpoint_auth_method` is the client's own declaration: registered
client metadata under RFC 7591 §2, where `"none"` is defined to mean the
client is public and has no secret. `client_type` is Coder's derived
copy, and it is what the token endpoint enforces on. RFC 7591 defines no
`client_type` metadata field; the column exists only as a
denormalization.

Registration used to persist the declaration verbatim while hardcoding
`client_type` to `'confidential'`, so rows exist declaring `"none"` on a
client stored confidential that was issued, and still requires, a real
secret. A client that reads its own metadata and believes it is public
will drop that secret and stop being able to exchange codes.

`000566` aligns the declaration to what is enforced, not the reverse.
Deriving enforcement from the declaration would reclassify every such
client as public and stop requiring the secret it holds, which is a
silent authentication downgrade. The down migration is deliberately
empty: the previous values are not recorded, and restoring them would
only reinstate metadata that tells a client to authenticate in a way the
server rejects.

## Application changes

`SET NOT NULL` changes the generated field from `sql.NullString` to
`string`, so the three write sites are updated to match. That is the
entire application diff and no behavior depends on it.

Refs
https://linear.app/codercom/issue/ENG-3029/oauth2-support-public-client

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-10 10:49:02 -07:00

208 lines
7.6 KiB
Go

package oauth2provider
import (
"database/sql"
"fmt"
"net/http"
"net/url"
"github.com/google/uuid"
"github.com/sqlc-dev/pqtype"
"cdr.dev/slog/v3"
"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/db2sdk"
"github.com/coder/coder/v2/coderd/database/dbtime"
"github.com/coder/coder/v2/coderd/httpapi"
"github.com/coder/coder/v2/coderd/httpmw"
"github.com/coder/coder/v2/codersdk"
)
// ListApps returns an http.HandlerFunc that handles GET /oauth2-provider/apps
func ListApps(db database.Store, accessURL *url.URL) http.HandlerFunc {
return func(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
rawUserID := r.URL.Query().Get("user_id")
if rawUserID == "" {
dbApps, err := db.GetOAuth2ProviderApps(ctx)
if err != nil {
httpapi.InternalServerError(rw, err)
return
}
httpapi.Write(ctx, rw, http.StatusOK, db2sdk.OAuth2ProviderApps(accessURL, dbApps))
return
}
userID, err := uuid.Parse(rawUserID)
if err != nil {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Invalid user UUID",
Detail: fmt.Sprintf("queried user_id=%q", userID),
})
return
}
userApps, err := db.GetOAuth2ProviderAppsByUserID(ctx, userID)
if err != nil {
httpapi.InternalServerError(rw, err)
return
}
sdkApps := make([]codersdk.OAuth2ProviderApp, 0, len(userApps))
for _, app := range userApps {
sdkApps = append(sdkApps, db2sdk.OAuth2ProviderApp(accessURL, app.OAuth2ProviderApp))
}
httpapi.Write(ctx, rw, http.StatusOK, sdkApps)
}
}
// GetApp returns an http.HandlerFunc that handles GET /oauth2-provider/apps/{app}
func GetApp(accessURL *url.URL) http.HandlerFunc {
return func(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
app := httpmw.OAuth2ProviderApp(r)
httpapi.Write(ctx, rw, http.StatusOK, db2sdk.OAuth2ProviderApp(accessURL, app))
}
}
// CreateApp returns an http.HandlerFunc that handles POST /oauth2-provider/apps
func CreateApp(db database.Store, accessURL *url.URL, auditor *audit.Auditor, logger slog.Logger) http.HandlerFunc {
return func(rw http.ResponseWriter, r *http.Request) {
var (
ctx = r.Context()
aReq, commitAudit = audit.InitRequest[database.OAuth2ProviderApp](rw, &audit.RequestParams{
Audit: *auditor,
Log: logger,
Request: r,
Action: database.AuditActionCreate,
})
)
defer commitAudit()
var req codersdk.PostOAuth2ProviderAppRequest
if !httpapi.Read(ctx, rw, r, &req) {
return
}
app, err := db.InsertOAuth2ProviderApp(ctx, database.InsertOAuth2ProviderAppParams{
ID: uuid.New(),
CreatedAt: dbtime.Now(),
UpdatedAt: dbtime.Now(),
Name: req.Name,
Icon: req.Icon,
CallbackURL: req.CallbackURL,
RedirectUris: []string{},
ClientType: "confidential",
DynamicallyRegistered: sql.NullBool{Bool: false, Valid: true},
ClientIDIssuedAt: sql.NullTime{},
ClientSecretExpiresAt: sql.NullTime{},
GrantTypes: []string{"authorization_code", "refresh_token"},
ResponseTypes: []string{"code"},
TokenEndpointAuthMethod: sql.NullString{String: "client_secret_post", Valid: true},
Scope: sql.NullString{},
Contacts: []string{},
ClientUri: sql.NullString{},
LogoUri: sql.NullString{},
TosUri: sql.NullString{},
PolicyUri: sql.NullString{},
JwksUri: sql.NullString{},
Jwks: pqtype.NullRawMessage{},
SoftwareID: sql.NullString{},
SoftwareVersion: sql.NullString{},
RegistrationAccessToken: nil,
RegistrationClientUri: sql.NullString{},
})
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error creating OAuth2 application.",
Detail: err.Error(),
})
return
}
aReq.New = app
httpapi.Write(ctx, rw, http.StatusCreated, db2sdk.OAuth2ProviderApp(accessURL, app))
}
}
// UpdateApp returns an http.HandlerFunc that handles PUT /oauth2-provider/apps/{app}
func UpdateApp(db database.Store, accessURL *url.URL, auditor *audit.Auditor, logger slog.Logger) http.HandlerFunc {
return func(rw http.ResponseWriter, r *http.Request) {
var (
ctx = r.Context()
app = httpmw.OAuth2ProviderApp(r)
aReq, commitAudit = audit.InitRequest[database.OAuth2ProviderApp](rw, &audit.RequestParams{
Audit: *auditor,
Log: logger,
Request: r,
Action: database.AuditActionWrite,
})
)
aReq.Old = app
defer commitAudit()
var req codersdk.PutOAuth2ProviderAppRequest
if !httpapi.Read(ctx, rw, r, &req) {
return
}
app, err := db.UpdateOAuth2ProviderAppByID(ctx, database.UpdateOAuth2ProviderAppByIDParams{
ID: app.ID,
UpdatedAt: dbtime.Now(),
Name: req.Name,
Icon: req.Icon,
CallbackURL: req.CallbackURL,
RedirectUris: app.RedirectUris, // Keep existing value
ClientType: app.ClientType, // Keep existing value
DynamicallyRegistered: app.DynamicallyRegistered, // Keep existing value
ClientSecretExpiresAt: app.ClientSecretExpiresAt, // Keep existing value
GrantTypes: app.GrantTypes, // Keep existing value
ResponseTypes: app.ResponseTypes, // Keep existing value
TokenEndpointAuthMethod: app.TokenEndpointAuthMethod, // Keep existing value
Scope: app.Scope, // Keep existing value
Contacts: app.Contacts, // Keep existing value
ClientUri: app.ClientUri, // Keep existing value
LogoUri: app.LogoUri, // Keep existing value
TosUri: app.TosUri, // Keep existing value
PolicyUri: app.PolicyUri, // Keep existing value
JwksUri: app.JwksUri, // Keep existing value
Jwks: app.Jwks, // Keep existing value
SoftwareID: app.SoftwareID, // Keep existing value
SoftwareVersion: app.SoftwareVersion, // Keep existing value
})
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error updating OAuth2 application.",
Detail: err.Error(),
})
return
}
aReq.New = app
httpapi.Write(ctx, rw, http.StatusOK, db2sdk.OAuth2ProviderApp(accessURL, app))
}
}
// DeleteApp returns an http.HandlerFunc that handles DELETE /oauth2-provider/apps/{app}
func DeleteApp(db database.Store, auditor *audit.Auditor, logger slog.Logger) http.HandlerFunc {
return func(rw http.ResponseWriter, r *http.Request) {
var (
ctx = r.Context()
app = httpmw.OAuth2ProviderApp(r)
aReq, commitAudit = audit.InitRequest[database.OAuth2ProviderApp](rw, &audit.RequestParams{
Audit: *auditor,
Log: logger,
Request: r,
Action: database.AuditActionDelete,
})
)
aReq.Old = app
defer commitAudit()
err := db.DeleteOAuth2ProviderAppByID(ctx, app.ID)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error deleting OAuth2 application.",
Detail: err.Error(),
})
return
}
rw.WriteHeader(http.StatusNoContent)
}
}