peezy-tech/jojo
2
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2026-05-13 06:20:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

fix: better error message for invalid tokens (#10727)

Browse source 
Fixes https://codeberg.org/forgejo/forgejo/issues/7514
When a token is used, provide a better error message when that token is not valid.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10727
Reviewed-by: Ellen Εμιλία Άννα Zscheile <fogti@noreply.codeberg.org>
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: zokki <zokki.softwareschmiede@gmail.com>
Co-committed-by: zokki <zokki.softwareschmiede@gmail.com>
This commit is contained in:
zokki 2026-01-13 18:18:40 +01:00 • committed by Gusted
parent a71392c6aa
commit 5afa5a2b5a
3 changed files with 47 additions and 19 deletions
Download patch file Download diff file Expand all files Collapse all files

25
services/auth/oauth2.go
View file

@ -147,7 +147,10 @@ func parseToken(req *http.Request) (string, bool) {
// userIDFromToken returns the user id corresponding to the OAuth token.
// It will set 'IsApiToken' to true if the token is an API token and
// set 'ApiTokenScope' to the scope of the access token
func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string, store DataStore) int64 {
func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string, store DataStore) (int64, error) {
if tokenSHA == "" {
return 0, auth_model.ErrAccessTokenEmpty{}
}
// Let's see if token is valid.
if strings.Contains(tokenSHA, ".") {
// First attempt to decode an actions JWT, returning the actions user
@ -155,7 +158,7 @@ func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string, store Dat
if CheckTaskIsRunning(ctx, taskID) {
store.GetData()["IsActionsToken"] = true
store.GetData()["ActionsTaskID"] = taskID
return user_model.ActionsUserID
return user_model.ActionsUserID, nil
}
}
@ -169,7 +172,7 @@ func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string, store Dat
store.GetData()["ApiTokenScope"] = auth_model.AccessTokenScopeAll // fallback to all
}
}
return uid
return uid, nil
}
t, err := auth_model.GetAccessTokenBySHA(ctx, tokenSHA)
if err != nil {
@ -182,19 +185,22 @@ func (o *OAuth2) userIDFromToken(ctx context.Context, tokenSHA string, store Dat
store.GetData()["IsActionsToken"] = true
store.GetData()["ActionsTaskID"] = task.ID
return user_model.ActionsUserID
return user_model.ActionsUserID, nil
}
} else if !auth_model.IsErrAccessTokenNotExist(err) && !auth_model.IsErrAccessTokenEmpty(err) {
log.Error("GetAccessTokenBySHA: %v", err)
}
return 0
return 0, err
}
if err := t.UpdateLastUsed(ctx); err != nil {
log.Error("UpdateLastUsed: %v", err)
}
if t.UID == 0 {
return 0, auth_model.ErrAccessTokenNotExist{}
}
store.GetData()["IsApiToken"] = true
store.GetData()["ApiTokenScope"] = t.Scope
return t.UID
return t.UID, nil
}
// Verify extracts the user ID from the OAuth token in the query parameters
@ -213,10 +219,9 @@ func (o *OAuth2) Verify(req *http.Request, w http.ResponseWriter, store DataStor
return nil, nil
}
id := o.userIDFromToken(req.Context(), token, store)
if id <= 0 && id != -2 { // -2 means actions, so we need to allow it.
return nil, user_model.ErrUserNotExist{}
id, err := o.userIDFromToken(req.Context(), token, store)
if err != nil {
return nil, err
}
log.Trace("OAuth2 Authorization: Found token for user[%d]", id)

39
services/auth/oauth2_test.go
View file

@ -7,6 +7,7 @@ import (
"net/http"
"testing"
"forgejo.org/models/auth"
"forgejo.org/models/unittest"
user_model "forgejo.org/models/user"
"forgejo.org/modules/web/middleware"
@ -27,11 +28,32 @@ func TestUserIDFromToken(t *testing.T) {
ds := make(middleware.ContextData)
o := OAuth2{}
uid := o.userIDFromToken(t.Context(), token, ds)
uid, err := o.userIDFromToken(t.Context(), token, ds)
require.NoError(t, err)
assert.Equal(t, int64(user_model.ActionsUserID), uid)
assert.Equal(t, true, ds["IsActionsToken"])
assert.Equal(t, ds["ActionsTaskID"], int64(RunningTaskID))
})
t.Run("Actions error-JWT", func(t *testing.T) {
cases := map[string]struct {
Token string
Error error
}{
"Empty": {"", auth.ErrAccessTokenEmpty{}},
"To short": {"abc", auth.ErrAccessTokenNotExist{Token: "abc"}},
}
ds := make(middleware.ContextData)
o := OAuth2{}
for name, c := range cases {
t.Run(name, func(t *testing.T) {
uid, err := o.userIDFromToken(t.Context(), c.Token, ds)
require.ErrorIs(t, err, c.Error)
assert.Equal(t, int64(0), uid)
})
}
})
}
func TestCheckTaskIsRunning(t *testing.T) {
@ -60,13 +82,14 @@ func TestParseToken(t *testing.T) {
ExpectedToken string
Expected bool
}{
"Token Uppercase": {Header: "Token 1234567890123456789012345687901325467890", ExpectedToken: "1234567890123456789012345687901325467890", Expected: true},
"Token Lowercase": {Header: "token 1234567890123456789012345687901325467890", ExpectedToken: "1234567890123456789012345687901325467890", Expected: true},
"Token Unicode": {Header: "to\u212Aen 1234567890123456789012345687901325467890", ExpectedToken: "", Expected: false},
"Bearer Uppercase": {Header: "Bearer 1234567890123456789012345687901325467890", ExpectedToken: "1234567890123456789012345687901325467890", Expected: true},
"Bearer Lowercase": {Header: "bearer 1234567890123456789012345687901325467890", ExpectedToken: "1234567890123456789012345687901325467890", Expected: true},
"Missing type": {Header: "1234567890123456789012345687901325467890", ExpectedToken: "", Expected: false},
"Three Parts": {Header: "abc 1234567890 test", ExpectedToken: "", Expected: false},
"Token Uppercase": {Header: "Token 1234567890123456789012345687901325467890", ExpectedToken: "1234567890123456789012345687901325467890", Expected: true},
"Token Lowercase": {Header: "token 1234567890123456789012345687901325467890", ExpectedToken: "1234567890123456789012345687901325467890", Expected: true},
"Token Unicode": {Header: "to\u212Aen 1234567890123456789012345687901325467890", ExpectedToken: "", Expected: false},
"Bearer Uppercase": {Header: "Bearer 1234567890123456789012345687901325467890", ExpectedToken: "1234567890123456789012345687901325467890", Expected: true},
"Bearer Lowercase": {Header: "bearer 1234567890123456789012345687901325467890", ExpectedToken: "1234567890123456789012345687901325467890", Expected: true},
"Missing type": {Header: "1234567890123456789012345687901325467890", ExpectedToken: "", Expected: false},
"Three Parts": {Header: "abc 1234567890 test", ExpectedToken: "", Expected: false},
"Token Three Parts": {Header: "Token 1234567890 test", ExpectedToken: "", Expected: false},
}
for name := range cases {

2
tests/integration/api_repo_test.go
View file

@ -52,7 +52,7 @@ func TestAPIUserReposWithWrongToken(t *testing.T) {
AddTokenAuth(wrongToken)
resp := MakeRequest(t, req, http.StatusUnauthorized)
assert.Contains(t, resp.Body.String(), "user does not exist")
assert.Contains(t, resp.Body.String(), "access token does not exist")
}
func TestAPISearchRepo(t *testing.T) {