diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index f64d82782672c..306507ee2070f 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -14952,6 +14952,12 @@ const docTemplate = `{ "responses": { "200": { "description": "Token successfully revoked" + }, + "413": { + "description": "Request Entity Too Large", + "schema": { + "$ref": "#/definitions/codersdk.OAuth2Error" + } } } } @@ -15012,6 +15018,12 @@ const docTemplate = `{ "schema": { "$ref": "#/definitions/oauth2.Token" } + }, + "413": { + "description": "Request Entity Too Large", + "schema": { + "$ref": "#/definitions/codersdk.OAuth2Error" + } } } }, @@ -22137,6 +22149,51 @@ const docTemplate = `{ } } }, + "codersdk.OAuth2Error": { + "type": "object", + "properties": { + "error": { + "$ref": "#/definitions/codersdk.OAuth2ErrorCode" + }, + "error_description": { + "type": "string" + }, + "error_uri": { + "type": "string" + } + } + }, + "codersdk.OAuth2ErrorCode": { + "type": "string", + "enum": [ + "invalid_request", + "invalid_client", + "invalid_grant", + "unauthorized_client", + "unsupported_grant_type", + "invalid_scope", + "access_denied", + "unsupported_response_type", + "server_error", + "temporarily_unavailable", + "unsupported_token_type", + "invalid_target" + ], + "x-enum-varnames": [ + "OAuth2ErrorCodeInvalidRequest", + "OAuth2ErrorCodeInvalidClient", + "OAuth2ErrorCodeInvalidGrant", + "OAuth2ErrorCodeUnauthorizedClient", + "OAuth2ErrorCodeUnsupportedGrantType", + "OAuth2ErrorCodeInvalidScope", + "OAuth2ErrorCodeAccessDenied", + "OAuth2ErrorCodeUnsupportedResponseType", + "OAuth2ErrorCodeServerError", + "OAuth2ErrorCodeTemporarilyUnavailable", + "OAuth2ErrorCodeUnsupportedTokenType", + "OAuth2ErrorCodeInvalidTarget" + ] + }, "codersdk.OAuth2GithubConfig": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index b98b8857a5ea1..b1845c24fc467 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -13276,6 +13276,12 @@ "responses": { "200": { "description": "Token successfully revoked" + }, + "413": { + "description": "Request Entity Too Large", + "schema": { + "$ref": "#/definitions/codersdk.OAuth2Error" + } } } } @@ -13332,6 +13338,12 @@ "schema": { "$ref": "#/definitions/oauth2.Token" } + }, + "413": { + "description": "Request Entity Too Large", + "schema": { + "$ref": "#/definitions/codersdk.OAuth2Error" + } } } }, @@ -20201,6 +20213,51 @@ } } }, + "codersdk.OAuth2Error": { + "type": "object", + "properties": { + "error": { + "$ref": "#/definitions/codersdk.OAuth2ErrorCode" + }, + "error_description": { + "type": "string" + }, + "error_uri": { + "type": "string" + } + } + }, + "codersdk.OAuth2ErrorCode": { + "type": "string", + "enum": [ + "invalid_request", + "invalid_client", + "invalid_grant", + "unauthorized_client", + "unsupported_grant_type", + "invalid_scope", + "access_denied", + "unsupported_response_type", + "server_error", + "temporarily_unavailable", + "unsupported_token_type", + "invalid_target" + ], + "x-enum-varnames": [ + "OAuth2ErrorCodeInvalidRequest", + "OAuth2ErrorCodeInvalidClient", + "OAuth2ErrorCodeInvalidGrant", + "OAuth2ErrorCodeUnauthorizedClient", + "OAuth2ErrorCodeUnsupportedGrantType", + "OAuth2ErrorCodeInvalidScope", + "OAuth2ErrorCodeAccessDenied", + "OAuth2ErrorCodeUnsupportedResponseType", + "OAuth2ErrorCodeServerError", + "OAuth2ErrorCodeTemporarilyUnavailable", + "OAuth2ErrorCodeUnsupportedTokenType", + "OAuth2ErrorCodeInvalidTarget" + ] + }, "codersdk.OAuth2GithubConfig": { "type": "object", "properties": { diff --git a/coderd/httpapi/httpapi.go b/coderd/httpapi/httpapi.go index d071ca8931989..1da178a6674f7 100644 --- a/coderd/httpapi/httpapi.go +++ b/coderd/httpapi/httpapi.go @@ -552,3 +552,17 @@ func WriteOAuth2Error(ctx context.Context, rw http.ResponseWriter, status int, e ErrorDescription: description, }) } + +// WriteOAuth2RequestTooLarge reports a request body over limit as an RFC 6749 +// error, for the OAuth2 endpoints that read a form body rather than decoding +// through Read. RFC 6749 defines no error code for a transport rejection, so +// invalid_request is the closest compliant framing. +// +// The limit is the one carried by the *http.MaxBytesError that tripped, so a +// caller of this function reports the bound that actually applied rather than +// the one it assumes applied. +func WriteOAuth2RequestTooLarge(ctx context.Context, rw http.ResponseWriter, limit int64) { + RecordRequestBodyLimit(ctx, limit) + WriteOAuth2Error(ctx, rw, http.StatusRequestEntityTooLarge, codersdk.OAuth2ErrorCodeInvalidRequest, + fmt.Sprintf("Maximum request body size is %d bytes.", limit)) +} diff --git a/coderd/httpmw/oauth2.go b/coderd/httpmw/oauth2.go index fccc89642c000..1031ba6255571 100644 --- a/coderd/httpmw/oauth2.go +++ b/coderd/httpmw/oauth2.go @@ -2,6 +2,7 @@ package httpmw import ( "context" + "errors" "fmt" "net" "net/http" @@ -374,6 +375,7 @@ type errorWriter interface { writeMissingClientID(ctx context.Context, rw http.ResponseWriter) writeInvalidClientID(ctx context.Context, rw http.ResponseWriter, err error) writeClientNotFound(ctx context.Context, rw http.ResponseWriter) + writeRequestTooLarge(ctx context.Context, rw http.ResponseWriter, limit int64) } // codersdkErrorWriter writes standard codersdk errors for general API endpoints @@ -402,6 +404,14 @@ func (*codersdkErrorWriter) writeClientNotFound(ctx context.Context, rw http.Res }) } +func (*codersdkErrorWriter) writeRequestTooLarge(ctx context.Context, rw http.ResponseWriter, limit int64) { + httpapi.RecordRequestBodyLimit(ctx, limit) + httpapi.Write(ctx, rw, http.StatusRequestEntityTooLarge, codersdk.Response{ + Message: "Request body too large.", + Detail: fmt.Sprintf("Maximum request body size is %d bytes.", limit), + }) +} + // oauth2ErrorWriter writes OAuth2-compliant errors for OAuth2 endpoints type oauth2ErrorWriter struct{} @@ -417,6 +427,10 @@ func (*oauth2ErrorWriter) writeClientNotFound(ctx context.Context, rw http.Respo httpapi.WriteOAuth2Error(ctx, rw, http.StatusUnauthorized, codersdk.OAuth2ErrorCodeInvalidClient, "The client credentials are invalid") } +func (*oauth2ErrorWriter) writeRequestTooLarge(ctx context.Context, rw http.ResponseWriter, limit int64) { + httpapi.WriteOAuth2RequestTooLarge(ctx, rw, limit) +} + // extractOAuth2ProviderAppBase is the internal implementation that uses the strategy pattern // instead of a control flag to handle different error formats. func extractOAuth2ProviderAppBase(db database.Store, errWriter errorWriter) func(http.Handler) http.Handler { @@ -424,6 +438,18 @@ func extractOAuth2ProviderAppBase(db database.Store, errWriter errorWriter) func return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() + // POST /oauth2/tokens and POST /oauth2/revoke authenticate the + // client from the request body, so they cannot require an API key + // and the body below is read before any authorization decision. + // Bound it here, ahead of every reader: r.ParseForm below, and the + // handlers downstream, all read through this wrap. + // + // net/http caps an unwrapped urlencoded body at its own 10 MiB + // maxFormSize, which is 2.5x the ceiling every other endpoint + // carries. It defers to a *http.MaxBytesReader when it finds one, + // so installing one replaces that cap with this one. + r.Body = http.MaxBytesReader(rw, r.Body, httpapi.DefaultMaxRequestBodyBytes) + // App can come from a URL param, query param, or form value. paramID := "app" var appID uuid.UUID @@ -441,9 +467,19 @@ func extractOAuth2ProviderAppBase(db database.Store, errWriter errorWriter) func paramAppID := r.URL.Query().Get("client_id") if paramAppID == "" { // Check the form params! - if r.ParseForm() == nil { + err := r.ParseForm() + if maxBytesErr, ok := errors.AsType[*http.MaxBytesError](err); ok { + // An oversized body is a size failure and is reported as + // one. Falling through would report the client_id this + // body may well have carried as missing. + errWriter.writeRequestTooLarge(ctx, rw, maxBytesErr.Limit) + return + } + if err == nil { paramAppID = r.Form.Get("client_id") } + // Any other parse failure is not fatal here: the client_id + // may still arrive through HTTP Basic below. } if paramAppID == "" { // RFC 6749 §2.3.1: confidential clients may authenticate via diff --git a/coderd/httpmw/oauth2_test.go b/coderd/httpmw/oauth2_test.go index 6638194ab3acc..31653a558ff5d 100644 --- a/coderd/httpmw/oauth2_test.go +++ b/coderd/httpmw/oauth2_test.go @@ -2,15 +2,21 @@ package httpmw_test import ( "context" + "encoding/json" "net/http" "net/http/httptest" "net/url" + "strconv" + "strings" "testing" + "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/oauth2" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" @@ -369,3 +375,46 @@ func (p *exchangeAssertingProvider) Exchange(_ context.Context, _ string, opts . func (*exchangeAssertingProvider) TokenSource(_ context.Context, _ *oauth2.Token) oauth2.TokenSource { return nil } + +// TestExtractOAuth2ProviderAppBodyTooLarge covers the pre-authentication body +// read on POST /oauth2/tokens and POST /oauth2/revoke. Those routes authenticate +// the client from the request body, so they carry no API key middleware and the +// form parse below runs before any authorization decision. +// +// net/http would otherwise cap the body at its own 10 MiB maxFormSize, which is +// larger than the ceiling every other endpoint carries. The rejection is +// asserted in the RFC 6749 error shape because a client that parses that shape +// cannot read a codersdk.Response, so the status alone is not sufficient. +func TestExtractOAuth2ProviderAppBodyTooLarge(t *testing.T) { + t.Parallel() + + db, _ := dbtestutil.NewDB(t) + + // Padding a valid form past the limit leaves size as the only reason to + // reject the request. client_id is present in the body, so a fallthrough to + // the missing-client_id path would be observable as a 400. + oversized := "client_id=" + uuid.NewString() + "&pad=" + + strings.Repeat("a", httpapi.DefaultMaxRequestBodyBytes) + require.Greater(t, len(oversized), httpapi.DefaultMaxRequestBodyBytes) + + var nextCalled bool + handler := httpmw.ExtractOAuth2ProviderAppWithOAuth2Errors(db)( + http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + nextCalled = true + }), + ) + + r := httptest.NewRequest(http.MethodPost, "/oauth2/tokens", strings.NewReader(oversized)) + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rw := httptest.NewRecorder() + + handler.ServeHTTP(rw, r) + + require.Equal(t, http.StatusRequestEntityTooLarge, rw.Code) + require.False(t, nextCalled, "handler ran on an oversized body") + + var errResp codersdk.OAuth2Error + require.NoError(t, json.Unmarshal(rw.Body.Bytes(), &errResp)) + require.Equal(t, codersdk.OAuth2ErrorCodeInvalidRequest, errResp.Error) + require.Contains(t, errResp.ErrorDescription, strconv.Itoa(httpapi.DefaultMaxRequestBodyBytes)) +} diff --git a/coderd/oauth2.go b/coderd/oauth2.go index 2e083eeca63f3..73595f19a2606 100644 --- a/coderd/oauth2.go +++ b/coderd/oauth2.go @@ -152,6 +152,7 @@ func (api *API) postOAuth2ProviderAppAuthorize() http.HandlerFunc { // @Param refresh_token formData string false "Refresh token, required if grant_type=refresh_token" // @Param grant_type formData codersdk.OAuth2ProviderGrantType true "Grant type" // @Success 200 {object} oauth2.Token +// @Failure 413 {object} codersdk.OAuth2Error // @Router /oauth2/tokens [post] func (api *API) postOAuth2ProviderAppToken() http.HandlerFunc { return oauth2provider.Tokens(api.Database, api.DeploymentValues.Sessions) @@ -176,6 +177,7 @@ func (api *API) deleteOAuth2ProviderAppTokens() http.HandlerFunc { // @Param token formData string true "The token to revoke" // @Param token_type_hint formData string false "Hint about token type (access_token or refresh_token)" // @Success 200 "Token successfully revoked" +// @Failure 413 {object} codersdk.OAuth2Error // @Router /oauth2/revoke [post] func (api *API) revokeOAuth2Token() http.HandlerFunc { return oauth2provider.RevokeToken(api.Database, api.Logger) diff --git a/coderd/oauth2_error_compliance_test.go b/coderd/oauth2_error_compliance_test.go index 5900e9ee1e80d..6189cdf507b9e 100644 --- a/coderd/oauth2_error_compliance_test.go +++ b/coderd/oauth2_error_compliance_test.go @@ -1,8 +1,10 @@ package coderd_test import ( + "encoding/json" "fmt" "net/http" + "strings" "testing" "time" @@ -384,12 +386,49 @@ func TestOAuth2SpecificErrorScenarios(t *testing.T) { // Error properly returned with bad request status }) + // A malformed body is rejected before validation, so it is the one path on + // this handler that does not go through req.Validate. It still owes the + // caller an RFC 7591 error rather than a codersdk.Response, which is the + // reason the decode is local to the handler rather than httpapi.Read. The + // body is sent raw because a typed request cannot express invalid JSON. t.Run("InvalidJSONStructure", func(t *testing.T) { t.Parallel() - // For invalid JSON structure, we'd need to make raw HTTP requests - // This is tested implicitly through the other tests since we're using - // typed requests that ensure proper JSON structure + for _, tc := range []struct { + name string + body string + // The decoder's own text, which the response must carry so a client + // integrator can tell an unterminated body from a mistyped field. + detail string + }{ + { + name: "UnterminatedObject", + body: `{"client_name": "test"`, + detail: "unexpected EOF", + }, + { + name: "WrongFieldType", + body: `{"redirect_uris": "https://example.com/callback"}`, + detail: "redirect_uris", + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitLong) + + res, err := client.Request(ctx, http.MethodPost, "/oauth2/register", + strings.NewReader(tc.body)) + require.NoError(t, err) + defer res.Body.Close() + + require.Equal(t, http.StatusBadRequest, res.StatusCode) + + var errResp OAuth2ErrorResponse + require.NoError(t, json.NewDecoder(res.Body).Decode(&errResp)) + require.Equal(t, "invalid_request", errResp.Error) + require.Contains(t, errResp.ErrorDescription, tc.detail) + }) + } }) t.Run("UnsupportedFields", func(t *testing.T) { diff --git a/coderd/oauth2provider/formlimit_test.go b/coderd/oauth2provider/formlimit_test.go new file mode 100644 index 0000000000000..9095454fe8ee8 --- /dev/null +++ b/coderd/oauth2provider/formlimit_test.go @@ -0,0 +1,88 @@ +package oauth2provider_test + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "cdr.dev/slog/v3/sloggers/slogtest" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/httpapi" + "github.com/coder/coder/v2/coderd/httpmw" + "github.com/coder/coder/v2/coderd/oauth2provider" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/testutil" +) + +// TestFormEndpointsBodyTooLarge covers the form-parsing OAuth2 endpoints when +// client_id arrives in the query string. +// +// ExtractOAuth2ProviderAppWithOAuth2Errors installs the body bound, but it +// parses the form only when client_id is absent from the query. When client_id +// is present the middleware resolves the app without touching the body, so the +// handler's own r.ParseForm is the first read and the bound trips there. These +// endpoints authenticate the client from the body and therefore carry no API key +// middleware, so this read is still pre-authentication. +func TestFormEndpointsBodyTooLarge(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + path string + handler func(database.Store) http.Handler + }{ + { + name: "Tokens", + path: "/oauth2/tokens", + handler: func(db database.Store) http.Handler { + return oauth2provider.Tokens(db, codersdk.SessionLifetime{}) + }, + }, + { + name: "Revoke", + path: "/oauth2/revoke", + handler: func(db database.Store) http.Handler { + return oauth2provider.RevokeToken(db, slogtest.Make(t, nil)) + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitLong) + + db, _ := dbtestutil.NewDB(t) + app := dbgen.OAuth2ProviderApp(t, db, database.OAuth2ProviderApp{}) + + handler := httpmw.ExtractOAuth2ProviderAppWithOAuth2Errors(db)(tc.handler(db)) + + // Padded past the limit so size is the only reason to reject. The + // form itself is well formed, so a rejection for any other reason + // would carry a different error code. + body := "grant_type=authorization_code&token=abc&pad=" + + strings.Repeat("a", httpapi.DefaultMaxRequestBodyBytes) + require.Greater(t, len(body), httpapi.DefaultMaxRequestBodyBytes) + + r := httptest.NewRequest(http.MethodPost, tc.path+"?client_id="+app.ID.String(), + strings.NewReader(body)).WithContext(ctx) + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rw := httptest.NewRecorder() + + handler.ServeHTTP(rw, r) + + require.Equal(t, http.StatusRequestEntityTooLarge, rw.Code, + "response body: %s", rw.Body.String()) + + var errResp codersdk.OAuth2Error + require.NoError(t, json.Unmarshal(rw.Body.Bytes(), &errResp)) + require.Equal(t, codersdk.OAuth2ErrorCodeInvalidRequest, errResp.Error) + require.Contains(t, errResp.ErrorDescription, strconv.Itoa(httpapi.DefaultMaxRequestBodyBytes)) + }) + } +} diff --git a/coderd/oauth2provider/registration.go b/coderd/oauth2provider/registration.go index 2261a5c5b51a2..9dd874054ef68 100644 --- a/coderd/oauth2provider/registration.go +++ b/coderd/oauth2provider/registration.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "encoding/json" + "errors" "fmt" "net/http" "net/url" @@ -57,8 +58,8 @@ func CreateDynamicClientRegistration(db database.Store, accessURL *url.URL, audi defer commitAudit() // Parse request - var req codersdk.OAuth2ClientRegistrationRequest - if !httpapi.Read(ctx, rw, r, &req) { + req, ok := readOAuth2ClientRegistrationRequest(ctx, rw, r) + if !ok { return } @@ -272,8 +273,8 @@ func UpdateClientConfiguration(db database.Store, auditor *audit.Auditor, logger } // Parse request - var req codersdk.OAuth2ClientRegistrationRequest - if !httpapi.Read(ctx, rw, r, &req) { + req, ok := readOAuth2ClientRegistrationRequest(ctx, rw, r) + if !ok { return } @@ -531,6 +532,39 @@ func generateRegistrationAccessToken() (plaintext string, hashed []byte, err err return apikey.GenerateSecret(secretLength) } +// readOAuth2ClientRegistrationRequest decodes a client registration request, +// bounded by httpapi.DefaultMaxRequestBodyBytes, and reports failures as RFC +// 7591 errors. +// +// It exists instead of httpapi.Read because these handlers route every other +// error through writeOAuth2RegistrationError, and httpapi.Read reports a +// codersdk.Response. Since http.MaxBytesReader surfaces the limit through the +// decoder's error, the error shape belongs to whoever decodes, so the decode +// happens here. RFC 7591 section 3.2.2 defines invalid_client_metadata and +// friends for semantic validation rather than transport rejection, so +// invalid_request is the closest compliant framing for both cases below. +func readOAuth2ClientRegistrationRequest(ctx context.Context, rw http.ResponseWriter, r *http.Request) (codersdk.OAuth2ClientRegistrationRequest, bool) { + var req codersdk.OAuth2ClientRegistrationRequest + + r.Body = http.MaxBytesReader(rw, r.Body, httpapi.DefaultMaxRequestBodyBytes) + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + if _, ok := errors.AsType[*http.MaxBytesError](err); ok { + httpapi.RecordRequestBodyLimit(ctx, httpapi.DefaultMaxRequestBodyBytes) + writeOAuth2RegistrationError(ctx, rw, http.StatusRequestEntityTooLarge, "invalid_request", + fmt.Sprintf("Maximum request body size is %d bytes.", httpapi.DefaultMaxRequestBodyBytes)) + return req, false + } + // The decoder's own text is the diagnosis a client integrator needs: it + // names the offending field and the type it expected. It describes the + // caller's own bytes, so it discloses nothing, and httpapi.Read has + // exposed it on every other endpoint for as long as it has existed. + writeOAuth2RegistrationError(ctx, rw, http.StatusBadRequest, "invalid_request", + fmt.Sprintf("Request body must be valid JSON: %s", err)) + return req, false + } + return req, true +} + // writeOAuth2RegistrationError writes RFC 7591 compliant error responses func writeOAuth2RegistrationError(_ context.Context, rw http.ResponseWriter, status int, errorCode, description string) { // RFC 7591 error response format diff --git a/coderd/oauth2provider/registration_test.go b/coderd/oauth2provider/registration_test.go index f23e82dbf76f9..c6284075e813e 100644 --- a/coderd/oauth2provider/registration_test.go +++ b/coderd/oauth2provider/registration_test.go @@ -6,6 +6,8 @@ import ( "net/http" "net/http/httptest" "net/url" + "strconv" + "strings" "testing" "github.com/stretchr/testify/require" @@ -13,6 +15,7 @@ import ( "cdr.dev/slog/v3/sloggers/slogtest" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/oauth2provider" "github.com/coder/coder/v2/coderd/tracing" "github.com/coder/coder/v2/coderd/util/ptr" @@ -97,3 +100,46 @@ func TestCreateDynamicClientRegistration_DCREnabled(t *testing.T) { }) } } + +// TestCreateDynamicClientRegistration_BodyTooLarge asserts that an oversized +// body is rejected with an RFC 7591 error body rather than a codersdk.Response. +// Every other error in this handler is protocol-shaped, and a client that parses +// the OAuth2 error shape would fail to read a codersdk.Response, so the status +// alone is not sufficient. +func TestCreateDynamicClientRegistration_BodyTooLarge(t *testing.T) { + t.Parallel() + + accessURL, err := url.Parse("https://oauth2-registration-too-large-test.example.com") + require.NoError(t, err) + + 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)) + + // One valid redirect URI padded past the limit, so size is the only reason + // to reject the request. + req := codersdk.OAuth2ClientRegistrationRequest{ + RedirectURIs: []string{"https://example.com/callback"}, + ClientName: strings.Repeat("a", httpapi.DefaultMaxRequestBodyBytes), + } + body, err := json.Marshal(req) + require.NoError(t, err) + require.Greater(t, len(body), httpapi.DefaultMaxRequestBodyBytes) + + 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.StatusRequestEntityTooLarge, rw.Code) + + var errResp map[string]string + require.NoError(t, json.Unmarshal(rw.Body.Bytes(), &errResp)) + require.Equal(t, "invalid_request", errResp["error"]) + require.Contains(t, errResp["error_description"], strconv.Itoa(httpapi.DefaultMaxRequestBodyBytes)) +} diff --git a/coderd/oauth2provider/revoke.go b/coderd/oauth2provider/revoke.go index bba2757775d56..844152e36f1d4 100644 --- a/coderd/oauth2provider/revoke.go +++ b/coderd/oauth2provider/revoke.go @@ -68,6 +68,14 @@ func RevokeToken(db database.Store, logger slog.Logger) http.HandlerFunc { req, err := extractRevocationRequest(r) if err != nil { + // ExtractOAuth2ProviderAppWithOAuth2Errors bounds the body, but it + // parses the form only when client_id is absent from the query + // string. When it is present, extractRevocationRequest performs the + // first read and the bound trips here rather than in the middleware. + if maxBytesErr, ok := errors.AsType[*http.MaxBytesError](err); ok { + httpapi.WriteOAuth2RequestTooLarge(ctx, rw, maxBytesErr.Limit) + return + } httpapi.WriteOAuth2Error(ctx, rw, http.StatusBadRequest, codersdk.OAuth2ErrorCodeInvalidRequest, err.Error()) return } diff --git a/coderd/oauth2provider/tokens.go b/coderd/oauth2provider/tokens.go index 1beedf8cc3535..b53cc7d973194 100644 --- a/coderd/oauth2provider/tokens.go +++ b/coderd/oauth2provider/tokens.go @@ -149,6 +149,14 @@ func Tokens(db database.Store, lifetimes codersdk.SessionLifetime) http.HandlerF req, validationErrs, err := extractTokenRequest(r, callbackURL) if err != nil { + // ExtractOAuth2ProviderAppWithOAuth2Errors bounds the body, but it + // parses the form only when client_id is absent from the query + // string. When it is present, extractTokenRequest performs the first + // read and the bound trips here rather than in the middleware. + if maxBytesErr, ok := errors.AsType[*http.MaxBytesError](err); ok { + httpapi.WriteOAuth2RequestTooLarge(ctx, rw, maxBytesErr.Limit) + return + } if errors.Is(err, errConflictingClientAuth) { httpapi.WriteOAuth2Error(ctx, rw, http.StatusBadRequest, codersdk.OAuth2ErrorCodeInvalidRequest, "Conflicting client credentials between Authorization header and request body") return diff --git a/docs/reference/api/enterprise.md b/docs/reference/api/enterprise.md index a6c7114b16252..6340120bbd7c4 100644 --- a/docs/reference/api/enterprise.md +++ b/docs/reference/api/enterprise.md @@ -5118,7 +5118,7 @@ curl -X POST http://coder-server:8080/oauth2/register \ ```sh # Example request using curl curl -X POST http://coder-server:8080/oauth2/revoke \ - + -H 'Accept: */*' ``` `POST /oauth2/revoke` @@ -5141,11 +5141,16 @@ token_type_hint: string | `» token` | body | string | true | The token to revoke | | `» token_type_hint` | body | string | false | Hint about token type (access_token or refresh_token) | +### Example responses + +> 413 Response + ### Responses -| Status | Meaning | Description | Schema | -|--------|---------------------------------------------------------|----------------------------|--------| -| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | Token successfully revoked | | +| Status | Meaning | Description | Schema | +|--------|-------------------------------------------------------------------------|----------------------------|--------------------------------------------------------| +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | Token successfully revoked | | +| 413 | [Payload Too Large](https://tools.ietf.org/html/rfc7231#section-6.5.11) | Request Entity Too Large | [codersdk.OAuth2Error](schemas.md#codersdkoauth2error) | ## OAuth2 token exchange @@ -5203,9 +5208,10 @@ grant_type: authorization_code ### Responses -| Status | Meaning | Description | Schema | -|--------|---------------------------------------------------------|-------------|----------------------------------------| -| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [oauth2.Token](schemas.md#oauth2token) | +| Status | Meaning | Description | Schema | +|--------|-------------------------------------------------------------------------|--------------------------|--------------------------------------------------------| +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [oauth2.Token](schemas.md#oauth2token) | +| 413 | [Payload Too Large](https://tools.ietf.org/html/rfc7231#section-6.5.11) | Request Entity Too Large | [codersdk.OAuth2Error](schemas.md#codersdkoauth2error) | ## Delete OAuth2 application tokens diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 527d79f185ebc..761c6a42653d8 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -9269,6 +9269,38 @@ Only certain features set these fields: - FeatureManagedAgentLimit - FeatureAgen |----------|------------------------------------------------------------|----------|--------------|-------------| | `github` | [codersdk.OAuth2GithubConfig](#codersdkoauth2githubconfig) | false | | | +## codersdk.OAuth2Error + +```json +{ + "error": "invalid_request", + "error_description": "string", + "error_uri": "string" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +|---------------------|------------------------------------------------------|----------|--------------|-------------| +| `error` | [codersdk.OAuth2ErrorCode](#codersdkoauth2errorcode) | false | | | +| `error_description` | string | false | | | +| `error_uri` | string | false | | | + +## codersdk.OAuth2ErrorCode + +```json +"invalid_request" +``` + +### Properties + +#### Enumerated Values + +| Value(s) | +|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `access_denied`, `invalid_client`, `invalid_grant`, `invalid_request`, `invalid_scope`, `invalid_target`, `server_error`, `temporarily_unavailable`, `unauthorized_client`, `unsupported_grant_type`, `unsupported_response_type`, `unsupported_token_type` | + ## codersdk.OAuth2GithubConfig ```json