Skip to content

Commit edb618c

Browse files
GiteaBotzeripath
andauthored
Handle OpenID discovery URL errors a little nicer when creating/editing sources (#23397) (#23403)
Backport #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 <[email protected]> Co-authored-by: zeripath <[email protected]>
1 parent 43cf04c commit edb618c

File tree

5 files changed

+40
-2
lines changed

5 files changed

+40
-2
lines changed

cmd/admin.go

+10-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package cmd
77
import (
88
"errors"
99
"fmt"
10+
"net/url"
1011
"os"
1112
"strings"
1213
"text/tabwriter"
@@ -469,11 +470,19 @@ func runAddOauth(c *cli.Context) error {
469470
return err
470471
}
471472

473+
config := parseOAuth2Config(c)
474+
if config.Provider == "openidConnect" {
475+
discoveryURL, err := url.Parse(config.OpenIDConnectAutoDiscoveryURL)
476+
if err != nil || (discoveryURL.Scheme != "http" && discoveryURL.Scheme != "https") {
477+
return fmt.Errorf("invalid Auto Discovery URL: %s (this must be a valid URL starting with http:// or https://)", config.OpenIDConnectAutoDiscoveryURL)
478+
}
479+
}
480+
472481
return auth_model.CreateSource(&auth_model.Source{
473482
Type: auth_model.OAuth2,
474483
Name: c.String("name"),
475484
IsActive: true,
476-
Cfg: parseOAuth2Config(c),
485+
Cfg: config,
477486
})
478487
}
479488

options/locale/locale_en-US.ini

+2
Original file line numberDiff line numberDiff line change
@@ -2806,6 +2806,8 @@ auths.still_in_used = The authentication source is still in use. Convert or dele
28062806
auths.deletion_success = The authentication source has been deleted.
28072807
auths.login_source_exist = The authentication source '%s' already exists.
28082808
auths.login_source_of_type_exist = An authentication source of this type already exists.
2809+
auths.unable_to_initialize_openid = Unable to initialize OpenID Connect Provider: %s
2810+
auths.invalid_openIdConnectAutoDiscoveryURL = Invalid Auto Discovery URL (this must be a valid URL starting with http:// or https://)
28092811
28102812
config.server_config = Server Configuration
28112813
config.app_name = Site Title

routers/web/admin/auths.go

+23
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,15 @@ func NewAuthSourcePost(ctx *context.Context) {
271271
}
272272
case auth.OAuth2:
273273
config = parseOAuth2Config(form)
274+
oauth2Config := config.(*oauth2.Source)
275+
if oauth2Config.Provider == "openidConnect" {
276+
discoveryURL, err := url.Parse(oauth2Config.OpenIDConnectAutoDiscoveryURL)
277+
if err != nil || (discoveryURL.Scheme != "http" && discoveryURL.Scheme != "https") {
278+
ctx.Data["Err_DiscoveryURL"] = true
279+
ctx.RenderWithErr(ctx.Tr("admin.auths.invalid_openIdConnectAutoDiscoveryURL"), tplAuthNew, form)
280+
return
281+
}
282+
}
274283
case auth.SSPI:
275284
var err error
276285
config, err = parseSSPIConfig(ctx, form)
@@ -305,6 +314,10 @@ func NewAuthSourcePost(ctx *context.Context) {
305314
if auth.IsErrSourceAlreadyExist(err) {
306315
ctx.Data["Err_Name"] = true
307316
ctx.RenderWithErr(ctx.Tr("admin.auths.login_source_exist", err.(auth.ErrSourceAlreadyExist).Name), tplAuthNew, form)
317+
} else if oauth2.IsErrOpenIDConnectInitialize(err) {
318+
ctx.Data["Err_DiscoveryURL"] = true
319+
unwrapped := err.(oauth2.ErrOpenIDConnectInitialize).Unwrap()
320+
ctx.RenderWithErr(ctx.Tr("admin.auths.unable_to_initialize_openid", unwrapped), tplAuthNew, form)
308321
} else {
309322
ctx.ServerError("auth.CreateSource", err)
310323
}
@@ -389,6 +402,15 @@ func EditAuthSourcePost(ctx *context.Context) {
389402
}
390403
case auth.OAuth2:
391404
config = parseOAuth2Config(form)
405+
oauth2Config := config.(*oauth2.Source)
406+
if oauth2Config.Provider == "openidConnect" {
407+
discoveryURL, err := url.Parse(oauth2Config.OpenIDConnectAutoDiscoveryURL)
408+
if err != nil || (discoveryURL.Scheme != "http" && discoveryURL.Scheme != "https") {
409+
ctx.Data["Err_DiscoveryURL"] = true
410+
ctx.RenderWithErr(ctx.Tr("admin.auths.invalid_openIdConnectAutoDiscoveryURL"), tplAuthEdit, form)
411+
return
412+
}
413+
}
392414
case auth.SSPI:
393415
config, err = parseSSPIConfig(ctx, form)
394416
if err != nil {
@@ -408,6 +430,7 @@ func EditAuthSourcePost(ctx *context.Context) {
408430
if err := auth.UpdateSource(source); err != nil {
409431
if oauth2.IsErrOpenIDConnectInitialize(err) {
410432
ctx.Flash.Error(err.Error(), true)
433+
ctx.Data["Err_DiscoveryURL"] = true
411434
ctx.HTML(http.StatusOK, tplAuthEdit)
412435
} else {
413436
ctx.ServerError("UpdateSource", err)

services/auth/source/oauth2/source_register.go

+4
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ func (err ErrOpenIDConnectInitialize) Error() string {
3636
return fmt.Sprintf("Failed to initialize OpenID Connect Provider with name '%s' with url '%s': %v", err.ProviderName, err.OpenIDConnectAutoDiscoveryURL, err.Cause)
3737
}
3838

39+
func (err ErrOpenIDConnectInitialize) Unwrap() error {
40+
return err.Cause
41+
}
42+
3943
// wrapOpenIDConnectInitializeError is used to wrap the error but this cannot be done in modules/auth/oauth2
4044
// inside oauth2: import cycle not allowed models -> modules/auth/oauth2 -> models
4145
func wrapOpenIDConnectInitializeError(err error, providerName string, source *Source) error {

templates/admin/auth/source/oauth.tmpl

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
<label for="oauth2_icon_url">{{.locale.Tr "admin.auths.oauth2_icon_url"}}</label>
2525
<input id="oauth2_icon_url" name="oauth2_icon_url" value="{{.oauth2_icon_url}}">
2626
</div>
27-
<div class="open_id_connect_auto_discovery_url required field">
27+
<div class="open_id_connect_auto_discovery_url required field{{if .Err_DiscoveryURL}} error{{end}}">
2828
<label for="open_id_connect_auto_discovery_url">{{.locale.Tr "admin.auths.openIdConnectAutoDiscoveryURL"}}</label>
2929
<input id="open_id_connect_auto_discovery_url" name="open_id_connect_auto_discovery_url" value="{{.open_id_connect_auto_discovery_url}}">
3030
</div>

0 commit comments

Comments
 (0)