mirror of
https://github.com/go-gitea/gitea.git
synced 2026-10-23 21:04:55 +02:00
fix(oauth2): decode form-encoded client credentials in HTTP Basic auth (#39926)
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: silverwind <me@silverwind.io>
This commit is contained in:
1 parent
cf7050b91d
commit
aaeb502b30
2 files changed
+85
-6
No files matched your search
@@ -131,12 +131,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
|
||||
@@ -499,15 +508,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{
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
Reference in new issue
Block a user