From a8d0c879c38e21a8e78db627119bf622d919ee75 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Fri, 19 Jul 2024 14:28:30 -0400 Subject: [PATCH] add skip secondary authorization option for public oauth2 clients (#31454) --- models/auth/oauth2.go | 43 +++++++++++-------- models/migrations/migrations.go | 2 + models/migrations/v1_23/v301.go | 14 ++++++ modules/structs/user_app.go | 22 +++++----- options/locale/locale_en-US.ini | 1 + routers/api/v1/user/app.go | 20 +++++---- routers/web/auth/oauth.go | 6 +-- routers/web/user/setting/oauth2_common.go | 20 +++++---- services/convert/convert.go | 15 ++++--- services/forms/user_form.go | 7 +-- templates/swagger/v1_json.tmpl | 8 ++++ .../applications_oauth2_edit_form.tmpl | 8 +++- .../settings/applications_oauth2_list.tmpl | 8 +++- web_src/js/features/oauth2-settings.ts | 5 +++ web_src/js/index.ts | 3 ++ 15 files changed, 120 insertions(+), 62 deletions(-) create mode 100644 models/migrations/v1_23/v301.go create mode 100644 web_src/js/features/oauth2-settings.ts diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index 7dca378e5d..c270e4856e 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -37,10 +37,11 @@ type OAuth2Application struct { // https://datatracker.ietf.org/doc/html/rfc6749#section-2.1 // "Authorization servers MUST record the client type in the client registration details" // https://datatracker.ietf.org/doc/html/rfc8252#section-8.4 - ConfidentialClient bool `xorm:"NOT NULL DEFAULT TRUE"` - RedirectURIs []string `xorm:"redirect_uris JSON TEXT"` - CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` - UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + ConfidentialClient bool `xorm:"NOT NULL DEFAULT TRUE"` + SkipSecondaryAuthorization bool `xorm:"NOT NULL DEFAULT FALSE"` + RedirectURIs []string `xorm:"redirect_uris JSON TEXT"` + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` } func init() { @@ -251,21 +252,23 @@ func GetOAuth2ApplicationByID(ctx context.Context, id int64) (app *OAuth2Applica // CreateOAuth2ApplicationOptions holds options to create an oauth2 application type CreateOAuth2ApplicationOptions struct { - Name string - UserID int64 - ConfidentialClient bool - RedirectURIs []string + Name string + UserID int64 + ConfidentialClient bool + SkipSecondaryAuthorization bool + RedirectURIs []string } // CreateOAuth2Application inserts a new oauth2 application func CreateOAuth2Application(ctx context.Context, opts CreateOAuth2ApplicationOptions) (*OAuth2Application, error) { clientID := uuid.New().String() app := &OAuth2Application{ - UID: opts.UserID, - Name: opts.Name, - ClientID: clientID, - RedirectURIs: opts.RedirectURIs, - ConfidentialClient: opts.ConfidentialClient, + UID: opts.UserID, + Name: opts.Name, + ClientID: clientID, + RedirectURIs: opts.RedirectURIs, + ConfidentialClient: opts.ConfidentialClient, + SkipSecondaryAuthorization: opts.SkipSecondaryAuthorization, } if err := db.Insert(ctx, app); err != nil { return nil, err @@ -275,11 +278,12 @@ func CreateOAuth2Application(ctx context.Context, opts CreateOAuth2ApplicationOp // UpdateOAuth2ApplicationOptions holds options to update an oauth2 application type UpdateOAuth2ApplicationOptions struct { - ID int64 - Name string - UserID int64 - ConfidentialClient bool - RedirectURIs []string + ID int64 + Name string + UserID int64 + ConfidentialClient bool + SkipSecondaryAuthorization bool + RedirectURIs []string } // UpdateOAuth2Application updates an oauth2 application @@ -305,6 +309,7 @@ func UpdateOAuth2Application(ctx context.Context, opts UpdateOAuth2ApplicationOp app.Name = opts.Name app.RedirectURIs = opts.RedirectURIs app.ConfidentialClient = opts.ConfidentialClient + app.SkipSecondaryAuthorization = opts.SkipSecondaryAuthorization if err = updateOAuth2Application(ctx, app); err != nil { return nil, err @@ -315,7 +320,7 @@ func UpdateOAuth2Application(ctx context.Context, opts UpdateOAuth2ApplicationOp } func updateOAuth2Application(ctx context.Context, app *OAuth2Application) error { - if _, err := db.GetEngine(ctx).ID(app.ID).UseBool("confidential_client").Update(app); err != nil { + if _, err := db.GetEngine(ctx).ID(app.ID).UseBool("confidential_client", "skip_secondary_authorization").Update(app); err != nil { return err } return nil diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 88faa6ba8b..0e13e89f00 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -593,6 +593,8 @@ var migrations = []Migration{ NewMigration("Add content version to issue and comment table", v1_23.AddContentVersionToIssueAndComment), // v300 -> v301 NewMigration("Add force-push branch protection support", v1_23.AddForcePushBranchProtection), + // v301 -> v302 + NewMigration("Add skip_secondary_authorization option to oauth2 application table", v1_23.AddSkipSecondaryAuthColumnToOAuth2ApplicationTable), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_23/v301.go b/models/migrations/v1_23/v301.go new file mode 100644 index 0000000000..b7797f6c6b --- /dev/null +++ b/models/migrations/v1_23/v301.go @@ -0,0 +1,14 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_23 //nolint + +import "xorm.io/xorm" + +// AddSkipSeconderyAuthToOAuth2ApplicationTable: add SkipSecondaryAuthorization column, setting existing rows to false +func AddSkipSecondaryAuthColumnToOAuth2ApplicationTable(x *xorm.Engine) error { + type oauth2Application struct { + SkipSecondaryAuthorization bool `xorm:"NOT NULL DEFAULT FALSE"` + } + return x.Sync(new(oauth2Application)) +} diff --git a/modules/structs/user_app.go b/modules/structs/user_app.go index 7f78fbd495..a7d2e28b41 100644 --- a/modules/structs/user_app.go +++ b/modules/structs/user_app.go @@ -31,21 +31,23 @@ type CreateAccessTokenOption struct { // CreateOAuth2ApplicationOptions holds options to create an oauth2 application type CreateOAuth2ApplicationOptions struct { - Name string `json:"name" binding:"Required"` - ConfidentialClient bool `json:"confidential_client"` - RedirectURIs []string `json:"redirect_uris" binding:"Required"` + Name string `json:"name" binding:"Required"` + ConfidentialClient bool `json:"confidential_client"` + SkipSecondaryAuthorization bool `json:"skip_secondary_authorization"` + RedirectURIs []string `json:"redirect_uris" binding:"Required"` } // OAuth2Application represents an OAuth2 application. // swagger:response OAuth2Application type OAuth2Application struct { - ID int64 `json:"id"` - Name string `json:"name"` - ClientID string `json:"client_id"` - ClientSecret string `json:"client_secret"` - ConfidentialClient bool `json:"confidential_client"` - RedirectURIs []string `json:"redirect_uris"` - Created time.Time `json:"created"` + ID int64 `json:"id"` + Name string `json:"name"` + ClientID string `json:"client_id"` + ClientSecret string `json:"client_secret"` + ConfidentialClient bool `json:"confidential_client"` + SkipSecondaryAuthorization bool `json:"skip_secondary_authorization"` + RedirectURIs []string `json:"redirect_uris"` + Created time.Time `json:"created"` } // OAuth2ApplicationList represents a list of OAuth2 applications. diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 95e8cd5b2e..746288bb9a 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -914,6 +914,7 @@ create_oauth2_application_success = You have successfully created a new OAuth2 a update_oauth2_application_success = You have successfully updated the OAuth2 application. oauth2_application_name = Application Name 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. +oauth2_skip_secondary_authorization = Skip authorization for public clients after granting access once. May pose a security risk. oauth2_redirect_uris = Redirect URIs. Please use a new line for every URI. save_application = Save oauth2_client_id = Client ID diff --git a/routers/api/v1/user/app.go b/routers/api/v1/user/app.go index 60354b1f26..5c28dd878d 100644 --- a/routers/api/v1/user/app.go +++ b/routers/api/v1/user/app.go @@ -223,10 +223,11 @@ func CreateOauth2Application(ctx *context.APIContext) { data := web.GetForm(ctx).(*api.CreateOAuth2ApplicationOptions) app, err := auth_model.CreateOAuth2Application(ctx, auth_model.CreateOAuth2ApplicationOptions{ - Name: data.Name, - UserID: ctx.Doer.ID, - RedirectURIs: data.RedirectURIs, - ConfidentialClient: data.ConfidentialClient, + Name: data.Name, + UserID: ctx.Doer.ID, + RedirectURIs: data.RedirectURIs, + ConfidentialClient: data.ConfidentialClient, + SkipSecondaryAuthorization: data.SkipSecondaryAuthorization, }) if err != nil { ctx.Error(http.StatusBadRequest, "", "error creating oauth2 application") @@ -381,11 +382,12 @@ func UpdateOauth2Application(ctx *context.APIContext) { data := web.GetForm(ctx).(*api.CreateOAuth2ApplicationOptions) app, err := auth_model.UpdateOAuth2Application(ctx, auth_model.UpdateOAuth2ApplicationOptions{ - Name: data.Name, - UserID: ctx.Doer.ID, - ID: appID, - RedirectURIs: data.RedirectURIs, - ConfidentialClient: data.ConfidentialClient, + Name: data.Name, + UserID: ctx.Doer.ID, + ID: appID, + RedirectURIs: data.RedirectURIs, + ConfidentialClient: data.ConfidentialClient, + SkipSecondaryAuthorization: data.SkipSecondaryAuthorization, }) if err != nil { if auth_model.IsErrOauthClientIDInvalid(err) || auth_model.IsErrOAuthApplicationNotFound(err) { diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 1205c2c578..0ccd460a78 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -469,9 +469,9 @@ func AuthorizeOAuth(ctx *context.Context) { return } - // Redirect if user already granted access and the application is confidential. - // I.e. always require authorization for public clients as recommended by RFC 6749 Section 10.2 - if app.ConfidentialClient && grant != nil { + // Redirect if user already granted access and the application is confidential or trusted otherwise + // I.e. always require authorization for untrusted public clients as recommended by RFC 6749 Section 10.2 + if (app.ConfidentialClient || app.SkipSecondaryAuthorization) && grant != nil { code, err := grant.GenerateNewAuthorizationCode(ctx, form.RedirectURI, form.CodeChallenge, form.CodeChallengeMethod) if err != nil { handleServerError(ctx, form.State, form.RedirectURI) diff --git a/routers/web/user/setting/oauth2_common.go b/routers/web/user/setting/oauth2_common.go index 4477dc570f..6029d7bedb 100644 --- a/routers/web/user/setting/oauth2_common.go +++ b/routers/web/user/setting/oauth2_common.go @@ -49,10 +49,11 @@ func (oa *OAuth2CommonHandlers) AddApp(ctx *context.Context) { // TODO validate redirect URI app, err := auth.CreateOAuth2Application(ctx, auth.CreateOAuth2ApplicationOptions{ - Name: form.Name, - RedirectURIs: util.SplitTrimSpace(form.RedirectURIs, "\n"), - UserID: oa.OwnerID, - ConfidentialClient: form.ConfidentialClient, + Name: form.Name, + RedirectURIs: util.SplitTrimSpace(form.RedirectURIs, "\n"), + UserID: oa.OwnerID, + ConfidentialClient: form.ConfidentialClient, + SkipSecondaryAuthorization: form.SkipSecondaryAuthorization, }) if err != nil { ctx.ServerError("CreateOAuth2Application", err) @@ -102,11 +103,12 @@ func (oa *OAuth2CommonHandlers) EditSave(ctx *context.Context) { // TODO validate redirect URI var err error if ctx.Data["App"], err = auth.UpdateOAuth2Application(ctx, auth.UpdateOAuth2ApplicationOptions{ - ID: ctx.PathParamInt64("id"), - Name: form.Name, - RedirectURIs: util.SplitTrimSpace(form.RedirectURIs, "\n"), - UserID: oa.OwnerID, - ConfidentialClient: form.ConfidentialClient, + ID: ctx.PathParamInt64("id"), + Name: form.Name, + RedirectURIs: util.SplitTrimSpace(form.RedirectURIs, "\n"), + UserID: oa.OwnerID, + ConfidentialClient: form.ConfidentialClient, + SkipSecondaryAuthorization: form.SkipSecondaryAuthorization, }); err != nil { ctx.ServerError("UpdateOAuth2Application", err) return diff --git a/services/convert/convert.go b/services/convert/convert.go index 3e3fca28ca..d70c1b940a 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -455,13 +455,14 @@ func ToTopicResponse(topic *repo_model.Topic) *api.TopicResponse { // ToOAuth2Application convert from auth.OAuth2Application to api.OAuth2Application func ToOAuth2Application(app *auth.OAuth2Application) *api.OAuth2Application { return &api.OAuth2Application{ - ID: app.ID, - Name: app.Name, - ClientID: app.ClientID, - ClientSecret: app.ClientSecret, - ConfidentialClient: app.ConfidentialClient, - RedirectURIs: app.RedirectURIs, - Created: app.CreatedUnix.AsTime(), + ID: app.ID, + Name: app.Name, + ClientID: app.ClientID, + ClientSecret: app.ClientSecret, + ConfidentialClient: app.ConfidentialClient, + SkipSecondaryAuthorization: app.SkipSecondaryAuthorization, + RedirectURIs: app.RedirectURIs, + Created: app.CreatedUnix.AsTime(), } } diff --git a/services/forms/user_form.go b/services/forms/user_form.go index b4be1e02b7..5b7a43642a 100644 --- a/services/forms/user_form.go +++ b/services/forms/user_form.go @@ -365,9 +365,10 @@ func (f *NewAccessTokenForm) GetScope() (auth_model.AccessTokenScope, error) { // EditOAuth2ApplicationForm form for editing oauth2 applications type EditOAuth2ApplicationForm struct { - Name string `binding:"Required;MaxSize(255)" form:"application_name"` - RedirectURIs string `binding:"Required" form:"redirect_uris"` - ConfidentialClient bool `form:"confidential_client"` + Name string `binding:"Required;MaxSize(255)" form:"application_name"` + RedirectURIs string `binding:"Required" form:"redirect_uris"` + ConfidentialClient bool `form:"confidential_client"` + SkipSecondaryAuthorization bool `form:"skip_secondary_authorization"` } // Validate validates the fields diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 5c46c3c9b8..3fc2698ff2 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -19875,6 +19875,10 @@ "type": "string" }, "x-go-name": "RedirectURIs" + }, + "skip_secondary_authorization": { + "type": "boolean", + "x-go-name": "SkipSecondaryAuthorization" } }, "x-go-package": "code.gitea.io/gitea/modules/structs" @@ -23002,6 +23006,10 @@ "type": "string" }, "x-go-name": "RedirectURIs" + }, + "skip_secondary_authorization": { + "type": "boolean", + "x-go-name": "SkipSecondaryAuthorization" } }, "x-go-package": "code.gitea.io/gitea/modules/structs" diff --git a/templates/user/settings/applications_oauth2_edit_form.tmpl b/templates/user/settings/applications_oauth2_edit_form.tmpl index e62115d226..944729117c 100644 --- a/templates/user/settings/applications_oauth2_edit_form.tmpl +++ b/templates/user/settings/applications_oauth2_edit_form.tmpl @@ -44,7 +44,13 @@
- + +
+
+
+
+ +