Record OAuth client type at registration

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.

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

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

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

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

models/fixtures/oauth2_grant.yml

@@ -20,4 +20,12 @@
   counter: 1
   scope: "openid profile email"
   created_unix: 1546869730
-  updated_unix: 1546869730
+  updated_unix: 1546869730
+
+- id: 4
+  user_id: 99
+  application_id: 2
+  counter: 1
+  scope: "whatever"
+  created_unix: 1546869730
+  updated_unix: 1546869730

models/migrations/fixtures/Test_addConfidentialClientColumnToOAuth2ApplicationTable/o_auth2_application.yml

@@ -0,0 +1,2 @@
+-
+  id: 1

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

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))
+}

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)
+	}
+}

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(),
 	}
 }
 

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.

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

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) {

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",

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