diff --git a/models/repo/repo_list.go b/models/repo/repo_list.go index 71ec8362f2..732d7b627c 100644 --- a/models/repo/repo_list.go +++ b/models/repo/repo_list.go @@ -188,6 +188,9 @@ type SearchRepoOptions struct { OnlyShowRelevant bool // Filters repositories based upon optional authorization restrictions. AuthorizationReducer RepositoryAuthorizationReducer + // Retrieve multiple repositories by their owner name & repository name, similar to [GetRepositoryByOwnerAndName] + // but in bulk. + OwnerAndName [][2]string } // UserOwnedRepoCond returns user ownered repositories @@ -495,6 +498,26 @@ func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond { cond = cond.And(opts.AuthorizationReducer.RepoReadAccessFilter()) } + if opts.OwnerAndName != nil { + if len(opts.OwnerAndName) > 0 { + // repository is indexed on `(owner_id, lower_name)`, but not on the `owner_name` field. Plus the `owner_name` + // field isn't ToLower'd. So this becomes a subquery: + subQuery := builder.Select("inner_repo.id").From("repository", "inner_repo"). + Join("INNER", "`user`", "`user`.id = inner_repo.owner_id") + for _, ownerAndName := range opts.OwnerAndName { + subQuery.Or(builder.Eq{ + "`user`.lower_name": strings.ToLower(ownerAndName[0]), + "inner_repo.lower_name": strings.ToLower(ownerAndName[1]), + }) + } + cond = cond.And(builder.In("id", subQuery)) + } else { + // If opts.OwnerAndName is a non-nil, empty array, then we want to return zero repositories. The loop to + // build the `Eq` conditions wouldn't occur, so we would have no filtering if this wasn't special-case'd. + cond = cond.And(builder.Eq{"1": "2"}) + } + } + return cond } diff --git a/models/repo/repo_list_test.go b/models/repo/repo_list_test.go index dda9687414..fc32226975 100644 --- a/models/repo/repo_list_test.go +++ b/models/repo/repo_list_test.go @@ -179,6 +179,26 @@ func getTestCases() []struct { opts: &repo_model.SearchRepoOptions{Keyword: "user20/", ListOptions: db.ListOptions{Page: 1, PageSize: 10}, Private: true, OwnerID: 0}, count: 4, }, + { + name: "OwnerAndName Single", + opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerAndName: [][2]string{{"user15", "big_test_public_1"}}}, + count: 1, + }, + { + name: "OwnerAndName Multiple", + opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerAndName: [][2]string{{"user15", "big_test_public_1"}, {"user15", "big_test_public_2"}}}, + count: 2, + }, + { + name: "OwnerAndName Miss", + opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerAndName: [][2]string{{"user15", "big_test_public_1"}, {"user15", "blah blah"}}}, + count: 1, + }, + { + name: "OwnerAndName Empty", + opts: &repo_model.SearchRepoOptions{ListOptions: db.ListOptions{Page: 1, PageSize: 10}, OwnerAndName: [][2]string{}}, + count: 0, + }, } return testCases diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index c6b65f8c84..67767b1985 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -193,6 +193,19 @@ "settings.must_enable_2fa": "This Forgejo instance requires users to enable two-factor authentication before they can access their accounts.", "settings.specific_repo_access": "Repository access", "settings.new_access_token": "New access token", + "settings.permissions_access_specific_repositories": "Specific repositories", + "settings.access_token.selected_repositories": { + "one": "Selected repository (%d)", + "other": "Selected repositories (%d)" + }, + "settings.access_token.available_repositories": "Available repositories", + "settings.access_token.no_repositories_selected": "No repositories selected.", + "settings.access_token.no_repositories_found": "No repositories found.", + "settings.access_token.remove": "Remove %s", + "settings.access_token.resource_all_help": "Allow access to all resources.", + "settings.access_token.resource_public_only_help": "Limit access to repositories and organizations that are public.", + "settings.access_token.resource_specific_repo_help": "Limit access to a specific list of repositories. Read-only access is permitted to all public repositories. Only permissions that allow access to repositories and issues can be enabled.", + "settings.access_token.admin_disabled": "Administrative permissions are disabled.", "error.must_enable_2fa": "This Forgejo instance requires users to enable two-factor authentication before they can access their accounts. Enable it at: %s", "avatar.constraints_hint": "Custom avatar may not exceed %[1]s in size or be larger than %[2]dx%[3]d pixels", "user.ghost.tooltip": "This user has been deleted, or cannot be matched.", diff --git a/routers/api/v1/user/app.go b/routers/api/v1/user/app.go index 2532396dca..8d8ead9234 100644 --- a/routers/api/v1/user/app.go +++ b/routers/api/v1/user/app.go @@ -18,10 +18,10 @@ import ( "forgejo.org/models/db" access_model "forgejo.org/models/perm/access" repo_model "forgejo.org/models/repo" - "forgejo.org/modules/optional" api "forgejo.org/modules/structs" "forgejo.org/modules/web" "forgejo.org/routers/api/v1/utils" + "forgejo.org/routers/web/shared/user" "forgejo.org/services/authz" "forgejo.org/services/context" "forgejo.org/services/convert" @@ -115,19 +115,6 @@ func ListAccessTokens(ctx *context.APIContext) { ctx.JSON(http.StatusOK, &apiTokens) } -func translateAccessTokenValidationError(ctx *context.Base, err error) optional.Option[string] { - switch { - case errors.Is(err, authz.ErrSpecifiedReposNone): - return optional.Some[string](ctx.Locale.TrString("access_token.error.specified_repos_none")) - case errors.Is(err, authz.ErrSpecifiedReposNoPublicOnly): - return optional.Some[string](ctx.Locale.TrString("access_token.error.specified_repos_and_public_only")) - case errors.Is(err, authz.ErrSpecifiedReposInvalidScope): - return optional.Some[string](ctx.Locale.TrString("access_token.error.specified_repos_and_invalid_scope")) - default: - return optional.None[string]() - } -} - // CreateAccessToken creates an access token func CreateAccessToken(ctx *context.APIContext) { // swagger:operation POST /users/{username}/tokens user userCreateToken @@ -228,7 +215,7 @@ func CreateAccessToken(ctx *context.APIContext) { } if err := authz.ValidateAccessToken(t, resourceRepos); err != nil { - s := translateAccessTokenValidationError(ctx.Base, err) + s := user.TranslateAccessTokenValidationError(ctx.Base, err) if has, str := s.Get(); has { ctx.Error(http.StatusBadRequest, "ValidateAccessToken", str) return diff --git a/routers/web/shared/user/access_token.go b/routers/web/shared/user/access_token.go new file mode 100644 index 0000000000..942054905b --- /dev/null +++ b/routers/web/shared/user/access_token.go @@ -0,0 +1,28 @@ +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package user + +import ( + "errors" + + "forgejo.org/modules/optional" + "forgejo.org/services/authz" + "forgejo.org/services/context" +) + +// Translate errors from [ValidateAccessToken] into user-visible messages, if the error is expected as a user-facing +// error. None[string] may be returned to indicate the error is an unexpected server-side error, not a user validation +// error. +func TranslateAccessTokenValidationError(ctx *context.Base, err error) optional.Option[string] { + switch { + case errors.Is(err, authz.ErrSpecifiedReposNone): + return optional.Some[string](ctx.Locale.TrString("access_token.error.specified_repos_none")) + case errors.Is(err, authz.ErrSpecifiedReposNoPublicOnly): + return optional.Some[string](ctx.Locale.TrString("access_token.error.specified_repos_and_public_only")) + case errors.Is(err, authz.ErrSpecifiedReposInvalidScope): + return optional.Some[string](ctx.Locale.TrString("access_token.error.specified_repos_and_invalid_scope")) + default: + return optional.None[string]() + } +} diff --git a/routers/web/user/setting/access_token.go b/routers/web/user/setting/access_token.go index 293bb19456..cdaad287a8 100644 --- a/routers/web/user/setting/access_token.go +++ b/routers/web/user/setting/access_token.go @@ -1,26 +1,84 @@ // Copyright 2014 The Gogs Authors. All rights reserved. // Copyright 2018 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later package setting import ( + stdCtx "context" + "fmt" + "html/template" "net/http" "slices" + "strings" auth_model "forgejo.org/models/auth" + "forgejo.org/models/db" + access_model "forgejo.org/models/perm/access" + repo_model "forgejo.org/models/repo" "forgejo.org/modules/base" "forgejo.org/modules/log" + "forgejo.org/modules/optional" "forgejo.org/modules/setting" "forgejo.org/modules/web" + "forgejo.org/routers/web/shared/user" + "forgejo.org/services/authz" "forgejo.org/services/context" "forgejo.org/services/forms" + + "xorm.io/builder" ) const ( tplAccessTokenEdit base.TplName = "user/settings/access_token_edit" ) +func getSelectedRepos(ctx *context.Context, selectedReposRaw []string) []*repo_model.Repository { + ownerAndName := make([][2]string, len(selectedReposRaw)) + for i, selected := range selectedReposRaw { + split := strings.SplitN(selected, "/", 2) // ownername/reponame + if len(split) != 2 { + ctx.Error(http.StatusBadRequest, fmt.Sprintf("invalid selected_repo: %s", selected)) + return nil + } + ownerAndName[i] = [2]string{split[0], split[1]} + } + + repoSearch := &repo_model.SearchRepoOptions{ + OwnerAndName: ownerAndName, + OrderBy: db.SearchOrderByAlphabetically, // match sorting in loadAccessTokenCreateData for consistency + Private: true, + } + + cond := repo_model.SearchRepositoryCondition(repoSearch) + repos, _, err := repo_model.SearchRepositoryByCondition(ctx, repoSearch, cond, false) + if err != nil { + ctx.ServerError("SearchRepositoryByCondition", err) + return nil + } else if len(repos) != len(selectedReposRaw) { + // One or more of the repositories couldn't be found by search by owner & name. + ctx.Error(http.StatusBadRequest, "GetRepositoryByOwnerAndName") // keep in sync w/ !permission.HasAccess below + return nil + } + + selectedRepos := make([]*repo_model.Repository, len(selectedReposRaw)) + for i, repo := range repos { + permission, err := access_model.GetUserRepoPermission(ctx, repo, ctx.Doer) + if err != nil { + ctx.ServerError("GetUserRepoPermission", err) + return nil + } else if !permission.HasAccess() { + // Prevent data existence probing -- ensure this error is the exact same as the (len(repos) != + // len(selectedReposRaw)) case above + ctx.Error(http.StatusBadRequest, "GetRepositoryByOwnerAndName") + return nil + } + selectedRepos[i] = repo + } + return selectedRepos +} + func loadAccessTokenCreateData(ctx *context.Context) { ctx.Data["AccessTokenScopePublicOnly"] = string(auth_model.AccessTokenScopePublicOnly) // note: SliceUtils.Contains won't work in the template if this is a `auth_model.AccessTokenScope`, so it's cast to a string here @@ -39,6 +97,113 @@ func loadAccessTokenCreateData(ctx *context.Context) { } slices.Sort(categories) ctx.Data["Categories"] = categories + + // Awkward -- GET and POST use different form bindings for the reasons explained on NewAccessTokenGetForm -- and + // this method can be called in both situations, on all GETs, and on some POSTs when validation errors occur. So + // both forms need to be handled here. + getForm, isGet := web.GetForm(ctx).(*forms.NewAccessTokenGetForm) + postForm, isPost := web.GetForm(ctx).(*forms.NewAccessTokenPostForm) + + if isGet { + // Manage the result of adding or removing a repository before we do anything with `form.SelectedRepo`... + changed := false + if getForm.AddSelectedRepo != "" { + getForm.SelectedRepo = append(getForm.SelectedRepo, getForm.AddSelectedRepo) + changed = true + } + if getForm.RemoveSelectedRepo != "" { + getForm.SelectedRepo = slices.DeleteFunc( + getForm.SelectedRepo, + func(r string) bool { return r == getForm.RemoveSelectedRepo }, + ) + changed = true + } + if changed { + // We've changed getForm.SelectedRepo, but a reference to this slice was already present in `ctx.Data` (the + // Bind middleware invokes AssignForm to put getForm values into `ctx.Data`). Replace the reference: + ctx.Data["selected_repo"] = getForm.SelectedRepo + } + } + + repoSearchText := "" + if isGet { + repoSearchText = getForm.RepoSearch + } + + selectedReposRaw := []string{} + if isGet { + selectedReposRaw = getForm.SelectedRepo + } else if isPost { + selectedReposRaw = postForm.SelectedRepo + } + selectedRepos := getSelectedRepos(ctx, selectedReposRaw) + if ctx.Written() { + return + } + ctx.Data["SelectedRepos"] = selectedRepos + + page := 1 + if isGet { + // Pagination on the repo search has form submit buttons that send the `set_page` param. It's then encoded into + // the page in the hidden input `page` which we fall back to, if anything else causes a form get (eg. adding or + // removing a repo). + if getForm.SetPage > 0 { + page = getForm.SetPage + } else if getForm.Page > 0 { + page = getForm.Page + } + } + pageSize := 10 + repoSearch := &repo_model.SearchRepoOptions{ + Actor: ctx.Doer, + Keyword: repoSearchText, + Private: true, + Archived: optional.Some(false), + + // Restrict repositories to those owned by, or collaborated with, by the user. Repo-specific access tokens + // could theoretically be created on any public repository as well, but there wouldn't be much point to that and + // it would really balloon the search results to an impractical number of repos. + OwnerID: ctx.Doer.ID, + + ListOptions: db.ListOptions{ + Page: page, + PageSize: pageSize, + }, + OrderBy: db.SearchOrderByAlphabetically, // match sorting in getSelectedRepos for consistency + } + cond := repo_model.SearchRepositoryCondition(repoSearch) + // Exclude all the repos that are currently in `form.SelectedRepo` from the search, by omitting them from the search + // condition. This prevents the UI from displaying the same repo on the left and right, and maintains the repo + // search and page-size correctly. + for _, selected := range selectedRepos { + cond = cond.And(builder.Neq{"id": selected.ID}) + } + repos, count, err := repo_model.SearchRepositoryByCondition(ctx, repoSearch, cond, false) + if err != nil { + log.Error("SearchRepository: %v", err) + ctx.JSON(http.StatusInternalServerError, nil) + return + } + ctx.Data["Repos"] = repos + + pager := context.NewPagination(int(count), pageSize, page, 3) + pager.SetDefaultParams(ctx) + ctx.Data["Page"] = pager + + autofocus := "" + if isGet { + switch { + // Token name will be autofocused the first time the page is loaded -- if form.Scope is empty then that would be + // a good sign it's the first load. + case len(getForm.Scope) == 0: + autofocus = "name" + // After submitting a search, refocus the search text box. Search invokes set_page=1 to reset the pagination + // which we'll use to detect this case. + case getForm.SetPage == 1: + autofocus = "search" + } + } + ctx.Data["Autofocus"] = autofocus } // Applications render manage access token page @@ -47,18 +212,32 @@ func AccessTokenCreate(ctx *context.Context) { ctx.Data["PageIsSettingsApplications"] = true loadAccessTokenCreateData(ctx) + if ctx.Written() { + return + } ctx.HTML(http.StatusOK, tplAccessTokenEdit) } // ApplicationsPost response for add user's access token func AccessTokenCreatePost(ctx *context.Context) { - form := web.GetForm(ctx).(*forms.NewAccessTokenForm) + form := web.GetForm(ctx).(*forms.NewAccessTokenPostForm) ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsApplications"] = true + renderWithError := func(msg template.HTML) { + loadAccessTokenCreateData(ctx) + if ctx.Written() { + return + } + ctx.RenderWithErr(msg, tplAccessTokenEdit, form) + } + if ctx.HasError() { loadAccessTokenCreateData(ctx) + if ctx.Written() { + return + } ctx.HTML(http.StatusOK, tplAccessTokenEdit) return } @@ -69,32 +248,66 @@ func AccessTokenCreatePost(ctx *context.Context) { return } if !scope.HasPermissionScope() { - loadAccessTokenCreateData(ctx) - ctx.RenderWithErr(ctx.Tr("settings.at_least_one_permission"), tplAccessTokenEdit, form) + renderWithError(ctx.Tr("settings.at_least_one_permission")) return } + t := &auth_model.AccessToken{ UID: ctx.Doer.ID, Name: form.Name, Scope: scope, + } - // maintain legacy behaviour until new UI options are added -- token has access to all resources, is not - // fine-grained - ResourceAllRepos: true, + var resourceRepos []*auth_model.AccessTokenResourceRepo + switch form.Resource { + case "all": + t.ResourceAllRepos = true + case "public-only": + t.ResourceAllRepos = true + newScopeUnnormalized := fmt.Sprintf("%s,%s", scope, auth_model.AccessTokenScopePublicOnly) + newScope, err := auth_model.AccessTokenScope(newScopeUnnormalized).Normalize() + if err != nil { + ctx.ServerError("AccessTokenScope.Normalize", err) + return + } + t.Scope = newScope + case "repo-specific": + t.ResourceAllRepos = false + selectedRepos := getSelectedRepos(ctx, form.SelectedRepo) + if ctx.Written() { + return + } + for _, repo := range selectedRepos { + resourceRepos = append(resourceRepos, &auth_model.AccessTokenResourceRepo{RepoID: repo.ID}) + } } exist, err := auth_model.AccessTokenByNameExists(ctx, t) if err != nil { ctx.ServerError("AccessTokenByNameExists", err) return - } - if exist { - loadAccessTokenCreateData(ctx) - ctx.RenderWithErr(ctx.Tr("settings.generate_token_name_duplicate", t.Name), tplAccessTokenEdit, form) + } else if exist { + renderWithError(ctx.Tr("settings.generate_token_name_duplicate", t.Name)) return } - if err := auth_model.NewAccessToken(ctx, t); err != nil { + if err := authz.ValidateAccessToken(t, resourceRepos); err != nil { + s := user.TranslateAccessTokenValidationError(ctx.Base, err) + if has, str := s.Get(); has { + renderWithError(template.HTML(template.HTMLEscapeString(str))) + return + } + ctx.ServerError("ValidateAccessToken", err) + return + } + + err = db.WithTx(ctx, func(ctx stdCtx.Context) error { + if err := auth_model.NewAccessToken(ctx, t); err != nil { + return err + } + return auth_model.InsertAccessTokenResourceRepos(ctx, t.ID, resourceRepos) + }) + if err != nil { ctx.ServerError("NewAccessToken", err) return } diff --git a/routers/web/user/setting/access_token_test.go b/routers/web/user/setting/access_token_test.go new file mode 100644 index 0000000000..21d27a7992 --- /dev/null +++ b/routers/web/user/setting/access_token_test.go @@ -0,0 +1,159 @@ +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package setting + +import ( + "net/http" + "net/http/httptest" + "slices" + "testing" + + repo_model "forgejo.org/models/repo" + "forgejo.org/models/unittest" + "forgejo.org/modules/templates" + "forgejo.org/modules/web" + "forgejo.org/services/context" + "forgejo.org/services/contexttest" + "forgejo.org/services/forms" + + "code.forgejo.org/go-chi/binding" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAccessTokenCreate(t *testing.T) { + unittest.PrepareTestEnv(t) + + ctx, resp := contexttest.MockContext(t, "user/settings/applications/tokens/new", + contexttest.MockContextOption{Render: templates.HTMLRenderer()}) + contexttest.LoadUser(t, ctx, 2) + + web.SetForm(ctx, &forms.NewAccessTokenGetForm{}) + AccessTokenCreate(ctx) + + assert.Equal(t, http.StatusOK, resp.Result().StatusCode) + assert.False(t, ctx.HasError(), "error: %s", ctx.GetErrMsg()) + + // check empty-form GET ctx.Data values + assert.Equal(t, "name", ctx.Data["Autofocus"]) + assert.Len(t, ctx.Data["Repos"], 10) + assert.Empty(t, ctx.Data["SelectedRepos"]) + + // check that repo in the search is rendered in the content + assert.Contains(t, resp.Body.String(), "org17/big_test_private_4") +} + +func TestAccessTokenCreatePost(t *testing.T) { + unittest.PrepareTestEnv(t) + + post := func(t *testing.T, form *forms.NewAccessTokenPostForm) (*context.Context, *httptest.ResponseRecorder) { + t.Helper() + + ctx, resp := contexttest.MockContext(t, "user/settings/applications/tokens/new", + contexttest.MockContextOption{Render: templates.HTMLRenderer()}) + contexttest.LoadUser(t, ctx, 2) + + ctx.AppendContextValue(context.WebContextKey, ctx) + binding.Bind(ctx.Req.WithContext(ctx), form) + web.SetForm(ctx, form) + AccessTokenCreatePost(ctx) + + return ctx, resp + } + + render := func(t *testing.T, form *forms.NewAccessTokenPostForm) (*context.Context, string) { + t.Helper() + ctx, resp := post(t, form) + assert.Equal(t, http.StatusOK, resp.Result().StatusCode) + return ctx, resp.Body.String() + } + + t.Run("retains form info on missing token name", func(t *testing.T) { + ctx, body := render(t, &forms.NewAccessTokenPostForm{ + Name: "", // absent + Resource: "repo-specific", + Scope: []string{"read:repository"}, + SelectedRepo: []string{"org17/big_test_private_4"}, + }) + require.Contains(t, body, "settings.token_nameform.require_error") + + assert.Equal(t, "repo-specific", ctx.Data["resource"]) + assert.Contains(t, ctx.Data["scope"], "read:repository") + assert.True(t, slices.ContainsFunc(ctx.Data["SelectedRepos"].([]*repo_model.Repository), func(r *repo_model.Repository) bool { + return r.OwnerName == "org17" && r.Name == "big_test_private_4" + }), "SelectedRepos missing org17/big_test_private_4") + }) + + t.Run("retains form info on missing scopes", func(t *testing.T) { + ctx, body := render(t, &forms.NewAccessTokenPostForm{ + Name: "my new token", + Resource: "repo-specific", + Scope: []string{"", "", ""}, // absent + SelectedRepo: []string{"org17/big_test_private_4"}, + }) + require.Contains(t, body, "settings.at_least_one_permission") + + assert.Equal(t, "my new token", ctx.Data["name"]) + assert.Equal(t, "repo-specific", ctx.Data["resource"]) + assert.True(t, slices.ContainsFunc(ctx.Data["SelectedRepos"].([]*repo_model.Repository), func(r *repo_model.Repository) bool { + return r.OwnerName == "org17" && r.Name == "big_test_private_4" + }), "SelectedRepos missing org17/big_test_private_4") + }) + + t.Run("retains form info on duplicate token name", func(t *testing.T) { + ctx, body := render(t, &forms.NewAccessTokenPostForm{ + Name: "Token A", // duplicate + Resource: "repo-specific", + Scope: []string{"read:repository"}, + SelectedRepo: []string{"org17/big_test_private_4"}, + }) + require.Contains(t, body, "settings.generate_token_name_duplicate") + + assert.Equal(t, "Token A", ctx.Data["name"]) + assert.Equal(t, "repo-specific", ctx.Data["resource"]) + assert.Contains(t, ctx.Data["scope"], "read:repository") + assert.True(t, slices.ContainsFunc(ctx.Data["SelectedRepos"].([]*repo_model.Repository), func(r *repo_model.Repository) bool { + return r.OwnerName == "org17" && r.Name == "big_test_private_4" + }), "SelectedRepos missing org17/big_test_private_4") + }) + + t.Run("retains form info on ValidateAccessToken error", func(t *testing.T) { + ctx, body := render(t, &forms.NewAccessTokenPostForm{ + Name: "my new token", + Resource: "repo-specific", + Scope: []string{"read:admin"}, // not permitted for repo-specific + SelectedRepo: []string{"org17/big_test_private_4"}, + }) + require.Contains(t, body, "access_token.error.specified_repos_and_invalid_scope") + + assert.Equal(t, "my new token", ctx.Data["name"]) + assert.Equal(t, "repo-specific", ctx.Data["resource"]) + assert.Contains(t, ctx.Data["scope"], "read:admin") + assert.True(t, slices.ContainsFunc(ctx.Data["SelectedRepos"].([]*repo_model.Repository), func(r *repo_model.Repository) bool { + return r.OwnerName == "org17" && r.Name == "big_test_private_4" + }), "SelectedRepos missing org17/big_test_private_4") + }) + + t.Run("invalid repo selected", func(t *testing.T) { + _, resp := post(t, &forms.NewAccessTokenPostForm{ + Name: "my new token", + Resource: "repo-specific", + Scope: []string{"read:admin"}, // not permitted for repo-specific + SelectedRepo: []string{"org17/big_test_private_4000000_does_not_exist"}, + }) + assert.Equal(t, http.StatusBadRequest, resp.Result().StatusCode) + assert.Equal(t, "GetRepositoryByOwnerAndName\n", resp.Body.String()) + }) + + t.Run("non-visible repo selected via IDOR", func(t *testing.T) { + _, resp := post(t, &forms.NewAccessTokenPostForm{ + Name: "my new token", + Resource: "repo-specific", + Scope: []string{"read:repository"}, + SelectedRepo: []string{"user30/empty"}, // private repo, user2 has no visibility + }) + assert.Equal(t, http.StatusBadRequest, resp.Result().StatusCode) + assert.Equal(t, "GetRepositoryByOwnerAndName\n", resp.Body.String()) // should be exact same response as "invalid repo selected" case + }) +} diff --git a/routers/web/web.go b/routers/web/web.go index ac5474e7ed..7463bf3ea2 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -635,8 +635,8 @@ func registerRoutes(m *web.Route) { // access token m.Group("/tokens", func() { m.Combo("/new"). - Get(user_setting.AccessTokenCreate). - Post(web.Bind(forms.NewAccessTokenForm{}), user_setting.AccessTokenCreatePost) + Get(web.Bind(forms.NewAccessTokenGetForm{}), user_setting.AccessTokenCreate). + Post(web.Bind(forms.NewAccessTokenPostForm{}), user_setting.AccessTokenCreatePost) m.Post("/delete", user_setting.DeleteAccessToken) m.Post("/regenerate", user_setting.RegenerateAccessToken) }) diff --git a/services/forms/user_form.go b/services/forms/user_form.go index 1c6ba58058..971177fcd0 100644 --- a/services/forms/user_form.go +++ b/services/forms/user_form.go @@ -356,19 +356,44 @@ func (f *EditVariableForm) Validate(req *http.Request, errs binding.Errors) bind return middleware.Validate(errs, ctx.Data, f, ctx.Locale) } -// NewAccessTokenForm form for creating access token -type NewAccessTokenForm struct { - Name string `binding:"Required;MaxSize(255)" locale:"settings.token_name"` - Scope []string +// NewAccessTokenGetForm form for creating access token. Similar to NewAccessTokenPostForm, but contains some data that +// is only part of the GET requests as the SelectedRepo field is built up interactively, and, it also removes the +// Required binding to allow this to be used on a formless GET request without displaying an initial error. +type NewAccessTokenGetForm struct { + Name string `binding:"MaxSize(255)" locale:"settings.token_name"` + Resource string // all, public-only, repo-specific + Scope []string + SelectedRepo []string // slice of ownername/reponame + + // Transient form values, not part of the final data for the access token form + RepoSearch string + AddSelectedRepo string // add a repo to SelectedRepo + RemoveSelectedRepo string // remove a repo from SelectedRepo + Page int // repo search page + SetPage int // repo search buttons } // Validate validates the fields -func (f *NewAccessTokenForm) Validate(req *http.Request, errs binding.Errors) binding.Errors { +func (f *NewAccessTokenGetForm) Validate(req *http.Request, errs binding.Errors) binding.Errors { ctx := context.GetValidateContext(req) return middleware.Validate(errs, ctx.Data, f, ctx.Locale) } -func (f *NewAccessTokenForm) GetScope() (auth_model.AccessTokenScope, error) { +// NewAccessTokenPostForm form for creating access token +type NewAccessTokenPostForm struct { + Name string `binding:"Required;MaxSize(255)" locale:"settings.token_name"` + Resource string `binding:"Required" locale:"settings.repo_and_org_access"` // all, public-only, repo-specific + Scope []string + SelectedRepo []string // slice of ownername/reponame +} + +// Validate validates the fields +func (f *NewAccessTokenPostForm) Validate(req *http.Request, errs binding.Errors) binding.Errors { + ctx := context.GetValidateContext(req) + return middleware.Validate(errs, ctx.Data, f, ctx.Locale) +} + +func (f *NewAccessTokenPostForm) GetScope() (auth_model.AccessTokenScope, error) { scope := strings.Join(f.Scope, ",") s, err := auth_model.AccessTokenScope(scope).Normalize() return s, err diff --git a/services/forms/user_form_test.go b/services/forms/user_form_test.go index 4ce63ab552..a52597184a 100644 --- a/services/forms/user_form_test.go +++ b/services/forms/user_form_test.go @@ -91,16 +91,16 @@ func TestRegisterForm_IsDomainAllowed_BlockedEmail(t *testing.T) { func TestNewAccessTokenForm_GetScope(t *testing.T) { tests := []struct { - form NewAccessTokenForm + form NewAccessTokenPostForm scope auth_model.AccessTokenScope expectedErr error }{ { - form: NewAccessTokenForm{Name: "test", Scope: []string{"read:repository"}}, + form: NewAccessTokenPostForm{Name: "test", Scope: []string{"read:repository"}}, scope: "read:repository", }, { - form: NewAccessTokenForm{Name: "test", Scope: []string{"read:repository", "write:user"}}, + form: NewAccessTokenPostForm{Name: "test", Scope: []string{"read:repository", "write:user"}}, scope: "read:repository,write:user", }, } diff --git a/templates/user/settings/access_token_edit.tmpl b/templates/user/settings/access_token_edit.tmpl index c53fafeb1a..5caf42b3e7 100644 --- a/templates/user/settings/access_token_edit.tmpl +++ b/templates/user/settings/access_token_edit.tmpl @@ -1,63 +1,180 @@ {{template "user/settings/layout_head" (dict "ctxData" . "pageClass" "user settings applications")}} -
-

- {{ctx.Locale.Tr "settings.manage_access_token"}} -

+
+

+ {{ctx.Locale.Tr "settings.generate_new_token"}} +

-
-
-
- {{ctx.Locale.Tr "settings.generate_new_token"}} -
-
- - -
-
+
+ +
+ + +
+
+
- - -
-
-
- {{ctx.Locale.Tr "settings.select_permissions"}} -
-

-

{{ctx.Locale.Tr "settings.access_token_desc" (printf "%s/api/swagger" AppSubUrl) "https://forgejo.org/docs/latest/user/token-scope/"}}

-

- - {{range .Categories}} -
- -
- -
+
+
+ + +

{{ctx.Locale.Tr "settings.access_token.resource_all_help"}}

- {{end}} -
- - -
+
+
+
+ + +

+ {{ctx.Locale.Tr "settings.access_token.resource_public_only_help"}} + {{if $.IsAdmin}}{{ctx.Locale.Tr "settings.access_token.admin_disabled"}}{{end}} +

+
+
+
+
+ + +

+ {{ctx.Locale.Tr "settings.access_token.resource_specific_repo_help"}} + {{if $.IsAdmin}}{{ctx.Locale.Tr "settings.access_token.admin_disabled"}}{{end}} +

+
+
+ +
+
+
+ +
+
+ {{ctx.Locale.Tr "settings.access_token.available_repositories"}} +
+ +
+ + {{svg "octicon-search" 16}} + +
+ + {{if eq (len .Repos) 0}} + {{ctx.Locale.Tr "settings.access_token.no_repositories_found"}} + {{else}} +
+ {{range .Repos}} + {{template "user/settings/repo_icon" .}} +
+ {{.FullName}} +
+ + {{end}} +
+ {{end}} + + {{/* Can't use base/paginate template include here because all the pagination links in + that template are simple links. Here, we need to turn them into form + buttons so that we can submit the current form. If a user just changed a value (eg. set + the token name, changed a selected permission) and then clicked a pagination button, the + new value that they changed needs to be submitted. base/paginate would allow preserving + old values from before the change, but not new updates. Implementing here also allows + the use of smaller styling. */}} + {{with .Page.Paginater}} + +
+ +
+ {{end}} +
+ + +
+
{{ctx.Locale.TrPluralString (len .SelectedRepos) "settings.access_token.selected_repositories" (len .SelectedRepos)}}
+ {{if eq (len .SelectedRepos) 0}} +
+ {{ctx.Locale.Tr "settings.access_token.no_repositories_selected"}} +
+ {{else}} +
+ {{range .SelectedRepos}} + + {{template "user/settings/repo_icon" .}} +
+ {{.FullName}} +
+ + {{end}} +
+ {{end}} +
+ +
+
+
+ +

+

{{ctx.Locale.Tr "settings.access_token_desc" (printf "%s/api/swagger" AppSubUrl) "https://forgejo.org/docs/latest/user/token-scope/"}}

+

+ + {{range .Categories}} +
+ +
+ +
+
+ {{end}} +
+ +
+ +
{{template "user/settings/layout_footer" .}} diff --git a/templates/user/settings/repo_icon.tmpl b/templates/user/settings/repo_icon.tmpl new file mode 100644 index 0000000000..553ce57bd7 --- /dev/null +++ b/templates/user/settings/repo_icon.tmpl @@ -0,0 +1,9 @@ +{{$repoIcon := "octicon-repo"}} +{{if .IsFork}} + {{$repoIcon = "octicon-repo-forked"}} +{{else if .IsMirror}} + {{$repoIcon = "octicon-mirror"}} +{{else if .IsPrivate}} + {{$repoIcon = "octicon-lock"}} +{{end}} +{{svg $repoIcon 16 "repo-list-icon tw-mr-2"}} diff --git a/tests/e2e/user-settings.test.e2e.ts b/tests/e2e/user-settings.test.e2e.ts index 9a6baefef5..1c52b5f9e1 100644 --- a/tests/e2e/user-settings.test.e2e.ts +++ b/tests/e2e/user-settings.test.e2e.ts @@ -141,8 +141,12 @@ test('User: Add access token', async ({browser}, workerInfo) => { const tokenName = globalThis.crypto.randomUUID(); await page.locator('#name').fill(tokenName); + await page.getByRole('radio', {name: /^All /}).click(); await page.locator('#scoped-access-submit').click(); + await expect(page.locator('.ui.info.message.flash-info')).toBeVisible(); + const flashText = await page.locator('.ui.info.message.flash-info').textContent(); + expect(flashText?.trim()).toMatch(/^[0-9a-f]{40}$/); await page.getByText(tokenName).isVisible(); }); @@ -161,7 +165,75 @@ test('User: Add access token validation error', async ({browser}, workerInfo) => await page.getByRole('button', {name: 'Generate token'}).click(); await page.getByText('has been used as an application name already.').isVisible(); - // validate that selected options (public-only, activitypub) are still selected. + // validate that selected options (public-only, activitypub) are still selected after the validation error. await expect(page.getByRole('radio', {name: 'Public only'})).toBeChecked(); await expect(page.getByRole('combobox', {name: 'activitypub'})).toHaveValue('read:activitypub'); }); + +test('User: Add specific repo access token', async ({browser}, workerInfo) => { + const page = await login({browser}, workerInfo); + await page.goto('/user/settings/applications'); + await page.getByRole('link', {name: 'New access token'}).click(); + + const tokenName = globalThis.crypto.randomUUID(); + await page.getByRole('textbox', {name: /^Token name/}).fill(tokenName); + await page.getByRole('combobox', {name: 'repository'}).selectOption('read:repository'); + + // clicking specific repositories will display currently available repositories: + await expect(page.getByText('org17/big_test_private_4')).toBeHidden(); + await page.getByRole('radio', {name: 'Specific repositories'}).click(); + await expect(page.getByText('org17/big_test_private_4')).toBeVisible(); + await expect(page.getByText('user2/commits_search_test')).toBeVisible(); // another repo, will be used to verify search worked + + await page.getByPlaceholder('Search repos…').fill('big_test_private_4'); + await page.getByRole('button', {name: 'Search…'}).click(); + + // verify search results visible: + await expect(page.getByText('org17/big_test_private_4')).toBeVisible(); + await expect(page.getByText('user2/commits_search_test')).toBeHidden(); + + // after performing a search, verify that the token name, 'selected repositories', and selected permissions are maintained + await expect(page.getByRole('textbox', {name: /^Token name/})).toHaveValue(tokenName); + await expect(page.getByRole('radio', {name: 'Specific repositories'})).toBeChecked(); + await expect(page.getByRole('combobox', {name: 'repository'})).toHaveValue('read:repository'); + + // Add the big_test_private_4 repo. + await page.getByRole('button', {name: 'Add org17/big_test_private_4'}).click(); + await expect(page.getByText('Selected repository (1)')).toBeVisible(); + await expect(page.getByText('org17/big_test_private_4')).toBeVisible(); + + // Remove it to test remove, and then re-add + await page.getByRole('button', {name: 'Remove org17/big_test_private_4'}).click(); + await expect(page.getByText('Selected repositories (0)')).toBeVisible(); + await expect(page.getByText('org17/big_test_private_4')).toBeVisible(); + await page.getByRole('button', {name: 'Add org17/big_test_private_4'}).click(); + + // Create the token and check for success. + await page.getByRole('button', {name: 'Generate token'}).click(); + await expect(page.locator('.ui.info.message.flash-info')).toBeVisible(); + const flashText = await page.locator('.ui.info.message.flash-info').textContent(); + expect(flashText?.trim()).toMatch(/^[0-9a-f]{40}$/); + await page.getByText(tokenName).isVisible(); +}); + +// Test that validation errors on the repo-specific access token page retain all the entered field values when the +// error is displayed. +test('User: Add specific repo access token error', async ({browser}, workerInfo) => { + const page = await login({browser}, workerInfo); + await page.goto('/user/settings/applications'); + await page.getByRole('link', {name: 'New access token'}).click(); + + await page.getByRole('textbox', {name: /^Token name/}).fill('Token A'); + await page.getByRole('combobox', {name: 'repository'}).selectOption('read:repository'); + await page.getByRole('radio', {name: 'Specific repositories'}).click(); + await page.getByRole('button', {name: 'Add org17/big_test_private_4'}).click(); + + // Create the token, verify error, then check all the fields for retained values. + await page.getByRole('button', {name: 'Generate token'}).click(); + await page.getByText('has been used as an application name already.').isVisible(); + + await expect(page.getByRole('textbox', {name: /^Token name/})).toHaveValue('Token A'); + await expect(page.getByRole('radio', {name: 'Specific repositories'})).toBeChecked(); + await expect(page.getByRole('combobox', {name: 'repository'})).toHaveValue('read:repository'); + await expect(page.getByRole('button', {name: 'Remove org17/big_test_private_4'})).toBeVisible(); +}); diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 464008e6e4..8657af839c 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -491,8 +491,18 @@ func createApplicationSettingsToken(t testing.TB, session *TestSession, name str urlValues := url.Values{} urlValues.Add("name", name) + publicOnly := false for _, scope := range scopes { - urlValues.Add("scope", string(scope)) + if scope == auth.AccessTokenScopePublicOnly { + publicOnly = true + } else { + urlValues.Add("scope", string(scope)) + } + } + if publicOnly { + urlValues.Add("resource", "public-only") + } else { + urlValues.Add("resource", "all") } req := NewRequestWithURLValues(t, "POST", "/user/settings/applications/tokens/new", urlValues) resp := session.MakeRequest(t, req, http.StatusSeeOther) diff --git a/web_src/css/user.css b/web_src/css/user.css index 6f58dceb66..71ac2b364b 100644 --- a/web_src/css/user.css +++ b/web_src/css/user.css @@ -141,3 +141,30 @@ .notifications-item:hover .notifications-updated { display: none; } + +/* user's access token edit page; /user/settings/applications/tokens/new; custom CSS for repo selector visibility */ + +/* #repo-selector-wrapper hides and shows the repository selection based upon the radio-button value, with an animation +to hint the connection between the two to the user. */ +#repo-selector-wrapper { + display: grid; + grid-template-rows: 0fr; + transition: grid-template-rows 0.3s ease-out; +} +body:has(input[name="resource"][value="repo-specific"]:checked) #repo-selector-wrapper { + grid-template-rows: 1fr; + transition: grid-template-rows 0.3s ease-in; +} + +/* Accessibility -- remove from the visibility tree when hidden so that it doesn't appear for screenreaders. Downside of +this is that the contents of the selector disappear during the ease-in animation, but it's barely noticable. The +expansion animation is the important one anyway, as it associates the selector box with the selection just made for the +user. */ +#repo-selector { + overflow: hidden; /* needed so that the grid-template-rows=0fr clips the selector out */ + visibility: hidden; +} + +body:has(input[name="resource"][value="repo-specific"]:checked) #repo-selector { + visibility: visible; +}