From 22fc170ac8accef7dd015a8caf4746d19d11e70d Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Fri, 21 Jun 2024 21:19:23 +0200 Subject: [PATCH 1/7] add skip secondary authorization option for public oauth2 clients --- models/auth/oauth2.go | 43 +++++++++++-------- models/migrations/migrations.go | 2 + models/migrations/v1_23/v300.go | 17 ++++++++ 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.js | 9 ++++ web_src/js/index.js | 4 ++ 15 files changed, 128 insertions(+), 62 deletions(-) create mode 100644 models/migrations/v1_23/v300.go create mode 100644 web_src/js/features/oauth2-settings.js diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index 7dca378e5d1a..c270e4856e76 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 08882fb1195c..8d33d68e765c 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -591,6 +591,8 @@ var migrations = []Migration{ // v299 -> v300 NewMigration("Add content version to issue and comment table", v1_23.AddContentVersionToIssueAndComment), + // v300 -> v301 + 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/v300.go b/models/migrations/v1_23/v300.go new file mode 100644 index 000000000000..74021d745c0a --- /dev/null +++ b/models/migrations/v1_23/v300.go @@ -0,0 +1,17 @@ +// 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 { + ID int64 + 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 7f78fbd495e2..a7d2e28b4129 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 815cba6eecaf..d4e34ae6985a 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -911,6 +911,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 client 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 60354b1f26ec..5c28dd878d2a 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 50f0dff2b62d..0d708b913467 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -470,9 +470,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 4477dc570f80..6029d7bedb0a 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 5db33ad85dfa..ae78a714a4f7 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -448,13 +448,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 b4be1e02b76b..5b7a43642ab2 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 4aa64c537611..375d61a1b0ea 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -19823,6 +19823,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" @@ -22924,6 +22928,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 e62115d22678..944729117cb0 100644 --- a/templates/user/settings/applications_oauth2_edit_form.tmpl +++ b/templates/user/settings/applications_oauth2_edit_form.tmpl @@ -44,7 +44,13 @@
- + +
+
+
+
+ +