mirrors/forgejo
1
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2025-11-04 08:21:11 +00:00
Code Issues Projects Releases Packages Wiki Activity

Fix incorrect /tokens api (#32085)

Browse source 
Fixes #32078

- Add missing scopes output.
- Disallow empty scope.

---------

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
(cherry picked from commit 08adbc468f8875fd4763c3656b334203c11adc0a)
This commit is contained in:
KN4CK3R 2024-09-20 21:00:39 +02:00 • committed by Earl Warren
commit 526054332a
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
2 changed files with 16 additions and 20 deletions
Download patch file Download diff file Expand all files Collapse all files

5
routers/api/v1/user/app.go
View file

@ -118,6 +118,10 @@ func CreateAccessToken(ctx *context.APIContext) {
ctx.Error(http.StatusBadRequest, "AccessTokenScope.Normalize", fmt.Errorf("invalid access token scope provided: %w", err)) ctx.Error(http.StatusBadRequest, "AccessTokenScope.Normalize", fmt.Errorf("invalid access token scope provided: %w", err))
return return
} }
if scope == "" {
ctx.Error(http.StatusBadRequest, "AccessTokenScope", "access token must have a scope")
return
}
t.Scope = scope t.Scope = scope
if err := auth_model.NewAccessToken(ctx, t); err != nil { if err := auth_model.NewAccessToken(ctx, t); err != nil {
@ -129,6 +133,7 @@ func CreateAccessToken(ctx *context.APIContext) {
Token: t.Token, Token: t.Token,
ID: t.ID, ID: t.ID,
TokenLastEight: t.TokenLastEight, TokenLastEight: t.TokenLastEight,
Scopes: t.Scope.StringSlice(),
}) })
} }

31
tests/integration/api_token_test.go
View file

