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 setting to force login through openid #21851

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions cmd/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,10 @@ var (
Value: "",
Usage: "Group Claim value for restricted users",
},
cli.BoolFlag{
Name: "force-oauth",
Usage: "set to force all logins to the configured oauth provider",
},
cli.StringFlag{
Name: "group-team-map",
Value: "",
Expand Down Expand Up @@ -856,6 +860,7 @@ func parseOAuth2Config(c *cli.Context) *oauth2.Source {
CustomURLMapping: customURLMapping,
IconURL: c.String("icon-url"),
SkipLocalTwoFA: c.Bool("skip-local-2fa"),
ForceOAuth: c.Bool("force-oauth"),
Scopes: c.StringSlice("scopes"),
RequiredClaimName: c.String("required-claim-name"),
RequiredClaimValue: c.String("required-claim-value"),
Expand Down Expand Up @@ -946,6 +951,9 @@ func runUpdateOauth(c *cli.Context) error {
if c.IsSet("restricted-group") {
oAuth2Config.RestrictedGroup = c.String("restricted-group")
}
if c.IsSet("force-oauth") {
oAuth2Config.ForceOAuth = c.BoolT("force-oauth")
}
if c.IsSet("group-team-map") {
oAuth2Config.GroupTeamMap = c.String("group-team-map")
}
Expand Down
2 changes: 2 additions & 0 deletions docs/content/doc/usage/command-line.en-us.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ Admin operations:
- `--custom-email-url`: Use a custom Email URL (option for GitHub).
- `--icon-url`: Custom icon URL for OAuth2 login source.
- `--skip-local-2fa`: Allow source to override local 2FA. (Optional)
- `--force-oauth`: Automatically redirect sign in to this OAuth provider (Optional)
- `--scopes`: Additional scopes to request for this OAuth2 source. (Optional)
- `--required-claim-name`: Claim name that has to be set to allow users to login with this source. (Optional)
- `--required-claim-value`: Claim value that has to be set to allow users to login with this source. (Optional)
Expand All @@ -157,6 +158,7 @@ Admin operations:
- `--custom-email-url`: Use a custom Email URL (option for GitHub).
- `--icon-url`: Custom icon URL for OAuth2 login source.
- `--skip-local-2fa`: Allow source to override local 2FA. (Optional)
- `--force-oauth`: Automatically redirect sign in to this OAuth provider (Optional)
- `--scopes`: Additional scopes to request for this OAuth2 source.
- `--required-claim-name`: Claim name that has to be set to allow users to login with this source. (Optional)
- `--required-claim-value`: Claim value that has to be set to allow users to login with this source. (Optional)
Expand Down
2 changes: 2 additions & 0 deletions options/locale/locale_de-DE.ini
Original file line number Diff line number Diff line change
Expand Up @@ -2666,6 +2666,8 @@ auths.oauth2_profileURL=Profil-URL
auths.oauth2_emailURL=E-Mail-URL
auths.skip_local_two_fa=Lokale 2FA überspringen
auths.skip_local_two_fa_helper=Leer lassen bedeutet, dass lokale User die 2FA immer noch bestehen müssen, um sich anzumelden
auths.force_o_auth=Anmelden durch diese Quelle erzwingen
auths.force_o_auth_helper=Setzen um Anmeldungen automatisch auf diesen OAuth Anbieter umzuleiten
auths.oauth2_tenant=Inhaber
auths.oauth2_scopes=Zusätzliche Bereiche
auths.oauth2_required_claim_name=Benötigter Claim-Name
Expand Down
2 changes: 2 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -2753,6 +2753,8 @@ auths.oauth2_profileURL = Profile URL
auths.oauth2_emailURL = Email URL
auths.skip_local_two_fa = Skip local 2FA
auths.skip_local_two_fa_helper = Leaving unset means local users with 2FA set will still have to pass 2FA to log on
auths.force_o_auth = Force login via this authentication
auths.force_o_auth_helper = Set this to automatically redirect sign in to this OAuth provider
auths.oauth2_tenant = Tenant
auths.oauth2_scopes = Additional Scopes
auths.oauth2_required_claim_name = Required Claim Name
Expand Down
1 change: 1 addition & 0 deletions routers/web/admin/auths.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ func parseOAuth2Config(form forms.AuthenticationForm) *oauth2.Source {
RequiredClaimName: form.Oauth2RequiredClaimName,
RequiredClaimValue: form.Oauth2RequiredClaimValue,
SkipLocalTwoFA: form.SkipLocalTwoFA,
ForceOAuth: form.ForceOAuth,
GroupClaimName: form.Oauth2GroupClaimName,
RestrictedGroup: form.Oauth2RestrictedGroup,
AdminGroup: form.Oauth2AdminGroup,
Expand Down
31 changes: 31 additions & 0 deletions routers/web/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,32 @@ func checkAutoLogin(ctx *context.Context) bool {
return false
}

func checkForceOAuth(ctx *context.Context) bool {
// Check if authentication is forced to OAuth

authSources, err := auth.GetActiveOAuth2ProviderSources()
if err != nil {
return false
}

var OAuthList []int64

for _, source := range authSources {
if forced, ok := source.Cfg.(auth_service.ForceOAuth); ok && forced.IsOAuthForced() {
OAuthList = append(OAuthList, source.ID)
app, err := auth.GetOAuth2ApplicationByID(ctx, OAuthList[0])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it looks up an application to get the redirect URL, not an authentication source.

if err != nil {
return false
}
url := app.PrimaryRedirectURI()
ctx.Redirect(url)
Comment on lines +148 to +158
Copy link
Contributor

@brechtvl brechtvl Feb 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference, we simplified like this to avoid involving the application unnecessarily.

Suggested change
var OAuthList []int64
for _, source := range authSources {
if forced, ok := source.Cfg.(auth_service.ForceOAuth); ok && forced.IsOAuthForced() {
OAuthList = append(OAuthList, source.ID)
app, err := auth.GetOAuth2ApplicationByID(ctx, OAuthList[0])
if err != nil {
return false
}
url := app.PrimaryRedirectURI()
ctx.Redirect(url)
for _, source := range authSources {
if forced, ok := source.Cfg.(auth_service.ForceOAuth); ok && forced.IsOAuthForced() {
ctx.Redirect(setting.AppSubURL + "/user/oauth2/" + source.Name)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least for my test case (openID) this doesn't work because you still land on the local login page

return true
}
}

return false
}

// SignIn render sign in page
func SignIn(ctx *context.Context) {
ctx.Data["Title"] = ctx.Tr("sign_in")
Expand All @@ -146,6 +172,11 @@ func SignIn(ctx *context.Context) {
return
}

// Check if authentication is forced to OAuth
if checkForceOAuth(ctx) {
return
}

orderedOAuth2Names, oauth2Providers, err := oauth2.GetActiveOAuth2Providers()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Thank you so much!! Are you able to re-use this GetActiveOAuth2Providers call, to reduce the duplication of DB calls?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that but sadly the map of Providers that GetActiveOAuth2Providers returns is a different object that doesn't contain all the Source attributes, including my ForceOAuth parameter

if err != nil {
ctx.ServerError("UserSignIn", err)
Expand Down
4 changes: 4 additions & 0 deletions services/auth/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ type LocalTwoFASkipper interface {
IsSkipLocalTwoFA() bool
}

type ForceOAuth interface {
IsOAuthForced() bool
}

// SynchronizableSource represents a source that can synchronize users
type SynchronizableSource interface {
Sync(ctx context.Context, updateExisting bool) error
Expand Down
1 change: 1 addition & 0 deletions services/auth/source/oauth2/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ type Source struct {
OpenIDConnectAutoDiscoveryURL string
CustomURLMapping *CustomURLMapping
IconURL string
ForceOAuth bool `json:",omitempty"`

Scopes []string
RequiredClaimName string
Expand Down
4 changes: 4 additions & 0 deletions services/auth/source/oauth2/source_authenticate.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,9 @@ func (source *Source) Authenticate(user *user_model.User, login, password string
return db.Authenticate(user, login, password)
}

func (source *Source) IsOAuthForced() bool {
return source.ForceOAuth
}

// NB: Oauth2 does not implement LocalTwoFASkipper for password authentication
// as its password authentication drops to db authentication
1 change: 1 addition & 0 deletions services/forms/auth_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ type AuthenticationForm struct {
Oauth2GroupTeamMap string `binding:"ValidGroupTeamMap"`
Oauth2GroupTeamMapRemoval bool
SkipLocalTwoFA bool
ForceOAuth bool
SSPIAutoCreateUsers bool
SSPIAutoActivateUsers bool
SSPIStripDomainNames bool
Expand Down
7 changes: 7 additions & 0 deletions templates/admin/auth/edit.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,13 @@
<p class="help">{{.locale.Tr "admin.auths.skip_local_two_fa_helper"}}</p>
</div>
</div>
<div class="optional field">
<div class="ui checkbox">
<label for="force_o_auth"><strong>{{.locale.Tr "admin.auths.force_o_auth"}}</strong></label>
<input id="force_o_auth" name="force_o_auth" type="checkbox" {{if $cfg.ForceOAuth}}checked{{end}}>
<p class="help">{{.locale.Tr "admin.auths.force_o_auth_helper"}}</p>
</div>
</div>
<div class="oauth2_use_custom_url inline field">
<div class="ui checkbox">
<label><strong>{{.locale.Tr "admin.auths.oauth2_use_custom_url"}}</strong></label>
Expand Down
7 changes: 7 additions & 0 deletions templates/admin/auth/source/oauth.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@
<p class="help">{{.locale.Tr "admin.auths.skip_local_two_fa_helper"}}</p>
</div>
</div>
<div class="optional field">
<div class="ui checkbox">
<label for="force_o_auth"><strong>{{.locale.Tr "admin.auths.force_o_auth"}}</strong></label>
<input id="force_o_auth" name="force_o_auth" type="checkbox" {{if .force_o_auth}}checked{{end}}>
<p class="help">{{.locale.Tr "admin.auths.force_o_auth_helper"}}</p>
</div>
</div>

<div class="oauth2_use_custom_url inline field">
<div class="ui checkbox">
Expand Down