Skip to content

Commit

Permalink
Handle OpenID discovery URL errors a little nicer when creating/editi…
Browse files Browse the repository at this point in the history
…ng sources (#23397)

When there is an error creating a new openIDConnect authentication
source try to handle the error a little better.

Close #23283

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>
  • Loading branch information
zeripath and techknowlogick authored Mar 10, 2023
1 parent f92e0a4 commit dad057b
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 2 deletions.
11 changes: 10 additions & 1 deletion cmd/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package cmd
import (
"errors"
"fmt"
"net/url"
"os"
"strings"
"text/tabwriter"
Expand Down Expand Up @@ -469,11 +470,19 @@ func runAddOauth(c *cli.Context) error {
return err
}

config := parseOAuth2Config(c)
if config.Provider == "openidConnect" {
discoveryURL, err := url.Parse(config.OpenIDConnectAutoDiscoveryURL)
if err != nil || (discoveryURL.Scheme != "http" && discoveryURL.Scheme != "https") {
return fmt.Errorf("invalid Auto Discovery URL: %s (this must be a valid URL starting with http:// or https://)", config.OpenIDConnectAutoDiscoveryURL)
}
}

return auth_model.CreateSource(&auth_model.Source{
Type: auth_model.OAuth2,
Name: c.String("name"),
IsActive: true,
Cfg: parseOAuth2Config(c),
Cfg: config,
})
}

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 @@ -2808,6 +2808,8 @@ auths.still_in_used = The authentication source is still in use. Convert or dele
auths.deletion_success = The authentication source has been deleted.
auths.login_source_exist = The authentication source '%s' already exists.
auths.login_source_of_type_exist = An authentication source of this type already exists.
auths.unable_to_initialize_openid = Unable to initialize OpenID Connect Provider: %s
auths.invalid_openIdConnectAutoDiscoveryURL = Invalid Auto Discovery URL (this must be a valid URL starting with http:// or https://)
config.server_config = Server Configuration
config.app_name = Site Title
Expand Down
23 changes: 23 additions & 0 deletions routers/web/admin/auths.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,15 @@ func NewAuthSourcePost(ctx *context.Context) {
}
case auth.OAuth2:
config = parseOAuth2Config(form)
oauth2Config := config.(*oauth2.Source)
if oauth2Config.Provider == "openidConnect" {
discoveryURL, err := url.Parse(oauth2Config.OpenIDConnectAutoDiscoveryURL)
if err != nil || (discoveryURL.Scheme != "http" && discoveryURL.Scheme != "https") {
ctx.Data["Err_DiscoveryURL"] = true
ctx.RenderWithErr(ctx.Tr("admin.auths.invalid_openIdConnectAutoDiscoveryURL"), tplAuthNew, form)
return
}
}
case auth.SSPI:
var err error
config, err = parseSSPIConfig(ctx, form)
Expand Down Expand Up @@ -305,6 +314,10 @@ func NewAuthSourcePost(ctx *context.Context) {
if auth.IsErrSourceAlreadyExist(err) {
ctx.Data["Err_Name"] = true
ctx.RenderWithErr(ctx.Tr("admin.auths.login_source_exist", err.(auth.ErrSourceAlreadyExist).Name), tplAuthNew, form)
} else if oauth2.IsErrOpenIDConnectInitialize(err) {
ctx.Data["Err_DiscoveryURL"] = true
unwrapped := err.(oauth2.ErrOpenIDConnectInitialize).Unwrap()
ctx.RenderWithErr(ctx.Tr("admin.auths.unable_to_initialize_openid", unwrapped), tplAuthNew, form)
} else {
ctx.ServerError("auth.CreateSource", err)
}
Expand Down Expand Up @@ -389,6 +402,15 @@ func EditAuthSourcePost(ctx *context.Context) {
}
case auth.OAuth2:
config = parseOAuth2Config(form)
oauth2Config := config.(*oauth2.Source)
if oauth2Config.Provider == "openidConnect" {
discoveryURL, err := url.Parse(oauth2Config.OpenIDConnectAutoDiscoveryURL)
if err != nil || (discoveryURL.Scheme != "http" && discoveryURL.Scheme != "https") {
ctx.Data["Err_DiscoveryURL"] = true
ctx.RenderWithErr(ctx.Tr("admin.auths.invalid_openIdConnectAutoDiscoveryURL"), tplAuthEdit, form)
return
}
}
case auth.SSPI:
config, err = parseSSPIConfig(ctx, form)
if err != nil {
Expand All @@ -408,6 +430,7 @@ func EditAuthSourcePost(ctx *context.Context) {
if err := auth.UpdateSource(source); err != nil {
if oauth2.IsErrOpenIDConnectInitialize(err) {
ctx.Flash.Error(err.Error(), true)
ctx.Data["Err_DiscoveryURL"] = true
ctx.HTML(http.StatusOK, tplAuthEdit)
} else {
ctx.ServerError("UpdateSource", err)
Expand Down
4 changes: 4 additions & 0 deletions services/auth/source/oauth2/source_register.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ func (err ErrOpenIDConnectInitialize) Error() string {
return fmt.Sprintf("Failed to initialize OpenID Connect Provider with name '%s' with url '%s': %v", err.ProviderName, err.OpenIDConnectAutoDiscoveryURL, err.Cause)
}

func (err ErrOpenIDConnectInitialize) Unwrap() error {
return err.Cause
}

// wrapOpenIDConnectInitializeError is used to wrap the error but this cannot be done in modules/auth/oauth2
// inside oauth2: import cycle not allowed models -> modules/auth/oauth2 -> models
func wrapOpenIDConnectInitializeError(err error, providerName string, source *Source) error {
Expand Down
2 changes: 1 addition & 1 deletion templates/admin/auth/source/oauth.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
<label for="oauth2_icon_url">{{.locale.Tr "admin.auths.oauth2_icon_url"}}</label>
<input id="oauth2_icon_url" name="oauth2_icon_url" value="{{.oauth2_icon_url}}">
</div>
<div class="open_id_connect_auto_discovery_url required field">
<div class="open_id_connect_auto_discovery_url required field{{if .Err_DiscoveryURL}} error{{end}}">
<label for="open_id_connect_auto_discovery_url">{{.locale.Tr "admin.auths.openIdConnectAutoDiscoveryURL"}}</label>
<input id="open_id_connect_auto_discovery_url" name="open_id_connect_auto_discovery_url" value="{{.open_id_connect_auto_discovery_url}}">
</div>
Expand Down

0 comments on commit dad057b

Please sign in to comment.