From f93ee5937bcb43aaf1e3b527d852487e80ae570b Mon Sep 17 00:00:00 2001 From: CaiCandong <50507092+CaiCandong@users.noreply.github.com> Date: Mon, 18 Sep 2023 08:21:15 +0800 Subject: [PATCH] Fix token endpoints ignore specified account (#27080) Fix #26234 close #26323 close #27040 --------- Co-authored-by: silverwind --- routers/api/v1/api.go | 12 +++++++++++- routers/api/v1/user/app.go | 12 +++++++++--- templates/swagger/v1_json.tmpl | 9 +++++++++ tests/integration/api_token_test.go | 23 +++++++++++++++++++++++ 4 files changed, 52 insertions(+), 4 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index d58e39920b..763d56ecd2 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -367,6 +367,16 @@ func reqOwner() func(ctx *context.APIContext) { } } +// reqSelfOrAdmin doer should be the same as the contextUser or site admin +func reqSelfOrAdmin() func(ctx *context.APIContext) { + return func(ctx *context.APIContext) { + if !ctx.IsUserSiteAdmin() && ctx.ContextUser != ctx.Doer { + ctx.Error(http.StatusForbidden, "reqSelfOrAdmin", "doer should be the site admin or be same as the contextUser") + return + } + } +} + // reqAdmin user should be an owner or a collaborator with admin write of a repository, or site admin func reqAdmin() func(ctx *context.APIContext) { return func(ctx *context.APIContext) { @@ -910,7 +920,7 @@ func Routes() *web.Route { m.Combo("").Get(user.ListAccessTokens). Post(bind(api.CreateAccessTokenOption{}), reqToken(), user.CreateAccessToken) m.Combo("/{id}").Delete(reqToken(), user.DeleteAccessToken) - }, reqBasicOrRevProxyAuth()) + }, reqSelfOrAdmin(), reqBasicOrRevProxyAuth()) m.Get("/activities/feeds", user.ListUserActivityFeeds) }, context_service.UserAssignmentAPI()) diff --git a/routers/api/v1/user/app.go b/routers/api/v1/user/app.go index e512ba9e4b..6972931abc 100644 --- a/routers/api/v1/user/app.go +++ b/routers/api/v1/user/app.go @@ -43,8 +43,10 @@ func ListAccessTokens(ctx *context.APIContext) { // responses: // "200": // "$ref": "#/responses/AccessTokenList" + // "403": + // "$ref": "#/responses/forbidden" - opts := auth_model.ListAccessTokensOptions{UserID: ctx.Doer.ID, ListOptions: utils.GetListOptions(ctx)} + opts := auth_model.ListAccessTokensOptions{UserID: ctx.ContextUser.ID, ListOptions: utils.GetListOptions(ctx)} count, err := auth_model.CountAccessTokens(ctx, opts) if err != nil { @@ -95,11 +97,13 @@ func CreateAccessToken(ctx *context.APIContext) { // "$ref": "#/responses/AccessToken" // "400": // "$ref": "#/responses/error" + // "403": + // "$ref": "#/responses/forbidden" form := web.GetForm(ctx).(*api.CreateAccessTokenOption) t := &auth_model.AccessToken{ - UID: ctx.Doer.ID, + UID: ctx.ContextUser.ID, Name: form.Name, } @@ -153,6 +157,8 @@ func DeleteAccessToken(ctx *context.APIContext) { // responses: // "204": // "$ref": "#/responses/empty" + // "403": + // "$ref": "#/responses/forbidden" // "404": // "$ref": "#/responses/notFound" // "422": @@ -164,7 +170,7 @@ func DeleteAccessToken(ctx *context.APIContext) { if tokenID == 0 { tokens, err := auth_model.ListAccessTokens(ctx, auth_model.ListAccessTokensOptions{ Name: token, - UserID: ctx.Doer.ID, + UserID: ctx.ContextUser.ID, }) if err != nil { ctx.Error(http.StatusInternalServerError, "ListAccessTokens", err) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 88dc9ea1ce..39c5a7fe11 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -16359,6 +16359,9 @@ "responses": { "200": { "$ref": "#/responses/AccessTokenList" + }, + "403": { + "$ref": "#/responses/forbidden" } } }, @@ -16396,6 +16399,9 @@ }, "400": { "$ref": "#/responses/error" + }, + "403": { + "$ref": "#/responses/forbidden" } } } @@ -16430,6 +16436,9 @@ "204": { "$ref": "#/responses/empty" }, + "403": { + "$ref": "#/responses/forbidden" + }, "404": { "$ref": "#/responses/notFound" }, diff --git a/tests/integration/api_token_test.go b/tests/integration/api_token_test.go index 1c63d07f22..a713922982 100644 --- a/tests/integration/api_token_test.go +++ b/tests/integration/api_token_test.go @@ -40,6 +40,29 @@ func TestAPIDeleteMissingToken(t *testing.T) { MakeRequest(t, req, http.StatusNotFound) } +// TestAPIGetTokensPermission ensures that only the admin can get tokens from other users +func TestAPIGetTokensPermission(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + // admin can get tokens for other users + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + req := NewRequestf(t, "GET", "/api/v1/users/user2/tokens") + req = AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusOK) + + // non-admin can get tokens for himself + user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + req = NewRequestf(t, "GET", "/api/v1/users/user2/tokens") + req = AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusOK) + + // non-admin can't get tokens for other users + user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + req = NewRequestf(t, "GET", "/api/v1/users/user2/tokens") + req = AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusForbidden) +} + type permission struct { category auth_model.AccessTokenScopeCategory level auth_model.AccessTokenScopeLevel