From 8d1cf92e128bb4b209949d1ee95de2f874b174c5 Mon Sep 17 00:00:00 2001 From: forgejo-backport-action Date: Sat, 30 Aug 2025 18:43:53 +0200 Subject: [PATCH] [v12.0/forgejo] fix: require password login for creation of new token (#9080) **Backport:** https://codeberg.org/forgejo/forgejo/pulls/9070 Obtaining a [personal access token via the API](https://forgejo.org/docs/latest/user/api-usage/#generating-and-listing-api-tokens) is no longer possible if the password used for basic authentication is an API token or an [OAuth2 token](https://forgejo.org/docs/latest/user/api-usage/#oauth2-provider): it has to be the user password. Such privilege escalation was only possible for tokens with write permissions to the user. This requirement is already enforced when API calls are made with an authorization header [as described in the documentation](https://forgejo.org/docs/latest/user/api-usage/#authentication), but it was not enforced with basic authentication. As a consequence it was possible for an API token with `write:user` permissions or an OAuth2 token to obtain a new token with a wider or identical scope. ## Release notes - Bug fixes - [PR](https://codeberg.org/forgejo/forgejo/pulls/9070): require password login for creation of new token Co-authored-by: Gusted Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9080 Reviewed-by: 0ko <0ko@noreply.codeberg.org> Co-authored-by: forgejo-backport-action Co-committed-by: forgejo-backport-action --- routers/api/v1/api.go | 7 ++- services/auth/basic.go | 1 + tests/integration/api_token_test.go | 81 +++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 2 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index c3e8b532cf..b50ca19148 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -415,8 +415,11 @@ func reqBasicOrRevProxyAuth() func(ctx *context.APIContext) { if ctx.IsSigned && setting.Service.EnableReverseProxyAuthAPI && ctx.Data["AuthedMethod"].(string) == auth.ReverseProxyMethodName { return } - if !ctx.IsBasicAuth { - ctx.Error(http.StatusUnauthorized, "reqBasicAuth", "auth required") + + // Require basic authorization method to be used and that basic + // authorization used password login to verify the user. + if passwordLogin, ok := ctx.Data["IsPasswordLogin"].(bool); !ok || !passwordLogin { + ctx.Error(http.StatusUnauthorized, "reqBasicAuth", "auth method not allowed") return } } diff --git a/services/auth/basic.go b/services/auth/basic.go index f259ad5f69..4ffe712744 100644 --- a/services/auth/basic.go +++ b/services/auth/basic.go @@ -151,6 +151,7 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore log.Trace("Basic Authorization: Logged in user %-v", u) + store.GetData()["IsPasswordLogin"] = true return u, nil } diff --git a/tests/integration/api_token_test.go b/tests/integration/api_token_test.go index 9d9a44b5d4..0899d0abfa 100644 --- a/tests/integration/api_token_test.go +++ b/tests/integration/api_token_test.go @@ -4,8 +4,10 @@ package integration import ( + "encoding/base64" "fmt" "net/http" + "net/url" "testing" auth_model "forgejo.org/models/auth" @@ -13,9 +15,11 @@ import ( user_model "forgejo.org/models/user" "forgejo.org/modules/log" api "forgejo.org/modules/structs" + "forgejo.org/modules/test" "forgejo.org/tests" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // TestAPICreateAndDeleteToken tests that token that was just created can be deleted @@ -580,3 +584,80 @@ func deleteAPIAccessToken(t *testing.T, accessToken api.AccessToken, user *user_ unittest.AssertNotExistsBean(t, &auth_model.AccessToken{ID: accessToken.ID}) } + +func TestAPITokenCreation(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + session := loginUser(t, "user4") + t.Run("Via API token", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteUser) + + req := NewRequestWithJSON(t, "POST", "/api/v1/users/user4/tokens", map[string]any{ + "name": "new-new-token", + "scopes": []auth_model.AccessTokenScope{auth_model.AccessTokenScopeWriteUser}, + }) + req.Request.Header.Set("Authorization", "basic "+base64.StdEncoding.EncodeToString([]byte("user4:"+token))) + + resp := MakeRequest(t, req, http.StatusUnauthorized) + + respMsg := map[string]any{} + DecodeJSON(t, resp, &respMsg) + + assert.EqualValues(t, "auth method not allowed", respMsg["message"]) + }) + + t.Run("Via OAuth2", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithValues(t, "POST", "/login/oauth/grant", map[string]string{ + "_csrf": GetCSRF(t, session, "/login/oauth/authorize?client_id=ce5a1322-42a7-11ed-b878-0242ac120002&redirect_uri=b&response_type=code&code_challenge_method=plain&code_challenge=CODE&state=thestate"), + "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", + "redirect_uri": "b", + "state": "thestate", + "granted": "true", + }) + resp := session.MakeRequest(t, req, http.StatusSeeOther) + + u, err := url.Parse(test.RedirectURL(resp)) + require.NoError(t, err) + + req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ + "client_id": "ce5a1322-42a7-11ed-b878-0242ac120002", + "code": u.Query().Get("code"), + "code_verifier": "CODE", + "grant_type": "authorization_code", + "redirect_uri": "b", + }) + resp = MakeRequest(t, req, http.StatusOK) + + var respBody map[string]any + DecodeJSON(t, resp, &respBody) + + req = NewRequestWithJSON(t, "POST", "/api/v1/users/user4/tokens", map[string]any{ + "name": "new-new-token", + "scopes": []auth_model.AccessTokenScope{auth_model.AccessTokenScopeWriteUser}, + }) + req.Request.Header.Set("Authorization", "basic "+base64.StdEncoding.EncodeToString([]byte("user4:"+respBody["access_token"].(string)))) + + resp = MakeRequest(t, req, http.StatusUnauthorized) + + respMsg := map[string]any{} + DecodeJSON(t, resp, &respMsg) + + assert.EqualValues(t, "auth method not allowed", respMsg["message"]) + }) + + t.Run("Via password", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithJSON(t, "POST", "/api/v1/users/user4/tokens", map[string]any{ + "name": "new-new-token", + "scopes": []auth_model.AccessTokenScope{auth_model.AccessTokenScopeWriteUser}, + }) + req.Request.Header.Set("Authorization", "basic "+base64.StdEncoding.EncodeToString([]byte("user4:"+userPassword))) + + MakeRequest(t, req, http.StatusCreated) + }) +}