From 716295015b12f142018284a7e37a82c2a24fdb4b Mon Sep 17 00:00:00 2001 From: Giteabot Date: Wed, 21 Oct 2026 23:22:15 -0700 Subject: [PATCH] fix(oauth2): decode form-encoded client credentials in HTTP Basic auth (#39926) (#39931) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backport #39926 by @fixfon Fixes https://github.com/go-gitea/gitea/issues/39925 RFC 6749 ยง2.3.1 has clients form-urlencode the client id and secret before building the HTTP Basic header. `openid-client` with `client_secret_basic` does this and sends `-` as `%2D` and `_` as `%5F`. The token and introspection endpoints used the raw header values, so these clients failed with `invalid_client`. Both endpoints now decode the values with `url.QueryUnescape` before the client lookup, the secret check and the body consistency check, and reject a value that does not decode. Plain HTTP Basic parsing in `httpauth` is unchanged, since the encoding only applies to OAuth client authentication. Clients that don't encode are unaffected, since no client id or secret Gitea has ever issued contains `+` or `%`. Co-authored-by: fixfon <12512757+fixfon@users.noreply.github.com> Co-authored-by: silverwind --- routers/web/auth/oauth2_provider.go | 20 +++++-- routers/web/auth/oauth2_provider_test.go | 71 ++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 6 deletions(-) create mode 100644 routers/web/auth/oauth2_provider_test.go diff --git a/routers/web/auth/oauth2_provider.go b/routers/web/auth/oauth2_provider.go index 5810924e5ac..cf768bdebd5 100644 --- a/routers/web/auth/oauth2_provider.go +++ b/routers/web/auth/oauth2_provider.go @@ -132,12 +132,21 @@ func InfoOAuth(ctx *context.Context) { ctx.JSON(http.StatusOK, response) } +// RFC 6749 section 2.3.1: client_secret_basic credentials are form-urlencoded +func parseClientSecretBasic(authHeader string) (clientID, clientSecret string, ok bool) { + parsed, _ := httpauth.ParseAuthorizationHeader(authHeader) + if parsed.BasicAuth == nil { + return "", "", false + } + clientID, errID := url.QueryUnescape(parsed.BasicAuth.Username) + clientSecret, errSecret := url.QueryUnescape(parsed.BasicAuth.Password) + return clientID, clientSecret, errID == nil && errSecret == nil +} + // IntrospectOAuth introspects an oauth token func IntrospectOAuth(ctx *context.Context) { var introspectingApp *auth.OAuth2Application - authHeader := ctx.Req.Header.Get("Authorization") - if parsed, ok := httpauth.ParseAuthorizationHeader(authHeader); ok && parsed.BasicAuth != nil { - clientID, clientSecret := parsed.BasicAuth.Username, parsed.BasicAuth.Password + if clientID, clientSecret, ok := parseClientSecretBasic(ctx.Req.Header.Get("Authorization")); ok { app, err := auth.GetOAuth2ApplicationByClientID(ctx, clientID) if err != nil && !auth.IsErrOauthClientIDInvalid(err) { // this is likely a database error; log it and respond without details @@ -495,15 +504,14 @@ func AccessTokenOAuth(ctx *context.Context) { // if there is no ClientID or ClientSecret in the request body, fill these fields by the Authorization header and ensure the provided field matches the Authorization header if form.ClientID == "" || form.ClientSecret == "" { if authHeader := ctx.Req.Header.Get("Authorization"); authHeader != "" { - parsed, ok := httpauth.ParseAuthorizationHeader(authHeader) - if !ok || parsed.BasicAuth == nil { + clientID, clientSecret, ok := parseClientSecretBasic(authHeader) + if !ok { handleAccessTokenError(ctx, oauth2_provider.AccessTokenError{ ErrorCode: oauth2_provider.AccessTokenErrorCodeInvalidRequest, ErrorDescription: "cannot parse basic auth header", }) return } - clientID, clientSecret := parsed.BasicAuth.Username, parsed.BasicAuth.Password // validate that any fields present in the form match the Basic auth header if form.ClientID != "" && form.ClientID != clientID { handleAccessTokenError(ctx, oauth2_provider.AccessTokenError{ diff --git a/routers/web/auth/oauth2_provider_test.go b/routers/web/auth/oauth2_provider_test.go new file mode 100644 index 00000000000..aa3a0ae898d --- /dev/null +++ b/routers/web/auth/oauth2_provider_test.go @@ -0,0 +1,71 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package auth + +import ( + "encoding/base64" + "net/http" + "strings" + "testing" + + "gitea.dev/models/auth" + "gitea.dev/models/unittest" + "gitea.dev/modules/json" + "gitea.dev/modules/test" + "gitea.dev/modules/web" + "gitea.dev/services/contexttest" + "gitea.dev/services/forms" + "gitea.dev/services/oauth2_provider" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestOAuth2FormEncodedBasicAuth(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + signingKey, err := oauth2_provider.CreateJWTSigningKey("HS256", make([]byte, 32)) + require.NoError(t, err) + defer test.MockVariableValue(&oauth2_provider.DefaultSigningKey, signingKey)() + + app := unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Application{ID: 1}) + const fixtureClientSecretPlaintext = "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=" + escape := strings.NewReplacer("-", "%2D", "=", "%3D").Replace + + t.Run("AccessToken accepts client matching body client_id and rejects unknown code", func(t *testing.T) { + ctx, resp := contexttest.MockContext(t, "POST /login/oauth/access_token") + ctx.Req.SetBasicAuth(escape(app.ClientID), escape(fixtureClientSecretPlaintext)) + web.SetForm(ctx, &forms.AccessTokenForm{GrantType: "authorization_code", ClientID: app.ClientID}) + AccessTokenOAuth(ctx) + + var accessTokenErr oauth2_provider.AccessTokenError + require.NoError(t, json.Unmarshal(resp.Body.Bytes(), &accessTokenErr)) + assert.Equal(t, "client is not authorized", accessTokenErr.ErrorDescription) + }) + + t.Run("Introspect accepts client and reports inactive token", func(t *testing.T) { + ctx, resp := contexttest.MockContext(t, "POST /login/oauth/introspect") + ctx.Req.SetBasicAuth(escape(app.ClientID), escape(fixtureClientSecretPlaintext)) + web.SetForm(ctx, &forms.IntrospectTokenForm{Token: "invalid"}) + IntrospectOAuth(ctx) + assert.Equal(t, http.StatusOK, resp.Code) + }) +} + +func TestParseClientSecretBasic(t *testing.T) { + basicAuth := func(username, password string) string { + return "Basic " + base64.StdEncoding.EncodeToString([]byte(username+":"+password)) + } + + clientID, clientSecret, ok := parseClientSecretBasic(basicAuth("a+b", "c%2Bd")) + assert.True(t, ok) + assert.Equal(t, "a b", clientID) + assert.Equal(t, "c+d", clientSecret) + + _, _, ok = parseClientSecretBasic(basicAuth("a%zz", "c")) + assert.False(t, ok) + _, _, ok = parseClientSecretBasic(basicAuth("a", "c%zz")) + assert.False(t, ok) + _, _, ok = parseClientSecretBasic("Bearer token") + assert.False(t, ok) +}