Skip to content

Commit 303af55

Browse files
gsvdwxiaoguang
andauthored
Improve "generate new access token" form (#33730)
Fix: #33519 As discussed in [PR #33614](#33614), the ScopedAccessTokenSelector Vue component is not particularly useful. This PR removes the component and reverts to using HTML templates. It also introduces some (hopefully) useful refactoring. The Vue component was causing the UX bug reported in the linked issue. Required form fields are now properly working, as expected (see screenshot). ![Screenshot from 2025-02-25 22-00-28](https://github.com/user-attachments/assets/41167854-0718-48b0-a3ee-75ca3a7b8b20) --------- Co-authored-by: wxiaoguang <[email protected]>
1 parent 8362a41 commit 303af55

21 files changed

+138
-298
lines changed

models/auth/access_token_scope.go

+13-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package auth
55

66
import (
77
"fmt"
8+
"slices"
89
"strings"
910

1011
"code.gitea.io/gitea/models/perm"
@@ -14,7 +15,7 @@ import (
1415
type AccessTokenScopeCategory int
1516

1617
const (
17-
AccessTokenScopeCategoryActivityPub = iota
18+
AccessTokenScopeCategoryActivityPub AccessTokenScopeCategory = iota
1819
AccessTokenScopeCategoryAdmin
1920
AccessTokenScopeCategoryMisc // WARN: this is now just a placeholder, don't remove it which will change the following values
2021
AccessTokenScopeCategoryNotification
@@ -193,6 +194,14 @@ var accessTokenScopes = map[AccessTokenScopeLevel]map[AccessTokenScopeCategory]A
193194
},
194195
}
195196

197+
func GetAccessTokenCategories() (res []string) {
198+
for _, cat := range accessTokenScopes[Read] {
199+
res = append(res, strings.TrimPrefix(string(cat), "read:"))
200+
}
201+
slices.Sort(res)
202+
return res
203+
}
204+
196205
// GetRequiredScopes gets the specific scopes for a given level and categories
197206
func GetRequiredScopes(level AccessTokenScopeLevel, scopeCategories ...AccessTokenScopeCategory) []AccessTokenScope {
198207
scopes := make([]AccessTokenScope, 0, len(scopeCategories))
@@ -270,6 +279,9 @@ func (s AccessTokenScope) parse() (accessTokenScopeBitmap, error) {
270279

271280
// StringSlice returns the AccessTokenScope as a []string
272281
func (s AccessTokenScope) StringSlice() []string {
282+
if s == "" {
283+
return nil
284+
}
273285
return strings.Split(string(s), ",")
274286
}
275287

models/auth/access_token_scope_test.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ type scopeTestNormalize struct {
1717
}
1818

1919
func TestAccessTokenScope_Normalize(t *testing.T) {
20+
assert.Equal(t, []string{"activitypub", "admin", "issue", "misc", "notification", "organization", "package", "repository", "user"}, GetAccessTokenCategories())
2021
tests := []scopeTestNormalize{
2122
{"", "", nil},
2223
{"write:misc,write:notification,read:package,write:notification,public-only", "public-only,write:misc,write:notification,read:package", nil},
@@ -25,7 +26,7 @@ func TestAccessTokenScope_Normalize(t *testing.T) {
2526
{"write:activitypub,write:admin,write:misc,write:notification,write:organization,write:package,write:issue,write:repository,write:user,public-only", "public-only,all", nil},
2627
}
2728

28-
for _, scope := range []string{"activitypub", "admin", "misc", "notification", "organization", "package", "issue", "repository", "user"} {
29+
for _, scope := range GetAccessTokenCategories() {
2930
tests = append(tests,
3031
scopeTestNormalize{AccessTokenScope(fmt.Sprintf("read:%s", scope)), AccessTokenScope(fmt.Sprintf("read:%s", scope)), nil},
3132
scopeTestNormalize{AccessTokenScope(fmt.Sprintf("write:%s", scope)), AccessTokenScope(fmt.Sprintf("write:%s", scope)), nil},
@@ -59,7 +60,7 @@ func TestAccessTokenScope_HasScope(t *testing.T) {
5960
{"public-only", "read:issue", false, nil},
6061
}
6162

62-
for _, scope := range []string{"activitypub", "admin", "misc", "notification", "organization", "package", "issue", "repository", "user"} {
63+
for _, scope := range GetAccessTokenCategories() {
6364
tests = append(tests,
6465
scopeTestHasScope{
6566
AccessTokenScope(fmt.Sprintf("read:%s", scope)),

options/locale/locale_en-US.ini

-1
Original file line numberDiff line numberDiff line change
@@ -917,7 +917,6 @@ delete_token_success = The token has been deleted. Applications using it no long
917917
repo_and_org_access = Repository and Organization Access
918918
permissions_public_only = Public only
919919
permissions_access_all = All (public, private, and limited)
920-
select_permissions = Select permissions
921920
permission_not_set = Not set
922921
permission_no_access = No Access
923922
permission_read = Read

routers/web/user/setting/applications.go

+27-7
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@ package setting
66

77
import (
88
"net/http"
9+
"strings"
910

1011
auth_model "code.gitea.io/gitea/models/auth"
1112
"code.gitea.io/gitea/models/db"
1213
user_model "code.gitea.io/gitea/models/user"
1314
"code.gitea.io/gitea/modules/setting"
1415
"code.gitea.io/gitea/modules/templates"
16+
"code.gitea.io/gitea/modules/util"
1517
"code.gitea.io/gitea/modules/web"
1618
"code.gitea.io/gitea/services/context"
1719
"code.gitea.io/gitea/services/forms"
@@ -39,18 +41,29 @@ func ApplicationsPost(ctx *context.Context) {
3941
ctx.Data["PageIsSettingsApplications"] = true
4042
ctx.Data["UserDisabledFeatures"] = user_model.DisabledFeaturesWithLoginType(ctx.Doer)
4143

42-
if ctx.HasError() {
43-
loadApplicationsData(ctx)
44-
45-
ctx.HTML(http.StatusOK, tplSettingsApplications)
46-
return
44+
_ = ctx.Req.ParseForm()
45+
var scopeNames []string
46+
for k, v := range ctx.Req.Form {
47+
if strings.HasPrefix(k, "scope-") {
48+
scopeNames = append(scopeNames, v...)
49+
}
4750
}
4851

49-
scope, err := form.GetScope()
52+
scope, err := auth_model.AccessTokenScope(strings.Join(scopeNames, ",")).Normalize()
5053
if err != nil {
5154
ctx.ServerError("GetScope", err)
5255
return
5356
}
57+
if scope == "" || scope == auth_model.AccessTokenScopePublicOnly {
58+
ctx.Flash.Error(ctx.Tr("settings.at_least_one_permission"), true)
59+
}
60+
61+
if ctx.HasError() {
62+
loadApplicationsData(ctx)
63+
ctx.HTML(http.StatusOK, tplSettingsApplications)
64+
return
65+
}
66+
5467
t := &auth_model.AccessToken{
5568
UID: ctx.Doer.ID,
5669
Name: form.Name,
@@ -99,7 +112,14 @@ func loadApplicationsData(ctx *context.Context) {
99112
}
100113
ctx.Data["Tokens"] = tokens
101114
ctx.Data["EnableOAuth2"] = setting.OAuth2.Enabled
102-
ctx.Data["IsAdmin"] = ctx.Doer.IsAdmin
115+
116+
// Handle specific ordered token categories for admin or non-admin users
117+
tokenCategoryNames := auth_model.GetAccessTokenCategories()
118+
if !ctx.Doer.IsAdmin {
119+
tokenCategoryNames = util.SliceRemoveAll(tokenCategoryNames, "admin")
120+
}
121+
ctx.Data["TokenCategories"] = tokenCategoryNames
122+
103123
if setting.OAuth2.Enabled {
104124
ctx.Data["Applications"], err = db.Find[auth_model.OAuth2Application](ctx, auth_model.FindOAuth2ApplicationsOptions{
105125
OwnerID: ctx.Doer.ID,

services/context/context.go

+7-4
Original file line numberDiff line numberDiff line change
@@ -213,13 +213,16 @@ func Contexter() func(next http.Handler) http.Handler {
213213
// Attention: this function changes ctx.Data and ctx.Flash
214214
// If HasError is called, then before Redirect, the error message should be stored by ctx.Flash.Error(ctx.GetErrMsg()) again.
215215
func (ctx *Context) HasError() bool {
216-
hasErr, ok := ctx.Data["HasError"]
217-
if !ok {
216+
hasErr, _ := ctx.Data["HasError"].(bool)
217+
hasErr = hasErr || ctx.Flash.ErrorMsg != ""
218+
if !hasErr {
218219
return false
219220
}
220-
ctx.Flash.ErrorMsg = ctx.GetErrMsg()
221+
if ctx.Flash.ErrorMsg == "" {
222+
ctx.Flash.ErrorMsg = ctx.GetErrMsg()
223+
}
221224
ctx.Data["Flash"] = ctx.Flash
222-
return hasErr.(bool)
225+
return hasErr
223226
}
224227

225228
// GetErrMsg returns error message in form validation.

services/forms/user_form.go

+1-10
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@ package forms
77
import (
88
"mime/multipart"
99
"net/http"
10-
"strings"
1110

12-
auth_model "code.gitea.io/gitea/models/auth"
1311
user_model "code.gitea.io/gitea/models/user"
1412
"code.gitea.io/gitea/modules/structs"
1513
"code.gitea.io/gitea/modules/web/middleware"
@@ -347,8 +345,7 @@ func (f *EditVariableForm) Validate(req *http.Request, errs binding.Errors) bind
347345

348346
// NewAccessTokenForm form for creating access token
349347
type NewAccessTokenForm struct {
350-
Name string `binding:"Required;MaxSize(255)" locale:"settings.token_name"`
351-
Scope []string
348+
Name string `binding:"Required;MaxSize(255)" locale:"settings.token_name"`
352349
}
353350

354351
// Validate validates the fields
@@ -357,12 +354,6 @@ func (f *NewAccessTokenForm) Validate(req *http.Request, errs binding.Errors) bi
357354
return middleware.Validate(errs, ctx.Data, f, ctx.Locale)
358355
}
359356

360-
func (f *NewAccessTokenForm) GetScope() (auth_model.AccessTokenScope, error) {
361-
scope := strings.Join(f.Scope, ",")
362-
s, err := auth_model.AccessTokenScope(scope).Normalize()
363-
return s, err
364-
}
365-
366357
// EditOAuth2ApplicationForm form for editing oauth2 applications
367358
type EditOAuth2ApplicationForm struct {
368359
Name string `binding:"Required;MaxSize(255)" form:"application_name"`

services/forms/user_form_test.go

-27
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,8 @@
44
package forms
55

66
import (
7-
"strconv"
87
"testing"
98

10-
auth_model "code.gitea.io/gitea/models/auth"
119
"code.gitea.io/gitea/modules/setting"
1210

1311
"github.com/gobwas/glob"
@@ -104,28 +102,3 @@ func TestRegisterForm_IsDomainAllowed_BlockedEmail(t *testing.T) {
104102
assert.Equal(t, v.valid, form.IsEmailDomainAllowed())
105103
}
106104
}
107-
108-
func TestNewAccessTokenForm_GetScope(t *testing.T) {
109-
tests := []struct {
110-
form NewAccessTokenForm
111-
scope auth_model.AccessTokenScope
112-
expectedErr error
113-
}{
114-
{
115-
form: NewAccessTokenForm{Name: "test", Scope: []string{"read:repository"}},
116-
scope: "read:repository",
117-
},
118-
{
119-
form: NewAccessTokenForm{Name: "test", Scope: []string{"read:repository", "write:user"}},
120-
scope: "read:repository,write:user",
121-
},
122-
}
123-
124-
for i, test := range tests {
125-
t.Run(strconv.Itoa(i), func(t *testing.T) {
126-
scope, err := test.form.GetScope()
127-
assert.Equal(t, test.expectedErr, err)
128-
assert.Equal(t, test.scope, scope)
129-
})
130-
}
131-
}

templates/user/settings/applications.tmpl

+34-42
Original file line numberDiff line numberDiff line change
@@ -50,49 +50,41 @@
5050
</div>
5151
</div>
5252
<div class="ui bottom attached segment">
53-
<h5 class="ui top header">
54-
{{ctx.Locale.Tr "settings.generate_new_token"}}
55-
</h5>
56-
<form id="scoped-access-form" class="ui form ignore-dirty" action="{{.Link}}" method="post">
57-
{{.CsrfTokenHtml}}
58-
<div class="field {{if .Err_Name}}error{{end}}">
59-
<label for="name">{{ctx.Locale.Tr "settings.token_name"}}</label>
60-
<input id="name" name="name" value="{{.name}}" autofocus required maxlength="255">
61-
</div>
62-
<div class="field">
63-
<label>{{ctx.Locale.Tr "settings.repo_and_org_access"}}</label>
64-
<label class="tw-cursor-pointer">
65-
<input class="enable-system tw-mt-1 tw-mr-1" type="radio" name="scope" value="{{$.AccessTokenScopePublicOnly}}">
66-
{{ctx.Locale.Tr "settings.permissions_public_only"}}
67-
</label>
68-
<label class="tw-cursor-pointer">
69-
<input class="enable-system tw-mt-1 tw-mr-1" type="radio" name="scope" value="" checked>
70-
{{ctx.Locale.Tr "settings.permissions_access_all"}}
71-
</label>
72-
</div>
73-
<details class="ui optional field">
74-
<summary class="tw-pb-4 tw-pl-1">
75-
{{ctx.Locale.Tr "settings.select_permissions"}}
76-
</summary>
77-
<p class="activity meta">
78-
<i>{{ctx.Locale.Tr "settings.access_token_desc" (HTMLFormat `href="%s/api/swagger" target="_blank"` AppSubUrl) (`href="https://docs.gitea.com/development/oauth2-provider#scopes" target="_blank"`|SafeHTML)}}</i>
79-
</p>
80-
<div id="scoped-access-token-selector"
81-
data-is-admin="{{if .IsAdmin}}true{{else}}false{{end}}"
82-
data-no-access-label="{{ctx.Locale.Tr "settings.permission_no_access"}}"
83-
data-read-label="{{ctx.Locale.Tr "settings.permission_read"}}"
84-
data-write-label="{{ctx.Locale.Tr "settings.permission_write"}}"
85-
data-locale-component-failed-to-load="{{ctx.Locale.Tr "graphs.component_failed_to_load"}}"
86-
>
53+
<details {{if or .name (not .Tokens)}}open{{end}}>
54+
<summary><h4 class="ui header tw-inline-block tw-my-2">{{ctx.Locale.Tr "settings.generate_new_token"}}</h4></summary>
55+
<form class="ui form ignore-dirty" action="{{.Link}}" method="post">
56+
{{.CsrfTokenHtml}}
57+
<div class="field {{if .Err_Name}}error{{end}}">
58+
<label for="name">{{ctx.Locale.Tr "settings.token_name"}}</label>
59+
<input id="name" name="name" value="{{.name}}" required maxlength="255">
8760
</div>
88-
</details>
89-
<button id="scoped-access-submit" class="ui primary button">
90-
{{ctx.Locale.Tr "settings.generate_token"}}
91-
</button>
92-
</form>{{/* Fomantic ".ui.form .warning.message" is hidden by default, so put the warning message out of the form*/}}
93-
<div id="scoped-access-warning" class="ui warning message center tw-hidden">
94-
{{ctx.Locale.Tr "settings.at_least_one_permission"}}
95-
</div>
61+
<div class="field">
62+
<div class="tw-my-2">{{ctx.Locale.Tr "settings.repo_and_org_access"}}</div>
63+
<label class="gt-checkbox">
64+
<input type="radio" name="scope-public-only" value="{{$.AccessTokenScopePublicOnly}}"> {{ctx.Locale.Tr "settings.permissions_public_only"}}
65+
</label>
66+
<label class="gt-checkbox">
67+
<input type="radio" name="scope-public-only" value="" checked> {{ctx.Locale.Tr "settings.permissions_access_all"}}
68+
</label>
69+
</div>
70+
<div>
71+
<div class="tw-my-2">{{ctx.Locale.Tr "settings.access_token_desc" (HTMLFormat `href="%s/api/swagger" target="_blank"` AppSubUrl) (`href="https://docs.gitea.com/development/oauth2-provider#scopes" target="_blank"`|SafeHTML)}}</div>
72+
<table class="ui table unstackable tw-my-2">
73+
{{range $category := .TokenCategories}}
74+
<tr>
75+
<td>{{$category}}</td>
76+
<td><label class="gt-checkbox"><input type="radio" name="scope-{{$category}}" value="" checked> {{ctx.Locale.Tr "settings.permission_no_access"}}</label></td>
77+
<td><label class="gt-checkbox"><input type="radio" name="scope-{{$category}}" value="read:{{$category}}"> {{ctx.Locale.Tr "settings.permission_read"}}</label></td>
78+
<td><label class="gt-checkbox"><input type="radio" name="scope-{{$category}}" value="write:{{$category}}"> {{ctx.Locale.Tr "settings.permission_write"}}</label></td>
79+
</tr>
80+
{{end}}
81+
</table>
82+
</div>
83+
<button class="ui primary button">
84+
{{ctx.Locale.Tr "settings.generate_token"}}
85+
</button>
86+
</form>
87+
</details>
9688
</div>
9789

9890
{{if .EnableOAuth2}}

templates/user/settings/applications_oauth2_list.tmpl

+27-27
Original file line numberDiff line numberDiff line change
@@ -48,33 +48,33 @@
4848
</div>
4949

5050
<div class="ui bottom attached segment">
51-
<h5 class="ui top header">
52-
{{ctx.Locale.Tr "settings.create_oauth2_application"}}
53-
</h5>
54-
<form class="ui form ignore-dirty" action="{{.Link}}/oauth2" method="post">
55-
{{.CsrfTokenHtml}}
56-
<div class="field {{if .Err_AppName}}error{{end}}">
57-
<label for="application-name">{{ctx.Locale.Tr "settings.oauth2_application_name"}}</label>
58-
<input id="application-name" name="application_name" value="{{.application_name}}" required maxlength="255">
59-
</div>
60-
<div class="field {{if .Err_RedirectURI}}error{{end}}">
61-
<label for="redirect-uris">{{ctx.Locale.Tr "settings.oauth2_redirect_uris"}}</label>
62-
<textarea name="redirect_uris" id="redirect-uris"></textarea>
63-
</div>
64-
<div class="field {{if .Err_ConfidentialClient}}error{{end}}">
65-
<div class="ui checkbox">
66-
<label>{{ctx.Locale.Tr "settings.oauth2_confidential_client"}}</label>
67-
<input class="disable-setting" type="checkbox" name="confidential_client" data-target="#skip-secondary-authorization" checked>
51+
<details {{if .application_name}}open{{end}}>
52+
<summary><h4 class="ui header tw-inline-block tw-my-2">{{ctx.Locale.Tr "settings.create_oauth2_application"}}</h4></summary>
53+
<form class="ui form ignore-dirty" action="{{.Link}}/oauth2" method="post">
54+
{{.CsrfTokenHtml}}
55+
<div class="field {{if .Err_AppName}}error{{end}}">
56+
<label for="application-name">{{ctx.Locale.Tr "settings.oauth2_application_name"}}</label>
57+
<input id="application-name" name="application_name" value="{{.application_name}}" required maxlength="255">
6858
</div>
69-
</div>
70-
<div class="field {{if .Err_SkipSecondaryAuthorization}}error{{end}} disabled" id="skip-secondary-authorization">
71-
<div class="ui checkbox">
72-
<label>{{ctx.Locale.Tr "settings.oauth2_skip_secondary_authorization"}}</label>
73-
<input type="checkbox" name="skip_secondary_authorization">
59+
<div class="field {{if .Err_RedirectURI}}error{{end}}">
60+
<label for="redirect-uris">{{ctx.Locale.Tr "settings.oauth2_redirect_uris"}}</label>
61+
<textarea name="redirect_uris" id="redirect-uris"></textarea>
7462
</div>
75-
</div>
76-
<button class="ui primary button">
77-
{{ctx.Locale.Tr "settings.create_oauth2_application_button"}}
78-
</button>
79-
</form>
63+
<div class="field {{if .Err_ConfidentialClient}}error{{end}}">
64+
<div class="ui checkbox">
65+
<label>{{ctx.Locale.Tr "settings.oauth2_confidential_client"}}</label>
66+
<input class="disable-setting" type="checkbox" name="confidential_client" data-target="#skip-secondary-authorization" checked>
67+
</div>
68+
</div>
69+
<div class="field {{if .Err_SkipSecondaryAuthorization}}error{{end}} disabled" id="skip-secondary-authorization">
70+
<div class="ui checkbox">
71+
<label>{{ctx.Locale.Tr "settings.oauth2_skip_secondary_authorization"}}</label>
72+
<input type="checkbox" name="skip_secondary_authorization">
73+
</div>
74+
</div>
75+
<button class="ui primary button">
76+
{{ctx.Locale.Tr "settings.create_oauth2_application_button"}}
77+
</button>
78+
</form>
79+
</details>
8080
</div>

tests/integration/api_admin_org_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func TestAPIAdminOrgCreateNotAdmin(t *testing.T) {
7676
defer tests.PrepareTestEnv(t)()
7777
nonAdminUsername := "user2"
7878
session := loginUser(t, nonAdminUsername)
79-
token := getTokenForLoggedInUser(t, session)
79+
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeAll)
8080
org := api.CreateOrgOption{
8181
UserName: "user2_org",
8282
FullName: "User2's organization",

0 commit comments

Comments
 (0)