diff --git a/docs/content/doc/developers/oauth2-provider.md b/docs/content/doc/developers/oauth2-provider.md index 9e6ab11742fb5..c6765f19e7aab 100644 --- a/docs/content/doc/developers/oauth2-provider.md +++ b/docs/content/doc/developers/oauth2-provider.md @@ -44,6 +44,12 @@ To use the Authorization Code Grant as a third party application it is required Currently Gitea does not support scopes (see [#4300](https://github.com/go-gitea/gitea/issues/4300)) and all third party applications will be granted access to all resources of the user and their organizations. +## Client types + +Gitea supports both confidential and public client types, [as defined by RFC 6749](https://datatracker.ietf.org/doc/html/rfc6749#section-2.1). + +For public clients, a redirect URI of a loopback IP address such as `http://127.0.0.1/` allows any port. Avoid using `localhost`, [as recommended by RFC 8252](https://datatracker.ietf.org/doc/html/rfc8252#section-8.3). + ## Example **Note:** This example does not use PKCE. diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index 9fdce2425372b..e42084c086bad 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -31,9 +31,14 @@ type OAuth2Application struct { Name string ClientID string `xorm:"unique"` ClientSecret string - RedirectURIs []string `xorm:"redirect_uris JSON TEXT"` - CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` - UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + // OAuth defines both Confidential and Public client types + // 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"` } func init() { @@ -57,15 +62,17 @@ func (app *OAuth2Application) PrimaryRedirectURI() string { // ContainsRedirectURI checks if redirectURI is allowed for app func (app *OAuth2Application) ContainsRedirectURI(redirectURI string) bool { - uri, err := url.Parse(redirectURI) - // ignore port for http loopback uris following https://datatracker.ietf.org/doc/html/rfc8252#section-7.3 - if err == nil && uri.Scheme == "http" && uri.Port() != "" { - ip := net.ParseIP(uri.Hostname()) - if ip != nil && ip.IsLoopback() { - // strip port - uri.Host = uri.Hostname() - if util.IsStringInSlice(uri.String(), app.RedirectURIs, true) { - return true + if !app.ConfidentialClient { + uri, err := url.Parse(redirectURI) + // ignore port for http loopback uris following https://datatracker.ietf.org/doc/html/rfc8252#section-7.3 + if err == nil && uri.Scheme == "http" && uri.Port() != "" { + ip := net.ParseIP(uri.Hostname()) + if ip != nil && ip.IsLoopback() { + // strip port + uri.Host = uri.Hostname() + if util.IsStringInSlice(uri.String(), app.RedirectURIs, true) { + return true + } } } } @@ -161,19 +168,21 @@ func GetOAuth2ApplicationsByUserID(ctx context.Context, userID int64) (apps []*O // CreateOAuth2ApplicationOptions holds options to create an oauth2 application type CreateOAuth2ApplicationOptions struct { - Name string - UserID int64 - RedirectURIs []string + Name string + UserID int64 + ConfidentialClient 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, + UID: opts.UserID, + Name: opts.Name, + ClientID: clientID, + RedirectURIs: opts.RedirectURIs, + ConfidentialClient: opts.ConfidentialClient, } if err := db.Insert(ctx, app); err != nil { return nil, err @@ -183,10 +192,11 @@ func CreateOAuth2Application(ctx context.Context, opts CreateOAuth2ApplicationOp // UpdateOAuth2ApplicationOptions holds options to update an oauth2 application type UpdateOAuth2ApplicationOptions struct { - ID int64 - Name string - UserID int64 - RedirectURIs []string + ID int64 + Name string + UserID int64 + ConfidentialClient bool + RedirectURIs []string } // UpdateOAuth2Application updates an oauth2 application @@ -207,6 +217,7 @@ func UpdateOAuth2Application(opts UpdateOAuth2ApplicationOptions) (*OAuth2Applic app.Name = opts.Name app.RedirectURIs = opts.RedirectURIs + app.ConfidentialClient = opts.ConfidentialClient if err = updateOAuth2Application(ctx, app); err != nil { return nil, err @@ -217,7 +228,7 @@ func UpdateOAuth2Application(opts UpdateOAuth2ApplicationOptions) (*OAuth2Applic } func updateOAuth2Application(ctx context.Context, app *OAuth2Application) error { - if _, err := db.GetEngine(ctx).ID(app.ID).Update(app); err != nil { + if _, err := db.GetEngine(ctx).ID(app.ID).UseBool("confidential_client").Update(app); err != nil { return err } return nil diff --git a/models/auth/oauth2_test.go b/models/auth/oauth2_test.go index 3815cb3b2c195..7a4df6b9acd7a 100644 --- a/models/auth/oauth2_test.go +++ b/models/auth/oauth2_test.go @@ -45,7 +45,8 @@ func TestOAuth2Application_ContainsRedirectURI(t *testing.T) { func TestOAuth2Application_ContainsRedirectURI_WithPort(t *testing.T) { app := &auth_model.OAuth2Application{ - RedirectURIs: []string{"http://127.0.0.1/", "http://::1/", "http://192.168.0.1/", "http://intranet/", "https://127.0.0.1/"}, + RedirectURIs: []string{"http://127.0.0.1/", "http://::1/", "http://192.168.0.1/", "http://intranet/", "https://127.0.0.1/"}, + ConfidentialClient: false, } // http loopback uris should ignore port diff --git a/models/fixtures/oauth2_application.yml b/models/fixtures/oauth2_application.yml index 34d5a887776a0..2f38cb58b6169 100644 --- a/models/fixtures/oauth2_application.yml +++ b/models/fixtures/oauth2_application.yml @@ -7,3 +7,14 @@ redirect_uris: '["a", "https://example.com/xyzzy"]' created_unix: 1546869730 updated_unix: 1546869730 + confidential_client: true +- + id: 2 + uid: 2 + name: "Test native app" + client_id: "ce5a1322-42a7-11ed-b878-0242ac120002" + client_secret: "$2a$10$UYRgUSgekzBp6hYe8pAdc.cgB4Gn06QRKsORUnIYTYQADs.YR/uvi" # bcrypt of "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA= + redirect_uris: '["http://127.0.0.1"]' + created_unix: 1546869730 + updated_unix: 1546869730 + confidential_client: false diff --git a/models/fixtures/oauth2_authorization_code.yml b/models/fixtures/oauth2_authorization_code.yml index 2abce16354a1b..d29502164e67f 100644 --- a/models/fixtures/oauth2_authorization_code.yml +++ b/models/fixtures/oauth2_authorization_code.yml @@ -6,3 +6,10 @@ redirect_uri: "a" valid_until: 3546869730 +- id: 2 + grant_id: 4 + code: "authcodepublic" + code_challenge: "CjvyTLSdR47G5zYenDA-eDWW4lRrO8yvjcWwbD_deOg" # Code Verifier: N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt + code_challenge_method: "S256" + redirect_uri: "http://127.0.0.1/" + valid_until: 3546869730 diff --git a/models/fixtures/oauth2_grant.yml b/models/fixtures/oauth2_grant.yml index e52a2bce959e6..e63286878b20f 100644 --- a/models/fixtures/oauth2_grant.yml +++ b/models/fixtures/oauth2_grant.yml @@ -20,4 +20,12 @@ counter: 1 scope: "openid profile email" created_unix: 1546869730 - updated_unix: 1546869730 \ No newline at end of file + updated_unix: 1546869730 + +- id: 4 + user_id: 99 + application_id: 2 + counter: 1 + scope: "whatever" + created_unix: 1546869730 + updated_unix: 1546869730 diff --git a/models/migrations/fixtures/Test_addConfidentialClientColumnToOAuth2ApplicationTable/o_auth2_application.yml b/models/migrations/fixtures/Test_addConfidentialClientColumnToOAuth2ApplicationTable/o_auth2_application.yml new file mode 100644 index 0000000000000..a88c2ef89f088 --- /dev/null +++ b/models/migrations/fixtures/Test_addConfidentialClientColumnToOAuth2ApplicationTable/o_auth2_application.yml @@ -0,0 +1,2 @@ +- + id: 1 diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index cca6c52d429a2..a6201c109074e 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -421,6 +421,8 @@ var migrations = []Migration{ NewMigration("Add TeamInvite table", addTeamInviteTable), // v229 -> v230 NewMigration("Update counts of all open milestones", updateOpenMilestoneCounts), + // v230 -> v231 + NewMigration("Add ConfidentialClient column (default true) to OAuth2Application table", addConfidentialClientColumnToOAuth2ApplicationTable), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v230.go b/models/migrations/v230.go new file mode 100644 index 0000000000000..f08e6a37641fc --- /dev/null +++ b/models/migrations/v230.go @@ -0,0 +1,18 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "xorm.io/xorm" +) + +// addConfidentialColumnToOAuth2ApplicationTable: add ConfidentialClient column, setting existing rows to true +func addConfidentialClientColumnToOAuth2ApplicationTable(x *xorm.Engine) error { + type OAuth2Application struct { + ConfidentialClient bool `xorm:"NOT NULL DEFAULT TRUE"` + } + + return x.Sync(new(OAuth2Application)) +} diff --git a/models/migrations/v230_test.go b/models/migrations/v230_test.go new file mode 100644 index 0000000000000..98ba3f5d97209 --- /dev/null +++ b/models/migrations/v230_test.go @@ -0,0 +1,46 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_addConfidentialClientColumnToOAuth2ApplicationTable(t *testing.T) { + // premigration + type OAuth2Application struct { + ID int64 + } + + // Prepare and load the testing database + x, deferable := prepareTestEnv(t, 0, new(OAuth2Application)) + defer deferable() + if x == nil || t.Failed() { + return + } + + if err := addConfidentialClientColumnToOAuth2ApplicationTable(x); err != nil { + assert.NoError(t, err) + return + } + + // postmigration + type ExpectedOAuth2Application struct { + ID int64 + ConfidentialClient bool + } + + got := []ExpectedOAuth2Application{} + if err := x.Table("o_auth2_application").Select("id, confidential_client").Find(&got); !assert.NoError(t, err) { + return + } + + assert.NotEmpty(t, got) + for _, e := range got { + assert.True(t, e.ConfidentialClient) + } +} diff --git a/modules/convert/convert.go b/modules/convert/convert.go index 187c67fa7680b..8c92bbb3712f2 100644 --- a/modules/convert/convert.go +++ b/modules/convert/convert.go @@ -392,12 +392,13 @@ 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, - RedirectURIs: app.RedirectURIs, - Created: app.CreatedUnix.AsTime(), + ID: app.ID, + Name: app.Name, + ClientID: app.ClientID, + ClientSecret: app.ClientSecret, + ConfidentialClient: app.ConfidentialClient, + RedirectURIs: app.RedirectURIs, + Created: app.CreatedUnix.AsTime(), } } diff --git a/modules/structs/user_app.go b/modules/structs/user_app.go index 44df5a6a49546..4cfa5538c8fcc 100644 --- a/modules/structs/user_app.go +++ b/modules/structs/user_app.go @@ -30,19 +30,21 @@ type CreateAccessTokenOption struct { // CreateOAuth2ApplicationOptions holds options to create an oauth2 application type CreateOAuth2ApplicationOptions struct { - Name string `json:"name" binding:"Required"` - RedirectURIs []string `json:"redirect_uris" binding:"Required"` + Name string `json:"name" binding:"Required"` + ConfidentialClient bool `json:"confidential_client"` + 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"` - 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"` + 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 3c2e70187cd75..1566dfc97d422 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -749,9 +749,7 @@ create_oauth2_application_button = Create Application create_oauth2_application_success = You've successfully created a new OAuth2 application. update_oauth2_application_success = You've successfully updated the OAuth2 application. oauth2_application_name = Application Name -oauth2_select_type = Which application type fits? -oauth2_type_web = Web (e.g. Node.JS, Tomcat, Go) -oauth2_type_native = Native (e.g. Mobile, Desktop, Browser) +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_redirect_uri = Redirect 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 a94db79239344..14f1592591864 100644 --- a/routers/api/v1/user/app.go +++ b/routers/api/v1/user/app.go @@ -213,9 +213,10 @@ 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, + Name: data.Name, + UserID: ctx.Doer.ID, + RedirectURIs: data.RedirectURIs, + ConfidentialClient: data.ConfidentialClient, }) if err != nil { ctx.Error(http.StatusBadRequest, "", "error creating oauth2 application") @@ -363,10 +364,11 @@ func UpdateOauth2Application(ctx *context.APIContext) { data := web.GetForm(ctx).(*api.CreateOAuth2ApplicationOptions) app, err := auth_model.UpdateOAuth2Application(auth_model.UpdateOAuth2ApplicationOptions{ - Name: data.Name, - UserID: ctx.Doer.ID, - ID: appID, - RedirectURIs: data.RedirectURIs, + Name: data.Name, + UserID: ctx.Doer.ID, + ID: appID, + RedirectURIs: data.RedirectURIs, + ConfidentialClient: data.ConfidentialClient, }) 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 c98385c8f6cd9..9c929d990ecaf 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -438,8 +438,21 @@ func AuthorizeOAuth(ctx *context.Context) { log.Error("Unable to save changes to the session: %v", err) } case "": - break + // "Authorization servers SHOULD reject authorization requests from native apps that don't use PKCE by returning an error message" + // https://datatracker.ietf.org/doc/html/rfc8252#section-8.1 + if !app.ConfidentialClient { + // "the authorization endpoint MUST return the authorization error response with the "error" value set to "invalid_request"" + // https://datatracker.ietf.org/doc/html/rfc7636#section-4.4.1 + handleAuthorizeError(ctx, AuthorizeError{ + ErrorCode: ErrorCodeInvalidRequest, + ErrorDescription: "PKCE is required for public clients", + State: form.State, + }, form.RedirectURI) + return + } default: + // "If the server supporting PKCE does not support the requested transformation, the authorization endpoint MUST return the authorization error response with "error" value set to "invalid_request"." + // https://www.rfc-editor.org/rfc/rfc7636#section-4.4.1 handleAuthorizeError(ctx, AuthorizeError{ ErrorCode: ErrorCodeInvalidRequest, ErrorDescription: "unsupported code challenge method", diff --git a/routers/web/user/setting/oauth2_common.go b/routers/web/user/setting/oauth2_common.go index f02f6ab0419d9..49ee5c7c2fd82 100644 --- a/routers/web/user/setting/oauth2_common.go +++ b/routers/web/user/setting/oauth2_common.go @@ -39,9 +39,10 @@ func (oa *OAuth2CommonHandlers) AddApp(ctx *context.Context) { // TODO validate redirect URI app, err := auth.CreateOAuth2Application(ctx, auth.CreateOAuth2ApplicationOptions{ - Name: form.Name, - RedirectURIs: []string{form.RedirectURI}, - UserID: oa.OwnerID, + Name: form.Name, + RedirectURIs: []string{form.RedirectURI}, + UserID: oa.OwnerID, + ConfidentialClient: form.ConfidentialClient, }) if err != nil { ctx.ServerError("CreateOAuth2Application", err) @@ -90,10 +91,11 @@ func (oa *OAuth2CommonHandlers) EditSave(ctx *context.Context) { // TODO validate redirect URI var err error if ctx.Data["App"], err = auth.UpdateOAuth2Application(auth.UpdateOAuth2ApplicationOptions{ - ID: ctx.ParamsInt64("id"), - Name: form.Name, - RedirectURIs: []string{form.RedirectURI}, - UserID: oa.OwnerID, + ID: ctx.ParamsInt64("id"), + Name: form.Name, + RedirectURIs: []string{form.RedirectURI}, + UserID: oa.OwnerID, + ConfidentialClient: form.ConfidentialClient, }); err != nil { ctx.ServerError("UpdateOAuth2Application", err) return diff --git a/services/forms/user_form.go b/services/forms/user_form.go index 8ce1d85c57781..036c2ca3ec2c5 100644 --- a/services/forms/user_form.go +++ b/services/forms/user_form.go @@ -379,8 +379,9 @@ func (f *NewAccessTokenForm) Validate(req *http.Request, errs binding.Errors) bi // EditOAuth2ApplicationForm form for editing oauth2 applications type EditOAuth2ApplicationForm struct { - Name string `binding:"Required;MaxSize(255)" form:"application_name"` - RedirectURI string `binding:"Required" form:"redirect_uri"` + Name string `binding:"Required;MaxSize(255)" form:"application_name"` + RedirectURI string `binding:"Required" form:"redirect_uri"` + ConfidentialClient bool `form:"confidential_client"` } // Validate validates the fields diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 728e88b734aa5..94fb67ab44568 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -14645,6 +14645,10 @@ "description": "CreateOAuth2ApplicationOptions holds options to create an oauth2 application", "type": "object", "properties": { + "confidential_client": { + "type": "boolean", + "x-go-name": "ConfidentialClient" + }, "name": { "type": "string", "x-go-name": "Name" @@ -17306,6 +17310,10 @@ "type": "string", "x-go-name": "ClientSecret" }, + "confidential_client": { + "type": "boolean", + "x-go-name": "ConfidentialClient" + }, "created": { "type": "string", "format": "date-time", diff --git a/templates/user/settings/applications_oauth2_edit_form.tmpl b/templates/user/settings/applications_oauth2_edit_form.tmpl index 60311983c9057..9d7273fd6d0bf 100644 --- a/templates/user/settings/applications_oauth2_edit_form.tmpl +++ b/templates/user/settings/applications_oauth2_edit_form.tmpl @@ -43,6 +43,10 @@ +