Record OAuth client type at registration (#21316)

The OAuth spec [defines two types of
client](https://datatracker.ietf.org/doc/html/rfc6749#section-2.1),
confidential and public. Previously Gitea assumed all clients to be
confidential.

> OAuth defines two client types, based on their ability to authenticate
securely with the authorization server (i.e., ability to
>   maintain the confidentiality of their client credentials):
>
>   confidential
> Clients capable of maintaining the confidentiality of their
credentials (e.g., client implemented on a secure server with
> restricted access to the client credentials), or capable of secure
client authentication using other means.
>
>   **public
> Clients incapable of maintaining the confidentiality of their
credentials (e.g., clients executing on the device used by the resource
owner, such as an installed native application or a web browser-based
application), and incapable of secure client authentication via any
other means.**
>
> The client type designation is based on the authorization server's
definition of secure authentication and its acceptable exposure levels
of client credentials. The authorization server SHOULD NOT make
assumptions about the client type.

 https://datatracker.ietf.org/doc/html/rfc8252#section-8.4

> Authorization servers MUST record the client type in the client
registration details in order to identify and process requests
accordingly.

Require PKCE for public clients:
https://datatracker.ietf.org/doc/html/rfc8252#section-8.1

> Authorization servers SHOULD reject authorization requests from native
apps that don't use PKCE by returning an error message

Fixes #21299

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

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