fix OIDC introspection authentication (#31632)

See discussion on #31561 for some background.

The introspect endpoint was using the OIDC token itself for
authentication. This fixes it to use basic authentication with the
client ID and secret instead:

* Applications with a valid client ID and secret should be able to
  successfully introspect an invalid token, receiving a 200 response
  with JSON data that indicates the token is invalid
* Requests with an invalid client ID and secret should not be able
  to introspect, even if the token itself is valid

Unlike #31561 (which just future-proofed the current behavior against
future changes to `DISABLE_QUERY_AUTH_TOKEN`), this is a potential
compatibility break (some introspection requests without valid client
IDs that would previously succeed will now fail). Affected deployments
must begin sending a valid HTTP basic authentication header with their
introspection requests, with the username set to a valid client ID and
the password set to the corresponding client secret.
This commit is contained in:
Shivaram Lingamneni 2024-07-23 14:43:03 +02:00 committed by GitHub
parent 24f9390f34
commit 2f1cb1d289
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 90 additions and 24 deletions

View File

@ -48,13 +48,10 @@ func BasicAuthDecode(encoded string) (string, string, error) {
return "", "", err return "", "", err
} }
auth := strings.SplitN(string(s), ":", 2) if username, password, ok := strings.Cut(string(s), ":"); ok {
return username, password, nil
if len(auth) != 2 {
return "", "", errors.New("invalid basic authentication")
} }
return "", "", errors.New("invalid basic authentication")
return auth[0], auth[1], nil
} }
// VerifyTimeLimitCode verify time limit code // VerifyTimeLimitCode verify time limit code

View File

