From 526054332acb221e061d3900bba2dc6e012da52d Mon Sep 17 00:00:00 2001
From: KN4CK3R <admin@oldschoolhack.me>
Date: Fri, 20 Sep 2024 21:00:39 +0200
Subject: [PATCH] Fix incorrect `/tokens` api (#32085)
Fixes #32078
- Add missing scopes output.
- Disallow empty scope.
---------
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
(cherry picked from commit 08adbc468f8875fd4763c3656b334203c11adc0a)
---
routers/api/v1/user/app.go | 5 +++++
tests/integration/api_token_test.go | 31 ++++++++++-------------------
2 files changed, 16 insertions(+), 20 deletions(-)
diff --git a/routers/api/v1/user/app.go b/routers/api/v1/user/app.go
index b61ebac7d0..d5b20f7703 100644
--- a/routers/api/v1/user/app.go
+++ b/routers/api/v1/user/app.go
@@ -118,6 +118,10 @@ func CreateAccessToken(ctx *context.APIContext) {
ctx.Error(http.StatusBadRequest, "AccessTokenScope.Normalize", fmt.Errorf("invalid access token scope provided: %w", err))
return
}
+ if scope == "" {
+ ctx.Error(http.StatusBadRequest, "AccessTokenScope", "access token must have a scope")
+ return
+ }
t.Scope = scope
if err := auth_model.NewAccessToken(ctx, t); err != nil {
@@ -129,6 +133,7 @@ func CreateAccessToken(ctx *context.APIContext) {
Token: t.Token,
ID: t.ID,
TokenLastEight: t.TokenLastEight,
+ Scopes: t.Scope.StringSlice(),
})
}
diff --git a/tests/integration/api_token_test.go b/tests/integration/api_token_test.go
index 9c7bf37330..01d18ef6f1 100644
--- a/tests/integration/api_token_test.go
+++ b/tests/integration/api_token_test.go
@@ -23,10 +23,10 @@ func TestAPICreateAndDeleteToken(t *testing.T) {
defer tests.PrepareTestEnv(t)()
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)
- 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)
}
@@ -72,19 +72,19 @@ func TestAPIDeleteTokensPermission(t *testing.T) {
user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
// 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").
AddBasicAuth(admin.Name)
MakeRequest(t, req, http.StatusNoContent)
// 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").
AddBasicAuth(user2.Name)
MakeRequest(t, req, http.StatusNoContent)
// 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").
AddBasicAuth(user4.Name)
MakeRequest(t, req, http.StatusForbidden)
@@ -520,7 +520,7 @@ func runTestCase(t *testing.T, testCase *requiredScopeTestCase, user *user_model
unauthorizedScopes = append(unauthorizedScopes, cateogoryUnauthorizedScopes...)
}
- accessToken := createAPIAccessTokenWithoutCleanUp(t, "test-token", user, &unauthorizedScopes)
+ accessToken := createAPIAccessTokenWithoutCleanUp(t, "test-token", user, unauthorizedScopes)
defer deleteAPIAccessToken(t, accessToken, user)
// 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
// 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{
- "name": tokenName,
- }
- 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
- }
+ "name": tokenName,
+ "scopes": scopes,
}
+
log.Debug("Requesting creation of token with scopes: %v", scopes)
req := NewRequestWithJSON(t, "POST", "/api/v1/users/"+user.LoginName+"/tokens", payload).
AddBasicAuth(user.Name)
@@ -563,8 +555,7 @@ func createAPIAccessTokenWithoutCleanUp(t *testing.T, tokenName string, user *us
return newAccessToken
}
-// createAPIAccessTokenWithoutCleanUp Delete an API access token and assert that
-// deletion succeeded.
+// deleteAPIAccessToken deletes an API access token and assert that deletion succeeded.
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).
AddBasicAuth(user.Name)