Skip to content

Commit 191a74d

Browse files
hickfordwxiaoguanglunny
authored
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 <[email protected]> Co-authored-by: Lunny Xiao <[email protected]>
1 parent e1ce45e commit 191a74d

File tree

22 files changed

+226
-60
lines changed

22 files changed

+226
-60
lines changed

Diff for: ‎docs/content/doc/developers/oauth2-provider.md

+6
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ To use the Authorization Code Grant as a third party application it is required
4444

4545
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.
4646

47+
## Client types
48+
49+
Gitea supports both confidential and public client types, [as defined by RFC 6749](https://datatracker.ietf.org/doc/html/rfc6749#section-2.1).
50+
51+
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).
52+
4753
## Example
4854

4955
**Note:** This example does not use PKCE.

Diff for: ‎models/auth/oauth2.go

+35-24
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,14 @@ type OAuth2Application struct {
3131
Name string
3232
ClientID string `xorm:"unique"`
3333
ClientSecret string
34-
RedirectURIs []string `xorm:"redirect_uris JSON TEXT"`
35-
CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
36-
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
34+
// OAuth defines both Confidential and Public client types
35+
// https://datatracker.ietf.org/doc/html/rfc6749#section-2.1
36+
// "Authorization servers MUST record the client type in the client registration details"
37+
// https://datatracker.ietf.org/doc/html/rfc8252#section-8.4
38+
ConfidentialClient bool `xorm:"NOT NULL DEFAULT TRUE"`
39+
RedirectURIs []string `xorm:"redirect_uris JSON TEXT"`
40+
CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
41+
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
3742
}
3843