@ -23,10 +23,10 @@ func TestAPICreateAndDeleteToken(t *testing.T) {
defer tests.PrepareTestEnv(t)() defer tests.PrepareTestEnv(t)()
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
newAccessToken := createAPIAccessTokenWithoutCleanUp(t, "test-key-1", user, nil) newAccessToken := createAPIAccessTokenWithoutCleanUp(t, "test-key-1", user, []auth_model.AccessTokenScope{auth_model.AccessTokenScopeAll})
deleteAPIAccessToken(t, newAccessToken, user) deleteAPIAccessToken(t, newAccessToken, user)
newAccessToken = createAPIAccessTokenWithoutCleanUp(t, "test-key-2", user, nil) newAccessToken = createAPIAccessTokenWithoutCleanUp(t, "test-key-2", user, []auth_model.AccessTokenScope{auth_model.AccessTokenScopeAll})
deleteAPIAccessToken(t, newAccessToken, user) deleteAPIAccessToken(t, newAccessToken, user)
} }
@ -72,19 +72,19 @@ func TestAPIDeleteTokensPermission(t *testing.T) {
user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
// admin can delete tokens for other users // admin can delete tokens for other users
createAPIAccessTokenWithoutCleanUp(t, "test-key-1", user2, nil) createAPIAccessTokenWithoutCleanUp(t, "test-key-1", user2, []auth_model.AccessTokenScope{auth_model.AccessTokenScopeAll})
req := NewRequest(t, "DELETE", "/api/v1/users/"+user2.LoginName+"/tokens/test-key-1"). req := NewRequest(t, "DELETE", "/api/v1/users/"+user2.LoginName+"/tokens/test-key-1").
AddBasicAuth(admin.Name) AddBasicAuth(admin.Name)
MakeRequest(t, req, http.StatusNoContent) MakeRequest(t, req, http.StatusNoContent)
// non-admin can delete tokens for himself // non-admin can delete tokens for himself
createAPIAccessTokenWithoutCleanUp(t, "test-key-2", user2, nil) createAPIAccessTokenWithoutCleanUp(t, "test-key-2", user2, []auth_model.AccessTokenScope{auth_model.AccessTokenScopeAll})
req = NewRequest(t, "DELETE", "/api/v1/users/"+user2.LoginName+"/tokens/test-key-2"). req = NewRequest(t, "DELETE", "/api/v1/users/"+user2.LoginName+"/tokens/test-key-2").
AddBasicAuth(user2.Name) AddBasicAuth(user2.Name)
MakeRequest(t, req, http.StatusNoContent) MakeRequest(t, req, http.StatusNoContent)
// non-admin can't delete tokens for other users // non-admin can't delete tokens for other users
createAPIAccessTokenWithoutCleanUp(t, "test-key-3", user2, nil) createAPIAccessTokenWithoutCleanUp(t, "test-key-3", user2, []auth_model.AccessTokenScope{auth_model.AccessTokenScopeAll})
req = NewRequest(t, "DELETE", "/api/v1/users/"+user2.LoginName+"/tokens/test-key-3"). req = NewRequest(t, "DELETE", "/api/v1/users/"+user2.LoginName+"/tokens/test-key-3").
AddBasicAuth(user4.Name) AddBasicAuth(user4.Name)
MakeRequest(t, req, http.StatusForbidden) MakeRequest(t, req, http.StatusForbidden)
@ -520,7 +520,7 @@ func runTestCase(t *testing.T, testCase *requiredScopeTestCase, user *user_model
unauthorizedScopes = append(unauthorizedScopes, cateogoryUnauthorizedScopes...) unauthorizedScopes = append(unauthorizedScopes, cateogoryUnauthorizedScopes...)
} }
accessToken := createAPIAccessTokenWithoutCleanUp(t, "test-token", user, &unauthorizedScopes) accessToken := createAPIAccessTokenWithoutCleanUp(t, "test-token", user, unauthorizedScopes)
defer deleteAPIAccessToken(t, accessToken, user) defer deleteAPIAccessToken(t, accessToken, user)
// Request the endpoint. Verify that permission is denied. // Request the endpoint. Verify that permission is denied.
@ -532,20 +532,12 @@ func runTestCase(t *testing.T, testCase *requiredScopeTestCase, user *user_model
// createAPIAccessTokenWithoutCleanUp Create an API access token and assert that // createAPIAccessTokenWithoutCleanUp Create an API access token and assert that
// creation succeeded. The caller is responsible for deleting the token. // creation succeeded. The caller is responsible for deleting the token.
func createAPIAccessTokenWithoutCleanUp(t *testing.T, tokenName string, user *user_model.User, scopes *[]auth_model.AccessTokenScope) api.AccessToken { func createAPIAccessTokenWithoutCleanUp(t *testing.T, tokenName string, user *user_model.User, scopes []auth_model.AccessTokenScope) api.AccessToken {
payload := map[string]any{ payload := map[string]any{
"name": tokenName, "name": tokenName,
} "scopes": scopes,
if scopes != nil {
for _, scope := range *scopes {
scopes, scopesExists := payload["scopes"].([]string)
if !scopesExists {
scopes = make([]string, 0)
}
scopes = append(scopes, string(scope))
payload["scopes"] = scopes
}
} }
log.Debug("Requesting creation of token with scopes: %v", scopes) log.Debug("Requesting creation of token with scopes: %v", scopes)
req := NewRequestWithJSON(t, "POST", "/api/v1/users/"+user.LoginName+"/tokens", payload). req := NewRequestWithJSON(t, "POST", "/api/v1/users/"+user.LoginName+"/tokens", payload).
AddBasicAuth(user.Name) AddBasicAuth(user.Name)
@ -563,8 +555,7 @@ func createAPIAccessTokenWithoutCleanUp(t *testing.T, tokenName string, user *us
return newAccessToken return newAccessToken
} }
// createAPIAccessTokenWithoutCleanUp Delete an API access token and assert that // deleteAPIAccessToken deletes an API access token and assert that deletion succeeded.
// deletion succeeded.
func deleteAPIAccessToken(t *testing.T, accessToken api.AccessToken, user *user_model.User) { func deleteAPIAccessToken(t *testing.T, accessToken api.AccessToken, user *user_model.User) {
req := NewRequestf(t, "DELETE", "/api/v1/users/"+user.LoginName+"/tokens/%d", accessToken.ID). req := NewRequestf(t, "DELETE", "/api/v1/users/"+user.LoginName+"/tokens/%d", accessToken.ID).
AddBasicAuth(user.Name) AddBasicAuth(user.Name)