Skip to content

Fix incorrect /tokens api (#32085) #32092

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 22, 2024
Merged

Fix incorrect /tokens api (#32085) #32092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0110f3b
Fix incorrect `/tokens` api (#32085)
KN4CK3R Sep 20, 2024
830a052
Merge branch 'release/v1.22' into backport-32085-v1.22
GiteaBot Sep 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions routers/api/v1/user/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -129,6 +133,7 @@ func CreateAccessToken(ctx *context.APIContext) {
Token: t.Token,
ID: t.ID,
TokenLastEight: t.TokenLastEight,
Scopes: t.Scope.StringSlice(),
})
}

Expand Down
31 changes: 11 additions & 20 deletions tests/integration/api_token_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand All @@ -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)
Expand All @@ -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)
Expand Down