Handle OpenID discovery URL errors a little nicer when creating/editing sources (#23397)

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,
  })
 }
 

options/locale/locale_en-US.ini

@@ -2806,6 +2806,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

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)

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 {

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>