@ -41,6 +41,9 @@ func TestBasicAuthDecode(t *testing.T) {
_, _, err = BasicAuthDecode("invalid") _, _, err = BasicAuthDecode("invalid")
assert.Error(t, err) assert.Error(t, err)
_, _, err = BasicAuthDecode("YWxpY2U=") // "alice", no colon
assert.Error(t, err)
} }
func TestVerifyTimeLimitCode(t *testing.T) { func TestVerifyTimeLimitCode(t *testing.T) {

View File

@ -5,7 +5,6 @@ package auth
import ( import (
go_context "context" go_context "context"
"encoding/base64"
"errors" "errors"
"fmt" "fmt"
"html" "html"
@ -326,10 +325,29 @@ func getOAuthGroupsForUser(ctx go_context.Context, user *user_model.User) ([]str
return groups, nil return groups, nil
} }
func parseBasicAuth(ctx *context.Context) (username, password string, err error) {
authHeader := ctx.Req.Header.Get("Authorization")
if authType, authData, ok := strings.Cut(authHeader, " "); ok && authType == "Basic" {
return base.BasicAuthDecode(authData)
}
return "", "", errors.New("invalid basic authentication")
}
// IntrospectOAuth introspects an oauth token // IntrospectOAuth introspects an oauth token
func IntrospectOAuth(ctx *context.Context) { func IntrospectOAuth(ctx *context.Context) {
if ctx.Doer == nil { clientIDValid := false
ctx.Resp.Header().Set("WWW-Authenticate", `Bearer realm=""`) if clientID, clientSecret, err := parseBasicAuth(ctx); err == nil {
app, err := auth.GetOAuth2ApplicationByClientID(ctx, clientID)
if err != nil && !auth.IsErrOauthClientIDInvalid(err) {
// this is likely a database error; log it and respond without details
log.Error("Error retrieving client_id: %v", err)
ctx.Error(http.StatusInternalServerError)
return
}
clientIDValid = err == nil && app.ValidateClientSecret([]byte(clientSecret))
}
if !clientIDValid {
ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm=""`)
ctx.PlainText(http.StatusUnauthorized, "no valid authorization") ctx.PlainText(http.StatusUnauthorized, "no valid authorization")
return return
} }
@ -639,9 +657,8 @@ 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 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 form.ClientID == "" || form.ClientSecret == "" {
authHeader := ctx.Req.Header.Get("Authorization") authHeader := ctx.Req.Header.Get("Authorization")
authContent := strings.SplitN(authHeader, " ", 2) if authType, authData, ok := strings.Cut(authHeader, " "); ok && authType == "Basic" {
if len(authContent) == 2 && authContent[0] == "Basic" { clientID, clientSecret, err := base.BasicAuthDecode(authData)
payload, err := base64.StdEncoding.DecodeString(authContent[1])
if err != nil { if err != nil {
handleAccessTokenError(ctx, AccessTokenError{ handleAccessTokenError(ctx, AccessTokenError{
ErrorCode: AccessTokenErrorCodeInvalidRequest, ErrorCode: AccessTokenErrorCodeInvalidRequest,
@ -649,30 +666,23 @@ func AccessTokenOAuth(ctx *context.Context) {
}) })
return return
} }
pair := strings.SplitN(string(payload), ":", 2) // validate that any fields present in the form match the Basic auth header
if len(pair) != 2 { if form.ClientID != "" && form.ClientID != clientID {
handleAccessTokenError(ctx, AccessTokenError{
ErrorCode: AccessTokenErrorCodeInvalidRequest,
ErrorDescription: "cannot parse basic auth header",
})
return
}
if form.ClientID != "" && form.ClientID != pair[0] {
handleAccessTokenError(ctx, AccessTokenError{ handleAccessTokenError(ctx, AccessTokenError{
ErrorCode: AccessTokenErrorCodeInvalidRequest, ErrorCode: AccessTokenErrorCodeInvalidRequest,
ErrorDescription: "client_id in request body inconsistent with Authorization header", ErrorDescription: "client_id in request body inconsistent with Authorization header",
}) })
return return
} }
form.ClientID = pair[0] form.ClientID = clientID
if form.ClientSecret != "" && form.ClientSecret != pair[1] { if form.ClientSecret != "" && form.ClientSecret != clientSecret {
handleAccessTokenError(ctx, AccessTokenError{ handleAccessTokenError(ctx, AccessTokenError{
ErrorCode: AccessTokenErrorCodeInvalidRequest, ErrorCode: AccessTokenErrorCodeInvalidRequest,
ErrorDescription: "client_secret in request body inconsistent with Authorization header", ErrorDescription: "client_secret in request body inconsistent with Authorization header",
}) })
return return
} }
form.ClientSecret = pair[1] form.ClientSecret = clientSecret
} }
} }

View File

@ -419,3 +419,59 @@ func TestRefreshTokenInvalidation(t *testing.T) {
assert.Equal(t, "unauthorized_client", string(parsedError.ErrorCode)) assert.Equal(t, "unauthorized_client", string(parsedError.ErrorCode))
assert.Equal(t, "token was already used", parsedError.ErrorDescription) assert.Equal(t, "token was already used", parsedError.ErrorDescription)
} }
func TestOAuthIntrospection(t *testing.T) {
defer tests.PrepareTestEnv(t)()
req := NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{
"grant_type": "authorization_code",
"client_id": "da7da3ba-9a13-4167-856f-3899de0b0138",
"client_secret": "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=",
"redirect_uri": "a",
"code": "authcode",
"code_verifier": "N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt",
})
resp := MakeRequest(t, req, http.StatusOK)
type response struct {
AccessToken string `json:"access_token"`
TokenType string `json:"token_type"`
ExpiresIn int64 `json:"expires_in"`
RefreshToken string `json:"refresh_token"`
}
parsed := new(response)
assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsed))
assert.True(t, len(parsed.AccessToken) > 10)
assert.True(t, len(parsed.RefreshToken) > 10)
// successful request with a valid client_id/client_secret and a valid token
req = NewRequestWithValues(t, "POST", "/login/oauth/introspect", map[string]string{
"token": parsed.AccessToken,
})
req.Header.Add("Authorization", "Basic ZGE3ZGEzYmEtOWExMy00MTY3LTg1NmYtMzg5OWRlMGIwMTM4OjRNSzhOYTZSNTVzbWRDWTBXdUNDdW1aNmhqUlBuR1k1c2FXVlJISGpKaUE9")
resp = MakeRequest(t, req, http.StatusOK)
type introspectResponse struct {
Active bool `json:"active"`
Scope string `json:"scope,omitempty"`
}
introspectParsed := new(introspectResponse)
assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), introspectParsed))
assert.True(t, introspectParsed.Active)
// successful request with a valid client_id/client_secret, but an invalid token
req = NewRequestWithValues(t, "POST", "/login/oauth/introspect", map[string]string{
"token": "xyzzy",
})
req.Header.Add("Authorization", "Basic ZGE3ZGEzYmEtOWExMy00MTY3LTg1NmYtMzg5OWRlMGIwMTM4OjRNSzhOYTZSNTVzbWRDWTBXdUNDdW1aNmhqUlBuR1k1c2FXVlJISGpKaUE9")
resp = MakeRequest(t, req, http.StatusOK)
introspectParsed = new(introspectResponse)
assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), introspectParsed))
assert.False(t, introspectParsed.Active)
// unsuccessful request with an invalid client_id/client_secret
req = NewRequestWithValues(t, "POST", "/login/oauth/introspect", map[string]string{
"token": parsed.AccessToken,
})
req.Header.Add("Authorization", "Basic ZGE3ZGEzYmEtOWExMy00MTY3LTg1NmYtMzg5OWRlMGIwMTM4OjRNSzhOYTZSNTVzbWRDWTBXdUNDdW1aNmhqUlBuR1k1c2FXVlJISGpK")
resp = MakeRequest(t, req, http.StatusUnauthorized)
assert.Contains(t, resp.Body.String(), "no valid authorization")
}