Skip to content

Commit a8d0c87

Browse files
denyskondenyskon
authored
add skip secondary authorization option for public oauth2 clients (#31454)
1 parent e9aa39b commit a8d0c87

File tree

15 files changed

+120
-62
lines changed

15 files changed

+120
-62
lines changed

‎models/auth/oauth2.go

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,11 @@ type OAuth2Application struct {
3737
// https://datatracker.ietf.org/doc/html/rfc6749#section-2.1
3838
// "Authorization servers MUST record the client type in the client registration details"
3939
// https://datatracker.ietf.org/doc/html/rfc8252#section-8.4
40-
ConfidentialClient bool `xorm:"NOT NULL DEFAULT TRUE"`
41-
RedirectURIs []string `xorm:"redirect_uris JSON TEXT"`
42-
CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
43-
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
40+
ConfidentialClient bool `xorm:"NOT NULL DEFAULT TRUE"`
41+
SkipSecondaryAuthorization bool `xorm:"NOT NULL DEFAULT FALSE"`
42+
RedirectURIs []string `xorm:"redirect_uris JSON TEXT"`
43+
CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
44+
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
4445
}
4546

4647
func init() {
@@ -251,21 +252,23 @@ func GetOAuth2ApplicationByID(ctx context.Context, id int64) (app *OAuth2Applica
251252

252253
// CreateOAuth2ApplicationOptions holds options to create an oauth2 application
253254
type CreateOAuth2ApplicationOptions struct {
254-
Name string
255-
UserID int64
256-
ConfidentialClient bool
257-
RedirectURIs []string
255+
Name string
256+
UserID int64
257+
ConfidentialClient bool
258+
SkipSecondaryAuthorization bool
259+
RedirectURIs []string
258260
}
259261

260262
// CreateOAuth2Application inserts a new oauth2 application
261263
func CreateOAuth2Application(ctx context.Context, opts CreateOAuth2ApplicationOptions) (*OAuth2Application, error) {
262264
clientID := uuid.New().String()
263265
app := &OAuth2Application{
264-
UID: opts.UserID,
265-
Name: opts.Name,
266-
ClientID: clientID,
267-
RedirectURIs: opts.RedirectURIs,
268-
ConfidentialClient: opts.ConfidentialClient,
266+
UID: opts.UserID,
267+
Name: opts.Name,
268+
ClientID: clientID,
269+
RedirectURIs: opts.RedirectURIs,
270+
ConfidentialClient: opts.ConfidentialClient,
271+
SkipSecondaryAuthorization: opts.SkipSecondaryAuthorization,
269272
}
270273
if err := db.Insert(ctx, app); err != nil {
271274
return nil, err
@@ -275,11 +278,12 @@ func CreateOAuth2Application(ctx context.Context, opts CreateOAuth2ApplicationOp
275278

276279
// UpdateOAuth2ApplicationOptions holds options to update an oauth2 application
277280
type UpdateOAuth2ApplicationOptions struct {
278-
ID int64
279-
Name string
280-
UserID int64
281-
ConfidentialClient bool
282-
RedirectURIs []string
281+
ID int64
282+
Name string
283+
UserID int64
284+
ConfidentialClient bool
285+
SkipSecondaryAuthorization bool
286+
RedirectURIs []string
283287
}
284288

285289
// UpdateOAuth2Application updates an oauth2 application
@@ -305,6 +309,7 @@ func UpdateOAuth2Application(ctx context.Context, opts UpdateOAuth2ApplicationOp
305309
app.Name = opts.Name
306310
app.RedirectURIs = opts.RedirectURIs
307311
app.ConfidentialClient = opts.ConfidentialClient
312+
app.SkipSecondaryAuthorization = opts.SkipSecondaryAuthorization
308313

309314
if err = updateOAuth2Application(ctx, app); err != nil {
310315
return nil, err
@@ -315,7 +320,7 @@ func UpdateOAuth2Application(ctx context.Context, opts UpdateOAuth2ApplicationOp
315320
}
316321

317322
func updateOAuth2Application(ctx context.Context, app *OAuth2Application) error {
318-
if _, err := db.GetEngine(ctx).ID(app.ID).UseBool("confidential_client").Update(app); err != nil {
323+
if _, err := db.GetEngine(ctx).ID(app.ID).UseBool("confidential_client", "skip_secondary_authorization").Update(app); err != nil {
319324
return err
320325
}
321326
return nil

‎models/migrations/migrations.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,8 @@ var migrations = []Migration{
593593
NewMigration("Add content version to issue and comment table", v1_23.AddContentVersionToIssueAndComment),
594594
// v300 -> v301
595595
NewMigration("Add force-push branch protection support", v1_23.AddForcePushBranchProtection),
596+
// v301 -> v302
597+
NewMigration("Add skip_secondary_authorization option to oauth2 application table", v1_23.AddSkipSecondaryAuthColumnToOAuth2ApplicationTable),
596598
}
597599

598600
// GetCurrentDBVersion returns the current db version

‎models/migrations/v1_23/v301.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package v1_23 //nolint
5+
6+
import "xorm.io/xorm"
7+
8+
// AddSkipSeconderyAuthToOAuth2ApplicationTable: add SkipSecondaryAuthorization column, setting existing rows to false
9+
func AddSkipSecondaryAuthColumnToOAuth2ApplicationTable(x *xorm.Engine) error {
10+
type oauth2Application struct {
11+
SkipSecondaryAuthorization bool `xorm:"NOT NULL DEFAULT FALSE"`
12+
}
13+
return x.Sync(new(oauth2Application))
14+
}

‎modules/structs/user_app.go

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,23 @@ type CreateAccessTokenOption struct {
3131

3232
// CreateOAuth2ApplicationOptions holds options to create an oauth2 application
3333
type CreateOAuth2ApplicationOptions struct {
34-
Name string `json:"name" binding:"Required"`
35-
ConfidentialClient bool `json:"confidential_client"`
36-
RedirectURIs []string `json:"redirect_uris" binding:"Required"`
34+
Name string `json:"name" binding:"Required"`
35+
ConfidentialClient bool `json:"confidential_client"`
36+
SkipSecondaryAuthorization bool `json:"skip_secondary_authorization"`
37+
RedirectURIs []string `json:"redirect_uris" binding:"Required"`
3738
}
3839

3940
// OAuth2Application represents an OAuth2 application.
4041
// swagger:response OAuth2Application
4142
type OAuth2Application struct {
42-
ID int64 `json:"id"`
43-
Name string `json:"name"`
44-
ClientID string `json:"client_id"`
45-
ClientSecret string `json:"client_secret"`
46-
ConfidentialClient bool `json:"confidential_client"`
47-
RedirectURIs []string `json:"redirect_uris"`
48-
Created time.Time `json:"created"`
43+
ID int64 `json:"id"`
44+
Name string `json:"name"`
45+
ClientID string `json:"client_id"`
46+
ClientSecret string `json:"client_secret"`
47+
ConfidentialClient bool `json:"confidential_client"`
48+
SkipSecondaryAuthorization bool `json:"skip_secondary_authorization"`
49+
RedirectURIs []string `json:"redirect_uris"`
50+
Created time.Time `json:"created"`
4951
}
5052

5153
// OAuth2ApplicationList represents a list of OAuth2 applications.

‎options/locale/locale_en-US.ini

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -914,6 +914,7 @@ create_oauth2_application_success = You have successfully created a new OAuth2 a
914914
update_oauth2_application_success = You have successfully updated the OAuth2 application.
915915
oauth2_application_name = Application Name
916916
oauth2_confidential_client = Confidential Client. Select for apps that keep the secret confidential, such as web apps. Do not select for native apps including desktop and mobile apps.
917+
oauth2_skip_secondary_authorization = Skip authorization for public clients after granting access once. <strong>May pose a security risk.</strong>
917918
oauth2_redirect_uris = Redirect URIs. Please use a new line for every URI.
918919
save_application = Save
919920
oauth2_client_id = Client ID

‎routers/api/v1/user/app.go

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -223,10 +223,11 @@ func CreateOauth2Application(ctx *context.APIContext) {
223223
data := web.GetForm(ctx).(*api.CreateOAuth2ApplicationOptions)
224224

225225
app, err := auth_model.CreateOAuth2Application(ctx, auth_model.CreateOAuth2ApplicationOptions{
226-
Name: data.Name,
227-
UserID: ctx.Doer.ID,
228-
RedirectURIs: data.RedirectURIs,
229-
ConfidentialClient: data.ConfidentialClient,
226+
Name: data.Name,
227+
UserID: ctx.Doer.ID,
228+
RedirectURIs: data.RedirectURIs,
229+
ConfidentialClient: data.ConfidentialClient,
230+
SkipSecondaryAuthorization: data.SkipSecondaryAuthorization,
230231
})
231232
if err != nil {
232233
ctx.Error(http.StatusBadRequest, "", "error creating oauth2 application")
@@ -381,11 +382,12 @@ func UpdateOauth2Application(ctx *context.APIContext) {
381382
data := web.GetForm(ctx).(*api.CreateOAuth2ApplicationOptions)
382383

383384
app, err := auth_model.UpdateOAuth2Application(ctx, auth_model.UpdateOAuth2ApplicationOptions{
384-
Name: data.Name,
385-
UserID: ctx.Doer.ID,
386-
ID: appID,
387-
RedirectURIs: data.RedirectURIs,
388-
ConfidentialClient: data.ConfidentialClient,
385+
Name: data.Name,
386+
UserID: ctx.Doer.ID,
387+
ID: appID,
388+
RedirectURIs: data.RedirectURIs,
389+
ConfidentialClient: data.ConfidentialClient,
390+
SkipSecondaryAuthorization: data.SkipSecondaryAuthorization,
389391
})
390392
if err != nil {
391393
if auth_model.IsErrOauthClientIDInvalid(err) || auth_model.IsErrOAuthApplicationNotFound(err) {

‎routers/web/auth/oauth.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -469,9 +469,9 @@ func AuthorizeOAuth(ctx *context.Context) {
469469
return
470470
}
471471

472-
// Redirect if user already granted access and the application is confidential.
473-
// I.e. always require authorization for public clients as recommended by RFC 6749 Section 10.2
474-
if app.ConfidentialClient && grant != nil {
472+
// Redirect if user already granted access and the application is confidential or trusted otherwise
473+
// I.e. always require authorization for untrusted public clients as recommended by RFC 6749 Section 10.2
474+
if (app.ConfidentialClient || app.SkipSecondaryAuthorization) && grant != nil {
475475
code, err := grant.GenerateNewAuthorizationCode(ctx, form.RedirectURI, form.CodeChallenge, form.CodeChallengeMethod)
476476
if err != nil {
477477
handleServerError(ctx, form.State, form.RedirectURI)

‎routers/web/user/setting/oauth2_common.go

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,11 @@ func (oa *OAuth2CommonHandlers) AddApp(ctx *context.Context) {
4949

5050
// TODO validate redirect URI
5151
app, err := auth.CreateOAuth2Application(ctx, auth.CreateOAuth2ApplicationOptions{
52-
Name: form.Name,
53-
RedirectURIs: util.SplitTrimSpace(form.RedirectURIs, "\n"),
54-
UserID: oa.OwnerID,
55-
ConfidentialClient: form.ConfidentialClient,
52+
Name: form.Name,
53+
RedirectURIs: util.SplitTrimSpace(form.RedirectURIs, "\n"),
54+
UserID: oa.OwnerID,
55+
ConfidentialClient: form.ConfidentialClient,
56+
SkipSecondaryAuthorization: form.SkipSecondaryAuthorization,
5657
})
5758
if err != nil {
5859
ctx.ServerError("CreateOAuth2Application", err)
@@ -102,11 +103,12 @@ func (oa *OAuth2CommonHandlers) EditSave(ctx *context.Context) {
102103
// TODO validate redirect URI
103104
var err error
104105
if ctx.Data["App"], err = auth.UpdateOAuth2Application(ctx, auth.UpdateOAuth2ApplicationOptions{
105-
ID: ctx.PathParamInt64("id"),
106-
Name: form.Name,
107-
RedirectURIs: util.SplitTrimSpace(form.RedirectURIs, "\n"),
108-
UserID: oa.OwnerID,
109-
ConfidentialClient: form.ConfidentialClient,
106+
ID: ctx.PathParamInt64("id"),
107+
Name: form.Name,
108+
RedirectURIs: util.SplitTrimSpace(form.RedirectURIs, "\n"),
109+
UserID: oa.OwnerID,
110+
ConfidentialClient: form.ConfidentialClient,
111+
SkipSecondaryAuthorization: form.SkipSecondaryAuthorization,
110112
}); err != nil {
111113
ctx.ServerError("UpdateOAuth2Application", err)
112114
return

‎services/convert/convert.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -455,13 +455,14 @@ func ToTopicResponse(topic *repo_model.Topic) *api.TopicResponse {
455455
// ToOAuth2Application convert from auth.OAuth2Application to api.OAuth2Application
456456
func ToOAuth2Application(app *auth.OAuth2Application) *api.OAuth2Application {
457457
return &api.OAuth2Application{
458-
ID: app.ID,
459-
Name: app.Name,
460-
ClientID: app.ClientID,
461-
ClientSecret: app.ClientSecret,
462-
ConfidentialClient: app.ConfidentialClient,
463-
RedirectURIs: app.RedirectURIs,
464-
Created: app.CreatedUnix.AsTime(),
458+
ID: app.ID,
459+
Name: app.Name,
460+
ClientID: app.ClientID,
461+
ClientSecret: app.ClientSecret,
462+
ConfidentialClient: app.ConfidentialClient,
463+
SkipSecondaryAuthorization: app.SkipSecondaryAuthorization,
464+
RedirectURIs: app.RedirectURIs,
465+
Created: app.CreatedUnix.AsTime(),
465466
}
466467
}
467468

‎services/forms/user_form.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -365,9 +365,10 @@ func (f *NewAccessTokenForm) GetScope() (auth_model.AccessTokenScope, error) {
365365

366366
// EditOAuth2ApplicationForm form for editing oauth2 applications
367367
type EditOAuth2ApplicationForm struct {
368-
Name string `binding:"Required;MaxSize(255)" form:"application_name"`
369-
RedirectURIs string `binding:"Required" form:"redirect_uris"`
370-
ConfidentialClient bool `form:"confidential_client"`
368+
Name string `binding:"Required;MaxSize(255)" form:"application_name"`
369+
RedirectURIs string `binding:"Required" form:"redirect_uris"`
370+
ConfidentialClient bool `form:"confidential_client"`
371+
SkipSecondaryAuthorization bool `form:"skip_secondary_authorization"`
371372
}
372373

373374
// Validate validates the fields

‎templates/swagger/v1_json.tmpl

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎templates/user/settings/applications_oauth2_edit_form.tmpl

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,13 @@
4444
<div class="field {{if .Err_ConfidentialClient}}error{{end}}">
4545
<div class="ui checkbox">
4646
<label>{{ctx.Locale.Tr "settings.oauth2_confidential_client"}}</label>
47-
<input type="checkbox" name="confidential_client" {{if .App.ConfidentialClient}}checked{{end}}>
47+
<input class="disable-setting" type="checkbox" name="confidential_client" data-target="#skip-secondary-authorization" {{if .App.ConfidentialClient}}checked{{end}}>
48+
</div>
49+
</div>
50+
<div class="field {{if .Err_SkipSecondaryAuthorization}}error{{end}} {{if .App.ConfidentialClient}}disabled{{end}}" id="skip-secondary-authorization">
51+
<div class="ui checkbox">
52+
<label>{{ctx.Locale.Tr "settings.oauth2_skip_secondary_authorization"}}</label>
53+
<input type="checkbox" name="skip_secondary_authorization" {{if .App.SkipSecondaryAuthorization}}checked{{end}}>
4854
</div>
4955
</div>
5056
<button class="ui primary button">

‎templates/user/settings/applications_oauth2_list.tmpl

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,13 @@
6464
<div class="field {{if .Err_ConfidentialClient}}error{{end}}">
6565
<div class="ui checkbox">
6666
<label>{{ctx.Locale.Tr "settings.oauth2_confidential_client"}}</label>
67-
<input type="checkbox" name="confidential_client" checked>
67+
<input class="disable-setting" type="checkbox" name="confidential_client" data-target="#skip-secondary-authorization" checked>
68+
</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">
6874
</div>
6975
</div>
7076
<button class="ui primary button">

‎web_src/js/features/oauth2-settings.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
export function initOAuth2SettingsDisableCheckbox() {
2+
for (const e of document.querySelectorAll('.disable-setting')) e.addEventListener('change', ({target}) => {
3+
document.querySelector(e.getAttribute('data-target')).classList.toggle('disabled', target.checked);
4+
});
5+
}

‎web_src/js/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ import {initDirAuto} from './modules/dirauto.ts';
7878
import {initRepositorySearch} from './features/repo-search.ts';
7979
import {initColorPickers} from './features/colorpicker.ts';
8080
import {initAdminSelfCheck} from './features/admin/selfcheck.ts';
81+
import {initOAuth2SettingsDisableCheckbox} from './features/oauth2-settings.ts';
8182
import {initGlobalFetchAction} from './features/common-fetch-action.ts';
8283
import {
8384
initFootLanguageMenu,
@@ -225,5 +226,7 @@ onDomReady(() => {
225226
initPdfViewer,
226227
initScopedAccessTokenCategories,
227228
initColorPickers,
229+
230+
initOAuth2SettingsDisableCheckbox,
228231
]);
229232
});

0 commit comments

Comments
 (0)