3944
func init() {
@@ -57,15 +62,17 @@ func (app *OAuth2Application) PrimaryRedirectURI() string {
5762

5863
// ContainsRedirectURI checks if redirectURI is allowed for app
5964
func (app *OAuth2Application) ContainsRedirectURI(redirectURI string) bool {
60-
uri, err := url.Parse(redirectURI)
61-
// ignore port for http loopback uris following https://datatracker.ietf.org/doc/html/rfc8252#section-7.3
62-
if err == nil && uri.Scheme == "http" && uri.Port() != "" {
63-
ip := net.ParseIP(uri.Hostname())
64-
if ip != nil && ip.IsLoopback() {
65-
// strip port
66-
uri.Host = uri.Hostname()
67-
if util.IsStringInSlice(uri.String(), app.RedirectURIs, true) {
68-
return true
65+
if !app.ConfidentialClient {
66+
uri, err := url.Parse(redirectURI)
67+
// ignore port for http loopback uris following https://datatracker.ietf.org/doc/html/rfc8252#section-7.3
68+
if err == nil && uri.Scheme == "http" && uri.Port() != "" {
69+
ip := net.ParseIP(uri.Hostname())
70+
if ip != nil && ip.IsLoopback() {
71+
// strip port
72+
uri.Host = uri.Hostname()
73+
if util.IsStringInSlice(uri.String(), app.RedirectURIs, true) {
74+
return true
75+
}
6976
}
7077
}
7178
}
@@ -161,19 +168,21 @@ func GetOAuth2ApplicationsByUserID(ctx context.Context, userID int64) (apps []*O
161168

162169
// CreateOAuth2ApplicationOptions holds options to create an oauth2 application
163170
type CreateOAuth2ApplicationOptions struct {
164-
Name string
165-
UserID int64
166-
RedirectURIs []string
171+
Name string
172+
UserID int64
173+
ConfidentialClient bool
174+
RedirectURIs []string
167175
}
168176

169177
// CreateOAuth2Application inserts a new oauth2 application
170178
func CreateOAuth2Application(ctx context.Context, opts CreateOAuth2ApplicationOptions) (*OAuth2Application, error) {
171179
clientID := uuid.New().String()
172180
app := &OAuth2Application{
173-
UID: opts.UserID,
174-
Name: opts.Name,
175-
ClientID: clientID,
176-
RedirectURIs: opts.RedirectURIs,
181+
UID: opts.UserID,
182+
Name: opts.Name,
183+
ClientID: clientID,
184+
RedirectURIs: opts.RedirectURIs,
185+
ConfidentialClient: opts.ConfidentialClient,
177186
}
178187
if err := db.Insert(ctx, app); err != nil {
179188
return nil, err
@@ -183,10 +192,11 @@ func CreateOAuth2Application(ctx context.Context, opts CreateOAuth2ApplicationOp
183192

184193
// UpdateOAuth2ApplicationOptions holds options to update an oauth2 application
185194
type UpdateOAuth2ApplicationOptions struct {
186-
ID int64
187-
Name string
188-
UserID int64
189-
RedirectURIs []string
195+
ID int64
196+
Name string
197+
UserID int64
198+
ConfidentialClient bool
199+
RedirectURIs []string
190200
}
191201

192202
// UpdateOAuth2Application updates an oauth2 application
@@ -207,6 +217,7 @@ func UpdateOAuth2Application(opts UpdateOAuth2ApplicationOptions) (*OAuth2Applic
207217

208218
app.Name = opts.Name
209219
app.RedirectURIs = opts.RedirectURIs
220+
app.ConfidentialClient = opts.ConfidentialClient
210221

211222
if err = updateOAuth2Application(ctx, app); err != nil {
212223
return nil, err
@@ -217,7 +228,7 @@ func UpdateOAuth2Application(opts UpdateOAuth2ApplicationOptions) (*OAuth2Applic
217228
}
218229

219230
func updateOAuth2Application(ctx context.Context, app *OAuth2Application) error {
220-
if _, err := db.GetEngine(ctx).ID(app.ID).Update(app); err != nil {
231+
if _, err := db.GetEngine(ctx).ID(app.ID).UseBool("confidential_client").Update(app); err != nil {
221232
return err
222233
}
223234
return nil

Diff for: ‎models/auth/oauth2_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ func TestOAuth2Application_ContainsRedirectURI(t *testing.T) {
4545

4646
func TestOAuth2Application_ContainsRedirectURI_WithPort(t *testing.T) {
4747
app := &auth_model.OAuth2Application{
48-
RedirectURIs: []string{"http://127.0.0.1/", "http://::1/", "http://192.168.0.1/", "http://intranet/", "https://127.0.0.1/"},
48+
RedirectURIs: []string{"http://127.0.0.1/", "http://::1/", "http://192.168.0.1/", "http://intranet/", "https://127.0.0.1/"},
49+
ConfidentialClient: false,
4950
}
5051

5152
// http loopback uris should ignore port

Diff for: ‎models/fixtures/oauth2_application.yml

+11
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,14 @@
77
redirect_uris: '["a", "https://example.com/xyzzy"]'
88
created_unix: 1546869730
99
updated_unix: 1546869730
10+
confidential_client: true
11+
-
12+
id: 2
13+
uid: 2
14+
name: "Test native app"
15+
client_id: "ce5a1322-42a7-11ed-b878-0242ac120002"
16+
client_secret: "$2a$10$UYRgUSgekzBp6hYe8pAdc.cgB4Gn06QRKsORUnIYTYQADs.YR/uvi" # bcrypt of "4MK8Na6R55smdCY0WuCCumZ6hjRPnGY5saWVRHHjJiA=
17+
redirect_uris: '["http://127.0.0.1"]'
18+
created_unix: 1546869730
19+
updated_unix: 1546869730
20+
confidential_client: false

Diff for: ‎models/fixtures/oauth2_authorization_code.yml

+7
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,10 @@
66
redirect_uri: "a"
77
valid_until: 3546869730
88

9+
- id: 2
10+
grant_id: 4
11+
code: "authcodepublic"
12+
code_challenge: "CjvyTLSdR47G5zYenDA-eDWW4lRrO8yvjcWwbD_deOg" # Code Verifier: N1Zo9-8Rfwhkt68r1r29ty8YwIraXR8eh_1Qwxg7yQXsonBt
13+
code_challenge_method: "S256"
14+
redirect_uri: "http://127.0.0.1/"
15+
valid_until: 3546869730

Diff for: ‎models/fixtures/oauth2_grant.yml

+9-1
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,12 @@
2020
counter: 1
2121
scope: "openid profile email"
2222
created_unix: 1546869730
23-
updated_unix: 1546869730
23+
updated_unix: 1546869730
24+
25+
- id: 4
26+
user_id: 99
27+
application_id: 2
28+
counter: 1
29+
scope: "whatever"
30+
created_unix: 1546869730
31+
updated_unix: 1546869730

Diff for: ‎models/migrations/fixtures/Test_addConfidentialClientColumnToOAuth2ApplicationTable/o_auth2_application.yml

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
-
2+
id: 1

Diff for: ‎models/migrations/migrations.go

+2
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,8 @@ var migrations = []Migration{
421421
NewMigration("Add TeamInvite table", addTeamInviteTable),
422422
// v229 -> v230
423423
NewMigration("Update counts of all open milestones", updateOpenMilestoneCounts),
424+
// v230 -> v231
425+
NewMigration("Add ConfidentialClient column (default true) to OAuth2Application table", addConfidentialClientColumnToOAuth2ApplicationTable),
424426
}
425427

426428
// GetCurrentDBVersion returns the current db version

Diff for: ‎models/migrations/v230.go

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Copyright 2022 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package migrations
6+
7+
import (
8+
"xorm.io/xorm"
9+
)
10+
11+
// addConfidentialColumnToOAuth2ApplicationTable: add ConfidentialClient column, setting existing rows to true
12+
func addConfidentialClientColumnToOAuth2ApplicationTable(x *xorm.Engine) error {
13+
type OAuth2Application struct {
14+
ConfidentialClient bool `xorm:"NOT NULL DEFAULT TRUE"`
15+
}
16+
17+
return x.Sync(new(OAuth2Application))
18+
}

Diff for: ‎models/migrations/v230_test.go

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// Copyright 2022 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package migrations
6+
7+
import (
8+
"testing"
9+
10+
"github.com/stretchr/testify/assert"
11+
)
12+
13+
func Test_addConfidentialClientColumnToOAuth2ApplicationTable(t *testing.T) {
14+
// premigration
15+
type OAuth2Application struct {
16+
ID int64
17+
}
18+
19+
// Prepare and load the testing database
20+
x, deferable := prepareTestEnv(t, 0, new(OAuth2Application))
21+
defer deferable()
22+
if x == nil || t.Failed() {
23+
return
24+
}
25+
26+
if err := addConfidentialClientColumnToOAuth2ApplicationTable(x); err != nil {
27+
assert.NoError(t, err)
28+
return
29+
}
30+
31+
// postmigration
32+
type ExpectedOAuth2Application struct {
33+
ID int64
34+
ConfidentialClient bool
35+
}
36+
37+
got := []ExpectedOAuth2Application{}
38+
if err := x.Table("o_auth2_application").Select("id, confidential_client").Find(&got); !assert.NoError(t, err) {
39+
return
40+
}
41+
42+
assert.NotEmpty(t, got)
43+
for _, e := range got {
44+
assert.True(t, e.ConfidentialClient)
45+
}
46+
}

Diff for: ‎modules/convert/convert.go

+7-6
Original file line numberDiff line numberDiff line change
@@ -392,12 +392,13 @@ func ToTopicResponse(topic *repo_model.Topic) *api.TopicResponse {
392392
// ToOAuth2Application convert from auth.OAuth2Application to api.OAuth2Application
393393
func ToOAuth2Application(app *auth.OAuth2Application) *api.OAuth2Application {
394394
return &api.OAuth2Application{
395-
ID: app.ID,
396-
Name: app.Name,
397-
ClientID: app.ClientID,
398-
ClientSecret: app.ClientSecret,
399-
RedirectURIs: app.RedirectURIs,
400-
Created: app.CreatedUnix.AsTime(),
395+
ID: app.ID,
396+
Name: app.Name,
397+
ClientID: app.ClientID,
398+
ClientSecret: app.ClientSecret,
399+
ConfidentialClient: app.ConfidentialClient,
400+
RedirectURIs: app.RedirectURIs,
401+
Created: app.CreatedUnix.AsTime(),
401402
}
402403
}
403404

Diff for: ‎modules/structs/user_app.go

+10-8
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,21 @@ type CreateAccessTokenOption struct {
3030

3131
// CreateOAuth2ApplicationOptions holds options to create an oauth2 application
3232
type CreateOAuth2ApplicationOptions struct {
33-
Name string `json:"name" binding:"Required"`
34-
RedirectURIs []string `json:"redirect_uris" binding:"Required"`
33+
Name string `json:"name" binding:"Required"`
34+
ConfidentialClient bool `json:"confidential_client"`
35+
RedirectURIs []string `json:"redirect_uris" binding:"Required"`
3536
}
3637

3738
// OAuth2Application represents an OAuth2 application.
3839
// swagger:response OAuth2Application
3940
type OAuth2Application struct {
40-
ID int64 `json:"id"`
41-
Name string `json:"name"`
42-
ClientID string `json:"client_id"`
43-
ClientSecret string `json:"client_secret"`
44-
RedirectURIs []string `json:"redirect_uris"`
45-
Created time.Time `json:"created"`
41+
ID int64 `json:"id"`
42+
Name string `json:"name"`
43+
ClientID string `json:"client_id"`
44+
ClientSecret string `json:"client_secret"`
45+
ConfidentialClient bool `json:"confidential_client"`
46+
RedirectURIs []string `json:"redirect_uris"`
47+
Created time.Time `json:"created"`
4648
}
4749

4850
// OAuth2ApplicationList represents a list of OAuth2 applications.

Diff for: ‎options/locale/locale_en-US.ini

+1-3
Original file line numberDiff line numberDiff line change
@@ -749,9 +749,7 @@ create_oauth2_application_button = Create Application
749749
create_oauth2_application_success = You've successfully created a new OAuth2 application.
750750
update_oauth2_application_success = You've successfully updated the OAuth2 application.
751751
oauth2_application_name = Application Name
752-
oauth2_select_type = Which application type fits?
753-
oauth2_type_web = Web (e.g. Node.JS, Tomcat, Go)
754-
oauth2_type_native = Native (e.g. Mobile, Desktop, Browser)
752+
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.
755753
oauth2_redirect_uri = Redirect URI
756754
save_application = Save
757755
oauth2_client_id = Client ID

Diff for: ‎routers/api/v1/user/app.go

+9-7
Original file line numberDiff line numberDiff line change
@@ -213,9 +213,10 @@ func CreateOauth2Application(ctx *context.APIContext) {
213213
data := web.GetForm(ctx).(*api.CreateOAuth2ApplicationOptions)
214214

215215
app, err := auth_model.CreateOAuth2Application(ctx, auth_model.CreateOAuth2ApplicationOptions{
216-
Name: data.Name,
217-
UserID: ctx.Doer.ID,
218-
RedirectURIs: data.RedirectURIs,
216+
Name: data.Name,
217+
UserID: ctx.Doer.ID,
218+
RedirectURIs: data.RedirectURIs,
219+
ConfidentialClient: data.ConfidentialClient,
219220
})
220221
if err != nil {
221222
ctx.Error(http.StatusBadRequest, "", "error creating oauth2 application")
@@ -363,10 +364,11 @@ func UpdateOauth2Application(ctx *context.APIContext) {
363364
data := web.GetForm(ctx).(*api.CreateOAuth2ApplicationOptions)
364365

365366
app, err := auth_model.UpdateOAuth2Application(auth_model.UpdateOAuth2ApplicationOptions{
366-
Name: data.Name,
367-
UserID: ctx.Doer.ID,
368-
ID: appID,
369-
RedirectURIs: data.RedirectURIs,
367+
Name: data.Name,
368+
UserID: ctx.Doer.ID,
369+
ID: appID,
370+
RedirectURIs: data.RedirectURIs,
371+
ConfidentialClient: data.ConfidentialClient,
370372
})
371373
if err != nil {
372374
if auth_model.IsErrOauthClientIDInvalid(err) || auth_model.IsErrOAuthApplicationNotFound(err) {

Diff for: ‎routers/web/auth/oauth.go

+14-1
Original file line numberDiff line numberDiff line change
@@ -438,8 +438,21 @@ func AuthorizeOAuth(ctx *context.Context) {
438438
log.Error("Unable to save changes to the session: %v", err)
439439
}
440440
case "":
441-
break
441+
// "Authorization servers SHOULD reject authorization requests from native apps that don't use PKCE by returning an error message"
442+
// https://datatracker.ietf.org/doc/html/rfc8252#section-8.1
443+
if !app.ConfidentialClient {
444+
// "the authorization endpoint MUST return the authorization error response with the "error" value set to "invalid_request""
445+
// https://datatracker.ietf.org/doc/html/rfc7636#section-4.4.1
446+
handleAuthorizeError(ctx, AuthorizeError{
447+
ErrorCode: ErrorCodeInvalidRequest,
448+
ErrorDescription: "PKCE is required for public clients",
449+
State: form.State,
450+
}, form.RedirectURI)
451+
return
452+
}
442453
default:
454+
// "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"."
455+
// https://www.rfc-editor.org/rfc/rfc7636#section-4.4.1
443456
handleAuthorizeError(ctx, AuthorizeError{
444457
ErrorCode: ErrorCodeInvalidRequest,
445458
ErrorDescription: "unsupported code challenge method",

Diff for: ‎routers/web/user/setting/oauth2_common.go

+9-7
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,10 @@ func (oa *OAuth2CommonHandlers) AddApp(ctx *context.Context) {
3939

4040
// TODO validate redirect URI
4141
app, err := auth.CreateOAuth2Application(ctx, auth.CreateOAuth2ApplicationOptions{
42-
Name: form.Name,
43-
RedirectURIs: []string{form.RedirectURI},
44-
UserID: oa.OwnerID,
42+
Name: form.Name,
43+
RedirectURIs: []string{form.RedirectURI},
44+
UserID: oa.OwnerID,
45+
ConfidentialClient: form.ConfidentialClient,
4546
})
4647
if err != nil {
4748
ctx.ServerError("CreateOAuth2Application", err)
@@ -90,10 +91,11 @@ func (oa *OAuth2CommonHandlers) EditSave(ctx *context.Context) {
9091
// TODO validate redirect URI
9192
var err error
9293
if ctx.Data["App"], err = auth.UpdateOAuth2Application(auth.UpdateOAuth2ApplicationOptions{
93-
ID: ctx.ParamsInt64("id"),
94-
Name: form.Name,
95-
RedirectURIs: []string{form.RedirectURI},
96-
UserID: oa.OwnerID,
94+
ID: ctx.ParamsInt64("id"),
95+
Name: form.Name,
96+
RedirectURIs: []string{form.RedirectURI},
97+
UserID: oa.OwnerID,
98+
ConfidentialClient: form.ConfidentialClient,
9799
}); err != nil {
98100
ctx.ServerError("UpdateOAuth2Application", err)
99101
return

0 commit comments

Comments
 (0)