Skip to content

Commit 3dacc64

Browse files
cemalkiliccemalkilic
authored
feat(custom-oidc): support non-standard discovery URLs (#2573)
## What kind of change does this PR introduce? Feature ## What is the current behavior? `discoveryUrl` is only validated during provider creation (admin endpoint) ## What is the new behavior? use `discoveryUrl` while making the actual OIDC discovery too ## Summary - Adds `OIDCProviderCache.GetProviderFromURL` so custom OIDC providers can fetch their discovery document from any URL, not just `{issuer}/.well-known/openid-configuration`. - Custom OIDC providers now always route through this path; the URL is sourced from `CustomOAuthProvider.GetDiscoveryURL()` (which falls back to the standard path when no override is configured). - Bounds discovery fetches with a 10s HTTP timeout and verifies the document's `issuer` matches the configured one before constructing the provider.
1 parent 169ad67 commit 3dacc64

8 files changed

Lines changed: 552 additions & 77 deletions

File tree

‎internal/api/custom_oauth_admin.go‎

Lines changed: 25 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,9 @@ package api
22

33
import (
44
"context"
5-
"encoding/json"
6-
"io"
75
"net/http"
86
"slices"
97
"strings"
10-
"time"
118

129
"github.com/go-chi/chi/v5"
1310
popslices "github.com/gobuffalo/pop/v6/slices"
@@ -666,61 +663,34 @@ func validateAuthorizationParams(params map[string]interface{}) error {
666663
return nil
667664
}
668665

669-
// maxDiscoveryResponseSize is the maximum size of an OIDC discovery response body (1 MB).
670-
const maxDiscoveryResponseSize = 1 << 20
671-
672-
// discoveryFetchTimeout is the timeout for fetching an OIDC discovery document.
673-
const discoveryFetchTimeout = 10 * time.Second
674-
675-
// fetchAndValidateDiscovery fetches the OIDC discovery document from the
676-
// provider's discovery URL and validates that it contains the required fields
677-
// per the OpenID Connect Discovery 1.0 specification. It also verifies that
678-
// the issuer in the discovery document matches the expected issuer.
666+
// fetchAndValidateDiscovery fetches the OIDC discovery document via the shared
667+
// utility, then applies admin-only validation: required fields per the OpenID
668+
// Connect Discovery 1.0 spec must be present before we persist the document.
669+
// Returns *models.OIDCDiscovery so the caller can store it directly.
679670
func fetchAndValidateDiscovery(ctx context.Context, discoveryURL, expectedIssuer string) (*models.OIDCDiscovery, error) {
680-
resp, err := utilities.FetchURLWithTimeout(ctx, discoveryURL, discoveryFetchTimeout)
671+
doc, err := utilities.FetchAndValidateOIDCDiscovery(ctx, discoveryURL, expectedIssuer)
681672
if err != nil {
682673
return nil, apierrors.NewBadRequestError(
683674
apierrors.ErrorCodeValidationFailed,
684-
"Failed to fetch OIDC discovery document from %q: %v", discoveryURL, err,
685-
)
686-
}
687-
defer resp.Body.Close()
688-
689-
if resp.StatusCode != http.StatusOK {
690-
return nil, apierrors.NewBadRequestError(
691-
apierrors.ErrorCodeValidationFailed,
692-
"OIDC discovery endpoint %q returned HTTP %d, expected 200", discoveryURL, resp.StatusCode,
675+
"OIDC discovery from %q failed: %v", discoveryURL, err,
693676
)
694677
}
695678

696-
body, err := io.ReadAll(io.LimitReader(resp.Body, maxDiscoveryResponseSize))
697-
if err != nil {
698-
return nil, apierrors.NewBadRequestError(
699-
apierrors.ErrorCodeValidationFailed,
700-
"Failed to read OIDC discovery response from %q", discoveryURL,
701-
)
702-
}
703-
704-
var discovery models.OIDCDiscovery
705-
if err := json.Unmarshal(body, &discovery); err != nil {
706-
return nil, apierrors.NewBadRequestError(
707-
apierrors.ErrorCodeValidationFailed,
708-
"OIDC discovery document from %q is not valid JSON", discoveryURL,
709-
)
710-
}
711-
712-
// Validate required fields per OpenID Connect Discovery 1.0 spec
679+
// Validate required fields per OpenID Connect Discovery 1.0 spec.
680+
// Stricter than the runtime path needs — runtime can tolerate missing
681+
// fields and surface a less helpful error at login; admin should reject
682+
// the configuration up front.
713683
var missing []string
714-
if discovery.Issuer == "" {
684+
if doc.Issuer == "" {
715685
missing = append(missing, "issuer")
716686
}
717-
if discovery.AuthorizationEndpoint == "" {
687+
if doc.AuthorizationEndpoint == "" {
718688
missing = append(missing, "authorization_endpoint")
719689
}
720-
if discovery.TokenEndpoint == "" {
690+
if doc.TokenEndpoint == "" {
721691
missing = append(missing, "token_endpoint")
722692
}
723-
if discovery.JwksURI == "" {
693+
if doc.JwksURI == "" {
724694
missing = append(missing, "jwks_uri")
725695
}
726696
if len(missing) > 0 {
@@ -730,16 +700,17 @@ func fetchAndValidateDiscovery(ctx context.Context, discoveryURL, expectedIssuer
730700
)
731701
}
732702

733-
// The issuer in the discovery document MUST exactly match the expected issuer
734-
// per OpenID Connect Discovery 1.0, Section 4.3.
735-
if discovery.Issuer != expectedIssuer {
736-
return nil, apierrors.NewBadRequestError(
737-
apierrors.ErrorCodeValidationFailed,
738-
"OIDC discovery issuer mismatch: discovery document reports %q but expected %q", discovery.Issuer, expectedIssuer,
739-
)
740-
}
741-
742-
return &discovery, nil
703+
return &models.OIDCDiscovery{
704+
Issuer: doc.Issuer,
705+
AuthorizationEndpoint: doc.AuthorizationEndpoint,
706+
TokenEndpoint: doc.TokenEndpoint,
707+
UserinfoEndpoint: doc.UserinfoEndpoint,
708+
JwksURI: doc.JwksURI,
709+
ScopesSupported: doc.ScopesSupported,
710+
ResponseTypesSupported: doc.ResponseTypesSupported,
711+
GrantTypesSupported: doc.GrantTypesSupported,
712+
SubjectTypesSupported: doc.SubjectTypesSupported,
713+
}, nil
743714
}
744715

745716
// validateAttributeMapping ensures no sensitive system fields are targeted

‎internal/api/external.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -690,8 +690,7 @@ func (a *API) loadCustomProvider(ctx context.Context, db *storage.Connection, id
690690
config := a.config
691691
var pConfig conf.OAuthProviderConfiguration
692692

693-
// Build the redirect URL
694-
redirectURL := config.API.ExternalURL + "/callback"
693+
redirectURL := strings.TrimRight(config.API.ExternalURL, "/") + "/callback"
695694

696695
// Parse scopes (space-separated per RFC 6749)
697696
var scopeList []string
@@ -765,14 +764,14 @@ func (a *API) loadCustomProvider(ctx context.Context, db *storage.Connection, id
765764
}
766765

767766
// Create custom OIDC provider instance
768-
// oidc.NewProvider() will automatically fetch discovery document
769767
p, err := provider.NewCustomOIDCProvider(
770768
ctx,
771769
customProvider.ClientID,
772770
clientSecret,
773771
redirectURL,
774772
scopeList,
775773
*customProvider.Issuer,
774+
customProvider.GetDiscoveryURL(),
776775
customProvider.PKCEEnabled,
777776
customProvider.AcceptableClientIDs,
778777
customProvider.AttributeMapping,

‎internal/api/provider/custom_oauth.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,16 @@ type CustomOIDCProvider struct {
110110
authorizationParams map[string]interface{}
111111
}
112112

113-
// NewCustomOIDCProvider creates a new custom OIDC provider
113+
// NewCustomOIDCProvider creates a new custom OIDC provider.
114+
// discoveryURL is the URL to fetch the OIDC discovery document from
115+
// (typically {issuer}/.well-known/openid-configuration, or an admin-configured
116+
// override).
114117
func NewCustomOIDCProvider(
115118
ctx context.Context,
116119
clientID, clientSecret, redirectURL string,
117120
scopes []string,
118121
issuer string,
122+
discoveryURL string,
119123
pkceEnabled bool,
120124
acceptableClientIDs []string,
121125
attributeMapping, authorizationParams map[string]interface{},
@@ -133,8 +137,7 @@ func NewCustomOIDCProvider(
133137
scopes = append([]string{"openid"}, scopes...)
134138
}
135139

136-
// Create OIDC provider - uses cache to avoid redundant discovery fetches
137-
oidcProvider, err := cache.GetProvider(ctx, issuer)
140+
oidcProvider, err := cache.GetProviderFromURL(ctx, issuer, discoveryURL)
138141
if err != nil {
139142
return nil, fmt.Errorf("failed to create OIDC provider: %w", err)
140143
}

‎internal/api/provider/custom_oauth_test.go‎

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"net/http"
77
"net/http/httptest"
88
"testing"
9+
"time"
910

1011
"github.com/stretchr/testify/assert"
1112
"github.com/stretchr/testify/require"
@@ -274,11 +275,12 @@ func TestNewCustomOIDCProvider(t *testing.T) {
274275
"https://myapp.com/callback",
275276
[]string{"profile", "email"}, // Without openid
276277
server.URL, // issuer
278+
server.URL + "/.well-known/openid-configuration",
277279
true, // PKCE enabled
278280
[]string{"ios-client", "android-client"},
279281
map[string]interface{}{"email": "user_email"},
280282
map[string]interface{}{"prompt": "consent"},
281-
NewOIDCProviderCache(0),
283+
newTestOIDCProviderCache(t, 0),
282284
)
283285

284286
require.NoError(t, err)
@@ -403,6 +405,7 @@ func TestCustomOIDCProvider_AuthCodeURL(t *testing.T) {
403405
"https://myapp.com/callback",
404406
[]string{"openid", "profile"},
405407
server.URL, // issuer
408+
server.URL + "/.well-known/openid-configuration",
406409
false,
407410
nil,
408411
nil,
@@ -412,7 +415,7 @@ func TestCustomOIDCProvider_AuthCodeURL(t *testing.T) {
412415
"ui_locales": "en",
413416
"login_hint": "user@example.com",
414417
},
415-
NewOIDCProviderCache(0),
418+
newTestOIDCProviderCache(t, 0),
416419
)
417420

418421
require.NoError(t, err)
@@ -431,6 +434,48 @@ func TestCustomOIDCProvider_AuthCodeURL(t *testing.T) {
431434
assert.Contains(t, authURL, "login_hint=user")
432435
}
433436

437+
func TestNewCustomOIDCProvider_CustomDiscoveryURL(t *testing.T) {
438+
var server *httptest.Server
439+
server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
440+
switch r.URL.Path {
441+
case "/custom-discovery":
442+
w.Header().Set("Content-Type", "application/json")
443+
json.NewEncoder(w).Encode(map[string]interface{}{
444+
"issuer": server.URL,
445+
"authorization_endpoint": server.URL + "/custom-authorize",
446+
"token_endpoint": server.URL + "/custom-token",
447+
"userinfo_endpoint": server.URL + "/custom-userinfo",
448+
"jwks_uri": server.URL + "/jwks",
449+
})
450+
case "/jwks":
451+
w.Header().Set("Content-Type", "application/json")
452+
json.NewEncoder(w).Encode(map[string]interface{}{"keys": []interface{}{}})
453+
default:
454+
http.NotFound(w, r)
455+
}
456+
}))
457+
defer server.Close()
458+
459+
p, err := NewCustomOIDCProvider(
460+
context.Background(),
461+
"test-client-id",
462+
"test-secret",
463+
"https://myapp.com/callback",
464+
[]string{"openid"},
465+
server.URL,
466+
server.URL+"/custom-discovery",
467+
false,
468+
nil, nil, nil,
469+
newTestOIDCProviderCache(t, time.Hour),
470+
)
471+
require.NoError(t, err)
472+
require.NotNil(t, p)
473+
474+
assert.Equal(t, server.URL+"/custom-authorize", p.config.Endpoint.AuthURL)
475+
assert.Equal(t, server.URL+"/custom-token", p.config.Endpoint.TokenURL)
476+
assert.Equal(t, server.URL+"/custom-userinfo", p.userinfoEndpoint)
477+
}
478+
434479
func TestCustomOIDCProvider_RequiresPKCE(t *testing.T) {
435480
// Mock OIDC provider server
436481
var server *httptest.Server
@@ -461,11 +506,12 @@ func TestCustomOIDCProvider_RequiresPKCE(t *testing.T) {
461506
"https://myapp.com/callback",
462507
[]string{"openid"},
463508
server.URL, // issuer
509+
server.URL + "/.well-known/openid-configuration",
464510
true, // PKCE enabled
465511
nil,
466512
nil,
467513
nil,
468-
NewOIDCProviderCache(0),
514+
newTestOIDCProviderCache(t, 0),
469515
)
470516

471517
require.NoError(t, err)
@@ -481,11 +527,12 @@ func TestCustomOIDCProvider_RequiresPKCE(t *testing.T) {
481527
"https://myapp.com/callback",
482528
[]string{"openid"},
483529
server.URL, // issuer
530+
server.URL + "/.well-known/openid-configuration",
484531
false, // PKCE disabled
485532
nil,
486533
nil,
487534
nil,
488-
NewOIDCProviderCache(0),
535+
newTestOIDCProviderCache(t, 0),
489536
)
490537

491538
require.NoError(t, err)

0 commit comments

Comments
 (0)