Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix OIDC introspection authentication #31632

Merged
merged 13 commits into from
Jul 23, 2024
Merged
43 changes: 25 additions & 18 deletions routers/web/auth/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package auth

import (
go_context "context"
"encoding/base64"
"errors"
"fmt"
"html"
Expand Down Expand Up @@ -327,10 +326,26 @@ func getOAuthGroupsForUser(ctx go_context.Context, user *user_model.User) ([]str
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" {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe auth scheme "Basic" should be compare case insensitively

"Note that both scheme and parameter names are matched case-insensitively."
https://www.rfc-editor.org/rfc/rfc7617.html#section-2

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. @slingamn was that a deliberate change or we just missed it in reviewing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't intentional :-) I just refactored the original implementation (#6293) which was case-sensitive. So there's no behavior change, but I agree that a case-insensitive comparison would be more correct.

return base.BasicAuthDecode(authData)
}
return "", "", errors.New("invalid basic authentication")
}

// IntrospectOAuth introspects an oauth token
func IntrospectOAuth(ctx *context.Context) {
if ctx.Doer == nil {
ctx.Resp.Header().Set("WWW-Authenticate", `Bearer realm=""`)
clientIDValid := false
if clientID, clientSecret, err := parseBasicAuth(ctx); err == nil {
if app, err := auth.GetOAuth2ApplicationByClientID(ctx, clientID); err == nil {
slingamn marked this conversation as resolved.
Show resolved Hide resolved
slingamn marked this conversation as resolved.
Show resolved Hide resolved
if app.ValidateClientSecret([]byte(clientSecret)) {
clientIDValid = true
}
slingamn marked this conversation as resolved.
Show resolved Hide resolved
}
}
if !clientIDValid {
ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm=""`)
ctx.PlainText(http.StatusUnauthorized, "no valid authorization")
return
}
Expand Down Expand Up @@ -640,40 +655,32 @@ 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 == "" {
authHeader := ctx.Req.Header.Get("Authorization")
authContent := strings.SplitN(authHeader, " ", 2)
if len(authContent) == 2 && authContent[0] == "Basic" {
payload, err := base64.StdEncoding.DecodeString(authContent[1])
if authType, authData, ok := strings.Cut(authHeader, " "); ok && authType == "Basic" {
clientID, clientSecret, err := base.BasicAuthDecode(authData)
if err != nil {
handleAccessTokenError(ctx, AccessTokenError{
ErrorCode: AccessTokenErrorCodeInvalidRequest,
ErrorDescription: "cannot parse basic auth header",
})
return
}
pair := strings.SplitN(string(payload), ":", 2)
if len(pair) != 2 {
handleAccessTokenError(ctx, AccessTokenError{
ErrorCode: AccessTokenErrorCodeInvalidRequest,
ErrorDescription: "cannot parse basic auth header",
})
return
}
if form.ClientID != "" && form.ClientID != pair[0] {
// validate that any fields present in the form match the Basic auth header
if form.ClientID != "" && form.ClientID != clientID {
handleAccessTokenError(ctx, AccessTokenError{
ErrorCode: AccessTokenErrorCodeInvalidRequest,
ErrorDescription: "client_id in request body inconsistent with Authorization header",
})
return
}
form.ClientID = pair[0]
if form.ClientSecret != "" && form.ClientSecret != pair[1] {
form.ClientID = clientID
if form.ClientSecret != "" && form.ClientSecret != clientSecret {
handleAccessTokenError(ctx, AccessTokenError{
ErrorCode: AccessTokenErrorCodeInvalidRequest,
ErrorDescription: "client_secret in request body inconsistent with Authorization header",
})
return
}
form.ClientSecret = pair[1]
form.ClientSecret = clientSecret
}
}

Expand Down
37 changes: 37 additions & 0 deletions tests/integration/oauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,3 +419,40 @@ func TestRefreshTokenInvalidation(t *testing.T) {
assert.Equal(t, "unauthorized_client", string(parsedError.ErrorCode))
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{
slingamn marked this conversation as resolved.
Show resolved Hide resolved
"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)

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)
}