Handle OpenID discovery URL errors a little nicer when creating/editi…

…ng Sources When there is an error creating a new openIDConnect authentication source try to handle the error a little better. Close go-gitea#23283 Signed-off-by: Andrew Thornton <[email protected]>
zeripath · Mar 9, 2023 · aa1bae9 · aa1bae9
1 parent 542cec9
commit aa1bae9
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 2 deletions.
diff --git a/cmd/admin.go b/cmd/admin.go
@@ -7,6 +7,7 @@ package cmd
 import (
 	"errors"
 	"fmt"
+	"net/url"
 	"os"
 	"strings"
 	"text/tabwriter"
@@ -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,
 	})
 }
 

diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
@@ -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

diff --git a/routers/web/admin/auths.go b/routers/web/admin/auths.go
@@ -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)
@@ -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)
 		}
@@ -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 {
@@ -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)

diff --git a/services/auth/source/oauth2/source_register.go b/services/auth/source/oauth2/source_register.go
@@ -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 {

diff --git a/templates/admin/auth/source/oauth.tmpl b/templates/admin/auth/source/oauth.tmpl
@@ -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>