Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add skip secondary authorization option for public oauth2 clients #31454

Merged
merged 10 commits into from
Jul 19, 2024
43 changes: 24 additions & 19 deletions models/auth/oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions models/migrations/v1_23/v300.go
Original file line number Diff line number Diff line change
@@ -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
denyskon marked this conversation as resolved.
Show resolved Hide resolved
SkipSecondaryAuthorization bool `xorm:"NOT NULL DEFAULT FALSE"`
}
return x.Sync(new(oauth2Application))
}
22 changes: 12 additions & 10 deletions modules/structs/user_app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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. <b>May pose a security risk.</b>
techknowlogick marked this conversation as resolved.
Show resolved Hide resolved
oauth2_redirect_uris = Redirect URIs. Please use a new line for every URI.
save_application = Save
oauth2_client_id = Client ID
Expand Down
20 changes: 11 additions & 9 deletions routers/api/v1/user/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions routers/web/auth/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
denyskon marked this conversation as resolved.
Show resolved Hide resolved
code, err := grant.GenerateNewAuthorizationCode(ctx, form.RedirectURI, form.CodeChallenge, form.CodeChallengeMethod)
if err != nil {
handleServerError(ctx, form.State, form.RedirectURI)
Expand Down
20 changes: 11 additions & 9 deletions routers/web/user/setting/oauth2_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
15 changes: 8 additions & 7 deletions services/convert/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
}

Expand Down
7 changes: 4 additions & 3 deletions services/forms/user_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions templates/swagger/v1_json.tmpl

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion templates/user/settings/applications_oauth2_edit_form.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,13 @@
<div class="field {{if .Err_ConfidentialClient}}error{{end}}">
<div class="ui checkbox">
<label>{{ctx.Locale.Tr "settings.oauth2_confidential_client"}}</label>
<input type="checkbox" name="confidential_client" {{if .App.ConfidentialClient}}checked{{end}}>
<input class="disable-setting" type="checkbox" name="confidential_client" data-target="#skip-secondary-authorization" {{if .App.ConfidentialClient}}checked{{end}}>
</div>
</div>
<div class="field {{if .Err_SkipSecondaryAuthorization}}error{{end}} {{if .App.ConfidentialClient}}disabled{{end}}" id="skip-secondary-authorization">
<div class="ui checkbox">
<label>{{ctx.Locale.Tr "settings.oauth2_skip_secondary_authorization"}}</label>
<input type="checkbox" name="skip_secondary_authorization" {{if .App.SkipSecondaryAuthorization}}checked{{end}}>
</div>
</div>
<button class="ui primary button">
Expand Down
8 changes: 7 additions & 1 deletion templates/user/settings/applications_oauth2_list.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,13 @@
<div class="field {{if .Err_ConfidentialClient}}error{{end}}">
<div class="ui checkbox">
<label>{{ctx.Locale.Tr "settings.oauth2_confidential_client"}}</label>
<input type="checkbox" name="confidential_client" checked>
<input class="disable-setting" type="checkbox" name="confidential_client" data-target="#skip-secondary-authorization" checked>
</div>
</div>
<div class="field {{if .Err_SkipSecondaryAuthorization}}error{{end}} disabled" id="skip-secondary-authorization">
<div class="ui checkbox">
<label>{{ctx.Locale.Tr "settings.oauth2_skip_secondary_authorization"}}</label>
<input type="checkbox" name="skip_secondary_authorization">
</div>
</div>
<button class="ui primary button">
Expand Down
9 changes: 9 additions & 0 deletions web_src/js/features/oauth2-settings.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export function initOAuth2SettingsDisableCheckbox() {
for (const e of document.querySelectorAll('.disable-setting')) e.addEventListener('change', ({target}) => {
if (target.checked) {
document.querySelector(e.getAttribute('data-target')).classList.add('disabled');
} else {
document.querySelector(e.getAttribute('data-target')).classList.remove('disabled');
}
denyskon marked this conversation as resolved.
Show resolved Hide resolved
});
}
3 changes: 3 additions & 0 deletions web_src/js/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ import {initDirAuto} from './modules/dirauto.js';
import {initRepositorySearch} from './features/repo-search.js';
import {initColorPickers} from './features/colorpicker.js';
import {initAdminSelfCheck} from './features/admin/selfcheck.js';
import {initOAuth2SettingsDisableCheckbox} from './features/oauth2-settings.js';
import {initGlobalFetchAction} from './features/common-fetch-action.js';
import {initFootLanguageMenu, initGlobalComponents, initHeadNavbarContentToggle} from './features/common-page.js';
import {
Expand Down Expand Up @@ -194,4 +195,6 @@ onDomReady(() => {
initPdfViewer();
initScopedAccessTokenCategories();
initColorPickers();

initOAuth2SettingsDisableCheckbox();
});