diff --git a/modules/base/tool.go b/modules/base/tool.go
index c4c0ec2dfc..7612fff73a 100644
--- a/modules/base/tool.go
+++ b/modules/base/tool.go
@@ -48,13 +48,10 @@ func BasicAuthDecode(encoded string) (string, string, error) {
return "", "", err
}
- auth := strings.SplitN(string(s), ":", 2)
-
- if len(auth) != 2 {
- return "", "", errors.New("invalid basic authentication")
+ if username, password, ok := strings.Cut(string(s), ":"); ok {
+ return username, password, nil
}
-
- return auth[0], auth[1], nil
+ return "", "", errors.New("invalid basic authentication")
}
// VerifyTimeLimitCode verify time limit code
diff --git a/modules/base/tool_test.go b/modules/base/tool_test.go
index eb38e2969e..81fd4b6a9e 100644
--- a/modules/base/tool_test.go
+++ b/modules/base/tool_test.go
@@ -41,6 +41,9 @@ func TestBasicAuthDecode(t *testing.T) {
_, _, err = BasicAuthDecode("invalid")
require.Error(t, err)
+
+ _, _, err = BasicAuthDecode("YWxpY2U=") // "alice", no colon
+ require.Error(t, err)
}
func TestVerifyTimeLimitCode(t *testing.T) {
diff --git a/release-notes/4724.md b/release-notes/4724.md
new file mode 100644
index 0000000000..4037c710b0
--- /dev/null
+++ b/release-notes/4724.md
@@ -0,0 +1 @@
+OIDC integrations that POST to `/login/oauth/introspect` without sending HTTP basic authentication will now fail with a 401 HTTP Unauthorized error. To fix the error, the client must begin sending HTTP basic authentication with a valid client ID and secret. This endpoint was previously authenticated via the introspection token itself, which is less secure.
diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go
index 8ffc2b711c..c9cdb08d9f 100644
--- a/routers/web/auth/oauth.go
+++ b/routers/web/auth/oauth.go
@@ -332,17 +332,37 @@ 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 && strings.EqualFold(authType, "Basic") {
+ 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 {
+ 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")
return
}
var response struct {
- Active bool `json:"active"`
- Scope string `json:"scope,omitempty"`
+ Active bool `json:"active"`
+ Scope string `json:"scope,omitempty"`
+ Username string `json:"username,omitempty"`
jwt.RegisteredClaims
}
@@ -359,6 +379,9 @@ func IntrospectOAuth(ctx *context.Context) {
response.Audience = []string{app.ClientID}
response.Subject = fmt.Sprint(grant.UserID)
}
+ if user, err := user_model.GetUserByID(ctx, grant.UserID); err == nil {
+ response.Username = user.Name
+ }
}
}
@@ -645,9 +668,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 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 && strings.EqualFold(authType, "Basic") {
+ clientID, clientSecret, err := base.BasicAuthDecode(authData)
if err != nil {
handleAccessTokenError(ctx, AccessTokenError{
ErrorCode: AccessTokenErrorCodeInvalidRequest,
@@ -655,30 +677,23 @@ func AccessTokenOAuth(ctx *context.Context) {
})
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
}
}
diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go
index 240d374945..785e6dd266 100644
--- a/tests/integration/oauth_test.go
+++ b/tests/integration/oauth_test.go
@@ -733,3 +733,69 @@ func TestOAuth_GrantApplicationOAuth(t *testing.T) {
resp = ctx.MakeRequest(t, req, http.StatusSeeOther)
assert.Contains(t, test.RedirectURL(resp), "error=access_denied&error_description=the+request+is+denied")
}
+
+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)
+
+ DecodeJSON(t, resp, parsed)
+ assert.Greater(t, len(parsed.AccessToken), 10)
+ assert.Greater(t, len(parsed.RefreshToken), 10)
+
+ type introspectResponse struct {
+ Active bool `json:"active"`
+ Scope string `json:"scope,omitempty"`
+ Username string `json:"username"`
+ }
+
+ // successful request with a valid client_id/client_secret and a valid token
+ t.Run("successful request with valid token", func(t *testing.T) {
+ 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)
+
+ introspectParsed := new(introspectResponse)
+ DecodeJSON(t, resp, introspectParsed)
+ assert.True(t, introspectParsed.Active)
+ assert.Equal(t, "user1", introspectParsed.Username)
+ })
+
+ // successful request with a valid client_id/client_secret, but an invalid token
+ t.Run("successful request with invalid token", func(t *testing.T) {
+ 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)
+ DecodeJSON(t, resp, introspectParsed)
+ assert.False(t, introspectParsed.Active)
+ })
+
+ // unsuccessful request with an invalid client_id/client_secret
+ t.Run("unsuccessful request due to invalid basic auth", func(t *testing.T) {
+ 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")
+ })
+}