From 40be3437f961830dba35d03c7fc76c5b86c57a3e Mon Sep 17 00:00:00 2001 From: bsaurusrex <82356519+bsaurusrex@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:05:35 +0800 Subject: [PATCH 1/6] feat: support OIDC discovery for generic OAuth providers Adds an optional issuer field to a generic OAuth provider. When set, any OAuth endpoint left empty (authUrl, tokenUrl, userinfoUrl) is filled from the issuer's /.well-known/openid-configuration document at startup, so a spec-compliant provider can be configured with just an issuer, client ID and secret. Explicitly configured endpoints are never overwritten and a provider with no issuer is unchanged, so this is backwards compatible. Discovery fails soft: an error is logged and the configured endpoints are used as before. Preset providers (google, github) are unaffected. Closes #974 Co-Authored-By: Claude Opus 5.5 --- internal/model/config.go | 1 + internal/service/oauth_broker_service.go | 8 +- internal/service/oauth_discovery.go | 87 +++++++++++++++++ internal/service/oauth_discovery_test.go | 116 +++++++++++++++++++++++ 4 files changed, 211 insertions(+), 1 deletion(-) create mode 100644 internal/service/oauth_discovery.go create mode 100644 internal/service/oauth_discovery_test.go diff --git a/internal/model/config.go b/internal/model/config.go index 9d5143904..5893d87f7 100644 --- a/internal/model/config.go +++ b/internal/model/config.go @@ -262,6 +262,7 @@ type OAuthServiceConfig struct { WhitelistFile string `description:"Path to the OAuth whitelist file for this provider." yaml:"whitelistFile,omitempty"` Scopes []string `description:"OAuth scopes." yaml:"scopes,omitempty"` RedirectURL string `description:"OAuth redirect URL." yaml:"redirectUrl,omitempty"` + Issuer string `description:"OIDC issuer URL. When set, any OAuth endpoint left empty is filled from its /.well-known/openid-configuration document." yaml:"issuer,omitempty"` AuthURL string `description:"OAuth authorization URL." yaml:"authUrl,omitempty"` TokenURL string `description:"OAuth token URL." yaml:"tokenUrl,omitempty"` UserinfoURL string `description:"OAuth userinfo URL." yaml:"userinfoUrl,omitempty"` diff --git a/internal/service/oauth_broker_service.go b/internal/service/oauth_broker_service.go index 4df0e8258..8f38cb012 100644 --- a/internal/service/oauth_broker_service.go +++ b/internal/service/oauth_broker_service.go @@ -55,7 +55,13 @@ func NewOAuthBrokerService(i OAuthBrokerServiceInput) *OAuthBrokerService { service.services[name] = presetFunc(cfg, i.Ctx) service.log.App.Debug().Str("service", name).Msg("Loaded OAuth service from preset") } else { - service.services[name] = NewOAuthService(cfg, name, i.Ctx) + resolved, err := resolveOIDCDiscovery(cfg, i.Ctx) + + if err != nil { + service.log.App.Warn().Err(err).Str("service", name).Msg("Failed to resolve OIDC discovery document, using the configured endpoints") + } + + service.services[name] = NewOAuthService(resolved, name, i.Ctx) service.log.App.Debug().Str("service", name).Msg("Loaded OAuth service from custom config") } } diff --git a/internal/service/oauth_discovery.go b/internal/service/oauth_discovery.go new file mode 100644 index 000000000..2f6f0f868 --- /dev/null +++ b/internal/service/oauth_discovery.go @@ -0,0 +1,87 @@ +package service + +import ( + "context" + "crypto/tls" + "encoding/json" + "fmt" + "net/http" + "strings" + "time" + + "github.com/tinyauthapp/tinyauth/internal/model" +) + +// oidcDiscoveryDocument holds the endpoints Tinyauth can fill from an OIDC provider's well-known +// configuration (https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata). +type oidcDiscoveryDocument struct { + AuthorizationEndpoint string `json:"authorization_endpoint"` + TokenEndpoint string `json:"token_endpoint"` + UserinfoEndpoint string `json:"userinfo_endpoint"` +} + +// resolveOIDCDiscovery fills any OAuth endpoint (authorization, token, userinfo) left empty from the +// provider's OIDC discovery document when an issuer is configured. Explicitly configured endpoints are +// never overwritten, so a provider with all endpoints set (or no issuer) is returned unchanged and the +// behaviour stays backwards compatible. It fails soft: on any error the original config is returned with +// the error, so startup continues and the existing "missing endpoint" handling surfaces later. +func resolveOIDCDiscovery(cfg model.OAuthServiceConfig, ctx context.Context) (model.OAuthServiceConfig, error) { + if cfg.Issuer == "" { + return cfg, nil + } + + if cfg.AuthURL != "" && cfg.TokenURL != "" && cfg.UserinfoURL != "" { + return cfg, nil + } + + url := strings.TrimRight(cfg.Issuer, "/") + "/.well-known/openid-configuration" + + client := &http.Client{ + Timeout: 30 * time.Second, + Transport: &http.Transport{ + Proxy: http.ProxyFromEnvironment, + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: cfg.Insecure, + MinVersion: tls.VersionTLS12, + }, + }, + } + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + + if err != nil { + return cfg, fmt.Errorf("failed to build OIDC discovery request: %w", err) + } + + resp, err := client.Do(req) + + if err != nil { + return cfg, fmt.Errorf("failed to fetch OIDC discovery document: %w", err) + } + + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return cfg, fmt.Errorf("OIDC discovery document returned status %d", resp.StatusCode) + } + + var doc oidcDiscoveryDocument + + if err := json.NewDecoder(resp.Body).Decode(&doc); err != nil { + return cfg, fmt.Errorf("failed to decode OIDC discovery document: %w", err) + } + + if cfg.AuthURL == "" { + cfg.AuthURL = doc.AuthorizationEndpoint + } + + if cfg.TokenURL == "" { + cfg.TokenURL = doc.TokenEndpoint + } + + if cfg.UserinfoURL == "" { + cfg.UserinfoURL = doc.UserinfoEndpoint + } + + return cfg, nil +} diff --git a/internal/service/oauth_discovery_test.go b/internal/service/oauth_discovery_test.go new file mode 100644 index 000000000..8497ce016 --- /dev/null +++ b/internal/service/oauth_discovery_test.go @@ -0,0 +1,116 @@ +package service + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/tinyauthapp/tinyauth/internal/model" +) + +func discoveryTestServer(t *testing.T, status int, body string) *httptest.Server { + t.Helper() + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _, _ = w.Write([]byte(body)) + }) + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + return server +} + +func TestResolveOIDCDiscovery(t *testing.T) { + t.Run("no issuer is returned unchanged", func(t *testing.T) { + cfg := model.OAuthServiceConfig{ClientID: "abc"} + + got, err := resolveOIDCDiscovery(cfg, context.Background()) + + require.NoError(t, err) + assert.Equal(t, cfg, got) + }) + + t.Run("fills missing endpoints from the discovery document", func(t *testing.T) { + server := discoveryTestServer(t, http.StatusOK, `{ + "authorization_endpoint": "https://idp.example.com/authorize", + "token_endpoint": "https://idp.example.com/token", + "userinfo_endpoint": "https://idp.example.com/userinfo" + }`) + + cfg := model.OAuthServiceConfig{Issuer: server.URL} + + got, err := resolveOIDCDiscovery(cfg, context.Background()) + + require.NoError(t, err) + assert.Equal(t, "https://idp.example.com/authorize", got.AuthURL) + assert.Equal(t, "https://idp.example.com/token", got.TokenURL) + assert.Equal(t, "https://idp.example.com/userinfo", got.UserinfoURL) + }) + + t.Run("does not overwrite explicitly configured endpoints", func(t *testing.T) { + server := discoveryTestServer(t, http.StatusOK, `{ + "authorization_endpoint": "https://idp.example.com/authorize", + "token_endpoint": "https://idp.example.com/token", + "userinfo_endpoint": "https://idp.example.com/userinfo" + }`) + + cfg := model.OAuthServiceConfig{ + Issuer: server.URL, + AuthURL: "https://custom.example.com/auth", + TokenURL: "https://custom.example.com/token", + // UserinfoURL is left empty, so only it should be filled + } + + got, err := resolveOIDCDiscovery(cfg, context.Background()) + + require.NoError(t, err) + assert.Equal(t, "https://custom.example.com/auth", got.AuthURL) + assert.Equal(t, "https://custom.example.com/token", got.TokenURL) + assert.Equal(t, "https://idp.example.com/userinfo", got.UserinfoURL) + }) + + t.Run("skips discovery when all endpoints are already set", func(t *testing.T) { + // The issuer points at a server that always errors; discovery must not be attempted. + server := discoveryTestServer(t, http.StatusInternalServerError, "boom") + + cfg := model.OAuthServiceConfig{ + Issuer: server.URL, + AuthURL: "https://custom.example.com/auth", + TokenURL: "https://custom.example.com/token", + UserinfoURL: "https://custom.example.com/userinfo", + } + + got, err := resolveOIDCDiscovery(cfg, context.Background()) + + require.NoError(t, err) + assert.Equal(t, cfg, got) + }) + + t.Run("fails soft on a non-200 response", func(t *testing.T) { + server := discoveryTestServer(t, http.StatusNotFound, "not found") + + cfg := model.OAuthServiceConfig{Issuer: server.URL} + + got, err := resolveOIDCDiscovery(cfg, context.Background()) + + require.Error(t, err) + assert.Empty(t, got.AuthURL) + assert.Empty(t, got.TokenURL) + assert.Empty(t, got.UserinfoURL) + }) + + t.Run("fails soft on an invalid document", func(t *testing.T) { + server := discoveryTestServer(t, http.StatusOK, "not json") + + cfg := model.OAuthServiceConfig{Issuer: server.URL} + + got, err := resolveOIDCDiscovery(cfg, context.Background()) + + require.Error(t, err) + assert.Empty(t, got.AuthURL) + }) +} From 02a4578b56f59318088d49fcbb98fcbd9ae25118 Mon Sep 17 00:00:00 2001 From: bsaurusrex <82356519+bsaurusrex@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:12:33 +0800 Subject: [PATCH 2/6] fix: cap the OIDC discovery response body size Read at most 1 MiB of the discovery document via io.LimitReader so a slow or hostile issuer cannot exhaust memory with an unbounded response body. A truncated body surfaces as the existing fail-soft decode error. Co-Authored-By: Claude Opus 5.5 --- internal/service/oauth_discovery.go | 7 ++++++- internal/service/oauth_discovery_test.go | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/internal/service/oauth_discovery.go b/internal/service/oauth_discovery.go index 2f6f0f868..9744fa483 100644 --- a/internal/service/oauth_discovery.go +++ b/internal/service/oauth_discovery.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "encoding/json" "fmt" + "io" "net/http" "strings" "time" @@ -12,6 +13,10 @@ import ( "github.com/tinyauthapp/tinyauth/internal/model" ) +// maxDiscoveryBodyBytes caps how much of a discovery document is read, so a slow or hostile issuer +// cannot exhaust memory with an unbounded response body. +const maxDiscoveryBodyBytes = 1 << 20 // 1 MiB + // oidcDiscoveryDocument holds the endpoints Tinyauth can fill from an OIDC provider's well-known // configuration (https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata). type oidcDiscoveryDocument struct { @@ -67,7 +72,7 @@ func resolveOIDCDiscovery(cfg model.OAuthServiceConfig, ctx context.Context) (mo var doc oidcDiscoveryDocument - if err := json.NewDecoder(resp.Body).Decode(&doc); err != nil { + if err := json.NewDecoder(io.LimitReader(resp.Body, maxDiscoveryBodyBytes)).Decode(&doc); err != nil { return cfg, fmt.Errorf("failed to decode OIDC discovery document: %w", err) } diff --git a/internal/service/oauth_discovery_test.go b/internal/service/oauth_discovery_test.go index 8497ce016..223f9e9c8 100644 --- a/internal/service/oauth_discovery_test.go +++ b/internal/service/oauth_discovery_test.go @@ -4,6 +4,7 @@ import ( "context" "net/http" "net/http/httptest" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -113,4 +114,19 @@ func TestResolveOIDCDiscovery(t *testing.T) { require.Error(t, err) assert.Empty(t, got.AuthURL) }) + + t.Run("fails soft on an oversized body instead of exhausting memory", func(t *testing.T) { + // Pad the document past the read cap so the body cannot be fully consumed; the truncated + // read must surface as a decode error rather than an unbounded allocation. + padding := strings.Repeat(" ", (2<<20)+1) + body := `{"authorization_endpoint": "https://idp.example.com/authorize"` + padding + `}` + server := discoveryTestServer(t, http.StatusOK, body) + + cfg := model.OAuthServiceConfig{Issuer: server.URL} + + got, err := resolveOIDCDiscovery(cfg, context.Background()) + + require.Error(t, err) + assert.Empty(t, got.AuthURL) + }) } From 6e25d792e036cbb96ca48108587cde0469a56e1e Mon Sep 17 00:00:00 2001 From: bsaurusrex <82356519+bsaurusrex@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:20:48 +0800 Subject: [PATCH 3/6] feat: validate the issuer in the OIDC discovery document Reject a discovery document whose issuer does not match the configured issuer (OIDC Discovery 1.0 section 4.3, RFC 8414 section 3.3), so a substitution or mix-up cannot repoint the endpoints (the token endpoint receives the client secret) at an unexpected provider. A trailing slash is not significant. Also document that discovery uses the provider's TLS settings, so 'insecure' disables certificate verification for it too. Co-Authored-By: Claude Opus 5.5 --- internal/model/config.go | 2 +- internal/service/oauth_discovery.go | 9 +++ internal/service/oauth_discovery_test.go | 91 +++++++++++++++++++----- 3 files changed, 83 insertions(+), 19 deletions(-) diff --git a/internal/model/config.go b/internal/model/config.go index 5893d87f7..b4c7178fe 100644 --- a/internal/model/config.go +++ b/internal/model/config.go @@ -262,7 +262,7 @@ type OAuthServiceConfig struct { WhitelistFile string `description:"Path to the OAuth whitelist file for this provider." yaml:"whitelistFile,omitempty"` Scopes []string `description:"OAuth scopes." yaml:"scopes,omitempty"` RedirectURL string `description:"OAuth redirect URL." yaml:"redirectUrl,omitempty"` - Issuer string `description:"OIDC issuer URL. When set, any OAuth endpoint left empty is filled from its /.well-known/openid-configuration document." yaml:"issuer,omitempty"` + Issuer string `description:"OIDC issuer URL. When set, any OAuth endpoint left empty is filled from its /.well-known/openid-configuration document at startup. The document is fetched with this provider's TLS settings, so enabling 'insecure' also disables certificate verification for discovery, and the issuer in the document must match this value." yaml:"issuer,omitempty"` AuthURL string `description:"OAuth authorization URL." yaml:"authUrl,omitempty"` TokenURL string `description:"OAuth token URL." yaml:"tokenUrl,omitempty"` UserinfoURL string `description:"OAuth userinfo URL." yaml:"userinfoUrl,omitempty"` diff --git a/internal/service/oauth_discovery.go b/internal/service/oauth_discovery.go index 9744fa483..21505cf29 100644 --- a/internal/service/oauth_discovery.go +++ b/internal/service/oauth_discovery.go @@ -20,6 +20,7 @@ const maxDiscoveryBodyBytes = 1 << 20 // 1 MiB // oidcDiscoveryDocument holds the endpoints Tinyauth can fill from an OIDC provider's well-known // configuration (https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata). type oidcDiscoveryDocument struct { + Issuer string `json:"issuer"` AuthorizationEndpoint string `json:"authorization_endpoint"` TokenEndpoint string `json:"token_endpoint"` UserinfoEndpoint string `json:"userinfo_endpoint"` @@ -76,6 +77,14 @@ func resolveOIDCDiscovery(cfg model.OAuthServiceConfig, ctx context.Context) (mo return cfg, fmt.Errorf("failed to decode OIDC discovery document: %w", err) } + // The issuer in the document MUST match the configured issuer (OIDC Discovery 1.0 section 4.3, + // RFC 8414 section 3.3). Rejecting a mismatch prevents a substitution/mix-up attack from pointing + // the endpoints (the token endpoint receives the client secret) at an unexpected provider. A + // trailing slash is not significant, so it is ignored. + if strings.TrimRight(doc.Issuer, "/") != strings.TrimRight(cfg.Issuer, "/") { + return cfg, fmt.Errorf("OIDC discovery issuer mismatch: document reports %q, expected %q", doc.Issuer, cfg.Issuer) + } + if cfg.AuthURL == "" { cfg.AuthURL = doc.AuthorizationEndpoint } diff --git a/internal/service/oauth_discovery_test.go b/internal/service/oauth_discovery_test.go index 223f9e9c8..7202c71a2 100644 --- a/internal/service/oauth_discovery_test.go +++ b/internal/service/oauth_discovery_test.go @@ -12,15 +12,18 @@ import ( "github.com/tinyauthapp/tinyauth/internal/model" ) -func discoveryTestServer(t *testing.T, status int, body string) *httptest.Server { +// discoveryTestServer serves a discovery document at the well-known path. docFn receives the server's +// own URL so a document can advertise a matching (or deliberately mismatched) issuer. +func discoveryTestServer(t *testing.T, status int, docFn func(issuer string) string) *httptest.Server { t.Helper() + var server *httptest.Server mux := http.NewServeMux() mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(status) - _, _ = w.Write([]byte(body)) + _, _ = w.Write([]byte(docFn(server.URL))) }) - server := httptest.NewServer(mux) + server = httptest.NewServer(mux) t.Cleanup(server.Close) return server } @@ -36,11 +39,14 @@ func TestResolveOIDCDiscovery(t *testing.T) { }) t.Run("fills missing endpoints from the discovery document", func(t *testing.T) { - server := discoveryTestServer(t, http.StatusOK, `{ - "authorization_endpoint": "https://idp.example.com/authorize", - "token_endpoint": "https://idp.example.com/token", - "userinfo_endpoint": "https://idp.example.com/userinfo" - }`) + server := discoveryTestServer(t, http.StatusOK, func(issuer string) string { + return `{ + "issuer": "` + issuer + `", + "authorization_endpoint": "https://idp.example.com/authorize", + "token_endpoint": "https://idp.example.com/token", + "userinfo_endpoint": "https://idp.example.com/userinfo" + }` + }) cfg := model.OAuthServiceConfig{Issuer: server.URL} @@ -52,12 +58,33 @@ func TestResolveOIDCDiscovery(t *testing.T) { assert.Equal(t, "https://idp.example.com/userinfo", got.UserinfoURL) }) + t.Run("accepts an issuer that differs only by a trailing slash", func(t *testing.T) { + server := discoveryTestServer(t, http.StatusOK, func(issuer string) string { + return `{ + "issuer": "` + issuer + `", + "authorization_endpoint": "https://idp.example.com/authorize", + "token_endpoint": "https://idp.example.com/token", + "userinfo_endpoint": "https://idp.example.com/userinfo" + }` + }) + + cfg := model.OAuthServiceConfig{Issuer: server.URL + "/"} + + got, err := resolveOIDCDiscovery(cfg, context.Background()) + + require.NoError(t, err) + assert.Equal(t, "https://idp.example.com/authorize", got.AuthURL) + }) + t.Run("does not overwrite explicitly configured endpoints", func(t *testing.T) { - server := discoveryTestServer(t, http.StatusOK, `{ - "authorization_endpoint": "https://idp.example.com/authorize", - "token_endpoint": "https://idp.example.com/token", - "userinfo_endpoint": "https://idp.example.com/userinfo" - }`) + server := discoveryTestServer(t, http.StatusOK, func(issuer string) string { + return `{ + "issuer": "` + issuer + `", + "authorization_endpoint": "https://idp.example.com/authorize", + "token_endpoint": "https://idp.example.com/token", + "userinfo_endpoint": "https://idp.example.com/userinfo" + }` + }) cfg := model.OAuthServiceConfig{ Issuer: server.URL, @@ -74,9 +101,32 @@ func TestResolveOIDCDiscovery(t *testing.T) { assert.Equal(t, "https://idp.example.com/userinfo", got.UserinfoURL) }) + t.Run("rejects a document whose issuer does not match", func(t *testing.T) { + server := discoveryTestServer(t, http.StatusOK, func(issuer string) string { + return `{ + "issuer": "https://evil.example.com", + "authorization_endpoint": "https://evil.example.com/authorize", + "token_endpoint": "https://evil.example.com/token", + "userinfo_endpoint": "https://evil.example.com/userinfo" + }` + }) + + cfg := model.OAuthServiceConfig{Issuer: server.URL} + + got, err := resolveOIDCDiscovery(cfg, context.Background()) + + require.Error(t, err) + assert.Contains(t, err.Error(), "issuer mismatch") + assert.Empty(t, got.AuthURL) + assert.Empty(t, got.TokenURL) + assert.Empty(t, got.UserinfoURL) + }) + t.Run("skips discovery when all endpoints are already set", func(t *testing.T) { // The issuer points at a server that always errors; discovery must not be attempted. - server := discoveryTestServer(t, http.StatusInternalServerError, "boom") + server := discoveryTestServer(t, http.StatusInternalServerError, func(issuer string) string { + return "boom" + }) cfg := model.OAuthServiceConfig{ Issuer: server.URL, @@ -92,7 +142,9 @@ func TestResolveOIDCDiscovery(t *testing.T) { }) t.Run("fails soft on a non-200 response", func(t *testing.T) { - server := discoveryTestServer(t, http.StatusNotFound, "not found") + server := discoveryTestServer(t, http.StatusNotFound, func(issuer string) string { + return "not found" + }) cfg := model.OAuthServiceConfig{Issuer: server.URL} @@ -105,7 +157,9 @@ func TestResolveOIDCDiscovery(t *testing.T) { }) t.Run("fails soft on an invalid document", func(t *testing.T) { - server := discoveryTestServer(t, http.StatusOK, "not json") + server := discoveryTestServer(t, http.StatusOK, func(issuer string) string { + return "not json" + }) cfg := model.OAuthServiceConfig{Issuer: server.URL} @@ -119,8 +173,9 @@ func TestResolveOIDCDiscovery(t *testing.T) { // Pad the document past the read cap so the body cannot be fully consumed; the truncated // read must surface as a decode error rather than an unbounded allocation. padding := strings.Repeat(" ", (2<<20)+1) - body := `{"authorization_endpoint": "https://idp.example.com/authorize"` + padding + `}` - server := discoveryTestServer(t, http.StatusOK, body) + server := discoveryTestServer(t, http.StatusOK, func(issuer string) string { + return `{"issuer": "` + issuer + `", "authorization_endpoint": "https://idp.example.com/authorize"` + padding + `}` + }) cfg := model.OAuthServiceConfig{Issuer: server.URL} From 7fc8fd2f0d38bdfae7655c017059719051b0ee36 Mon Sep 17 00:00:00 2001 From: bsaurusrex <82356519+bsaurusrex@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:33:33 +0800 Subject: [PATCH 4/6] fix: reject an OIDC discovery document missing a required endpoint A document that is valid JSON but omits authorization_endpoint, token_endpoint or userinfo_endpoint previously left the field empty and returned no error, so the broker built a provider with an empty endpoint and logged no warning. Validate the effective endpoints (explicit value wins, otherwise discovered) and fail soft when one is still missing, so the misconfiguration is surfaced instead of silently applied. Co-Authored-By: Claude Opus 5.5 --- internal/service/oauth_discovery.go | 41 +++++++++++++++++---- internal/service/oauth_discovery_test.go | 45 ++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 6 deletions(-) diff --git a/internal/service/oauth_discovery.go b/internal/service/oauth_discovery.go index 21505cf29..3906cb96e 100644 --- a/internal/service/oauth_discovery.go +++ b/internal/service/oauth_discovery.go @@ -85,17 +85,46 @@ func resolveOIDCDiscovery(cfg model.OAuthServiceConfig, ctx context.Context) (mo return cfg, fmt.Errorf("OIDC discovery issuer mismatch: document reports %q, expected %q", doc.Issuer, cfg.Issuer) } - if cfg.AuthURL == "" { - cfg.AuthURL = doc.AuthorizationEndpoint + // Determine the effective endpoints: an explicitly configured value always wins, otherwise the + // discovered one is used. The config is only mutated once every required endpoint is present, so a + // document that is valid JSON but omits an endpoint is rejected (and surfaces the fail-soft warning) + // instead of silently building a provider with an empty endpoint. + authURL := cfg.AuthURL + if authURL == "" { + authURL = doc.AuthorizationEndpoint } - if cfg.TokenURL == "" { - cfg.TokenURL = doc.TokenEndpoint + tokenURL := cfg.TokenURL + if tokenURL == "" { + tokenURL = doc.TokenEndpoint } - if cfg.UserinfoURL == "" { - cfg.UserinfoURL = doc.UserinfoEndpoint + userinfoURL := cfg.UserinfoURL + if userinfoURL == "" { + userinfoURL = doc.UserinfoEndpoint } + var missing []string + + if authURL == "" { + missing = append(missing, "authorization_endpoint") + } + + if tokenURL == "" { + missing = append(missing, "token_endpoint") + } + + if userinfoURL == "" { + missing = append(missing, "userinfo_endpoint") + } + + if len(missing) > 0 { + return cfg, fmt.Errorf("OIDC discovery document from %q is missing required endpoint(s): %s", cfg.Issuer, strings.Join(missing, ", ")) + } + + cfg.AuthURL = authURL + cfg.TokenURL = tokenURL + cfg.UserinfoURL = userinfoURL + return cfg, nil } diff --git a/internal/service/oauth_discovery_test.go b/internal/service/oauth_discovery_test.go index 7202c71a2..e598ec9c0 100644 --- a/internal/service/oauth_discovery_test.go +++ b/internal/service/oauth_discovery_test.go @@ -122,6 +122,51 @@ func TestResolveOIDCDiscovery(t *testing.T) { assert.Empty(t, got.UserinfoURL) }) + t.Run("rejects a document that omits a required endpoint", func(t *testing.T) { + // Valid JSON and a matching issuer, but no token_endpoint: this must error (and surface the + // fail-soft warning) rather than silently building a provider with an empty token endpoint. + server := discoveryTestServer(t, http.StatusOK, func(issuer string) string { + return `{ + "issuer": "` + issuer + `", + "authorization_endpoint": "https://idp.example.com/authorize", + "userinfo_endpoint": "https://idp.example.com/userinfo" + }` + }) + + cfg := model.OAuthServiceConfig{Issuer: server.URL} + + got, err := resolveOIDCDiscovery(cfg, context.Background()) + + require.Error(t, err) + assert.Contains(t, err.Error(), "token_endpoint") + assert.Empty(t, got.AuthURL) + assert.Empty(t, got.TokenURL) + assert.Empty(t, got.UserinfoURL) + }) + + t.Run("uses an explicit endpoint to satisfy one the document omits", func(t *testing.T) { + // The document omits token_endpoint, but it is configured explicitly, so discovery still succeeds. + server := discoveryTestServer(t, http.StatusOK, func(issuer string) string { + return `{ + "issuer": "` + issuer + `", + "authorization_endpoint": "https://idp.example.com/authorize", + "userinfo_endpoint": "https://idp.example.com/userinfo" + }` + }) + + cfg := model.OAuthServiceConfig{ + Issuer: server.URL, + TokenURL: "https://custom.example.com/token", + } + + got, err := resolveOIDCDiscovery(cfg, context.Background()) + + require.NoError(t, err) + assert.Equal(t, "https://idp.example.com/authorize", got.AuthURL) + assert.Equal(t, "https://custom.example.com/token", got.TokenURL) + assert.Equal(t, "https://idp.example.com/userinfo", got.UserinfoURL) + }) + t.Run("skips discovery when all endpoints are already set", func(t *testing.T) { // The issuer points at a server that always errors; discovery must not be attempted. server := discoveryTestServer(t, http.StatusInternalServerError, func(issuer string) string { From e6361c567a4388a66a2b993cdeccd4cfbcd308ad Mon Sep 17 00:00:00 2001 From: bsaurusrex <82356519+bsaurusrex@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:37:50 +0800 Subject: [PATCH 5/6] fix: require HTTPS for OIDC discovery unless insecure is set Refuse to fetch the discovery document from a non-HTTPS issuer, and do not follow a redirect that downgrades to a non-HTTPS URL, unless this provider's insecure option is explicitly set. Fetching endpoints over cleartext would let an intermediary swap the token endpoint and capture the client secret during the exchange. OIDC discovery requires secure issuer transport. Co-Authored-By: Claude Opus 5.5 --- internal/model/config.go | 2 +- internal/service/oauth_discovery.go | 29 +++++++++- internal/service/oauth_discovery_test.go | 70 +++++++++++++----------- 3 files changed, 66 insertions(+), 35 deletions(-) diff --git a/internal/model/config.go b/internal/model/config.go index b4c7178fe..c7cc309b9 100644 --- a/internal/model/config.go +++ b/internal/model/config.go @@ -262,7 +262,7 @@ type OAuthServiceConfig struct { WhitelistFile string `description:"Path to the OAuth whitelist file for this provider." yaml:"whitelistFile,omitempty"` Scopes []string `description:"OAuth scopes." yaml:"scopes,omitempty"` RedirectURL string `description:"OAuth redirect URL." yaml:"redirectUrl,omitempty"` - Issuer string `description:"OIDC issuer URL. When set, any OAuth endpoint left empty is filled from its /.well-known/openid-configuration document at startup. The document is fetched with this provider's TLS settings, so enabling 'insecure' also disables certificate verification for discovery, and the issuer in the document must match this value." yaml:"issuer,omitempty"` + Issuer string `description:"OIDC issuer URL. When set, any OAuth endpoint left empty is filled from its /.well-known/openid-configuration document at startup. The issuer must be HTTPS and must match the issuer in the document; a non-HTTPS issuer and certificate verification both require this provider's 'insecure' option." yaml:"issuer,omitempty"` AuthURL string `description:"OAuth authorization URL." yaml:"authUrl,omitempty"` TokenURL string `description:"OAuth token URL." yaml:"tokenUrl,omitempty"` UserinfoURL string `description:"OAuth userinfo URL." yaml:"userinfoUrl,omitempty"` diff --git a/internal/service/oauth_discovery.go b/internal/service/oauth_discovery.go index 3906cb96e..556ae9640 100644 --- a/internal/service/oauth_discovery.go +++ b/internal/service/oauth_discovery.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "net/url" "strings" "time" @@ -40,7 +41,20 @@ func resolveOIDCDiscovery(cfg model.OAuthServiceConfig, ctx context.Context) (mo return cfg, nil } - url := strings.TrimRight(cfg.Issuer, "/") + "/.well-known/openid-configuration" + // OIDC discovery requires secure transport (OpenID Connect Discovery 1.0), otherwise an intermediary + // could swap the discovered endpoints (the token endpoint receives the client secret). A non-HTTPS + // issuer is only allowed when the operator explicitly sets this provider's insecure flag. + issuerURL, err := url.Parse(cfg.Issuer) + + if err != nil { + return cfg, fmt.Errorf("invalid OIDC issuer URL %q: %w", cfg.Issuer, err) + } + + if issuerURL.Scheme != "https" && !cfg.Insecure { + return cfg, fmt.Errorf("refusing to fetch OIDC discovery from non-HTTPS issuer %q, set this provider's insecure option to allow it", cfg.Issuer) + } + + discoveryURL := strings.TrimRight(cfg.Issuer, "/") + "/.well-known/openid-configuration" client := &http.Client{ Timeout: 30 * time.Second, @@ -51,9 +65,20 @@ func resolveOIDCDiscovery(cfg model.OAuthServiceConfig, ctx context.Context) (mo MinVersion: tls.VersionTLS12, }, }, + // Do not let a redirect downgrade the transport to plaintext (or any non-HTTPS scheme) unless + // insecure is set; that would reopen the interception window the HTTPS requirement closes. + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= 10 { + return fmt.Errorf("stopped after 10 redirects") + } + if req.URL.Scheme != "https" && !cfg.Insecure { + return fmt.Errorf("refusing to follow OIDC discovery redirect to non-HTTPS URL %q", req.URL.Redacted()) + } + return nil + }, } - req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, discoveryURL, nil) if err != nil { return cfg, fmt.Errorf("failed to build OIDC discovery request: %w", err) diff --git a/internal/service/oauth_discovery_test.go b/internal/service/oauth_discovery_test.go index e598ec9c0..746523a3d 100644 --- a/internal/service/oauth_discovery_test.go +++ b/internal/service/oauth_discovery_test.go @@ -13,7 +13,8 @@ import ( ) // discoveryTestServer serves a discovery document at the well-known path. docFn receives the server's -// own URL so a document can advertise a matching (or deliberately mismatched) issuer. +// own URL so a document can advertise a matching (or deliberately mismatched) issuer. The server speaks +// plain HTTP, so tests that expect discovery to proceed set the provider's Insecure flag. func discoveryTestServer(t *testing.T, status int, docFn func(issuer string) string) *httptest.Server { t.Helper() var server *httptest.Server @@ -28,6 +29,15 @@ func discoveryTestServer(t *testing.T, status int, docFn func(issuer string) str return server } +func validDoc(issuer string) string { + return `{ + "issuer": "` + issuer + `", + "authorization_endpoint": "https://idp.example.com/authorize", + "token_endpoint": "https://idp.example.com/token", + "userinfo_endpoint": "https://idp.example.com/userinfo" + }` +} + func TestResolveOIDCDiscovery(t *testing.T) { t.Run("no issuer is returned unchanged", func(t *testing.T) { cfg := model.OAuthServiceConfig{ClientID: "abc"} @@ -39,16 +49,9 @@ func TestResolveOIDCDiscovery(t *testing.T) { }) t.Run("fills missing endpoints from the discovery document", func(t *testing.T) { - server := discoveryTestServer(t, http.StatusOK, func(issuer string) string { - return `{ - "issuer": "` + issuer + `", - "authorization_endpoint": "https://idp.example.com/authorize", - "token_endpoint": "https://idp.example.com/token", - "userinfo_endpoint": "https://idp.example.com/userinfo" - }` - }) + server := discoveryTestServer(t, http.StatusOK, validDoc) - cfg := model.OAuthServiceConfig{Issuer: server.URL} + cfg := model.OAuthServiceConfig{Issuer: server.URL, Insecure: true} got, err := resolveOIDCDiscovery(cfg, context.Background()) @@ -59,16 +62,9 @@ func TestResolveOIDCDiscovery(t *testing.T) { }) t.Run("accepts an issuer that differs only by a trailing slash", func(t *testing.T) { - server := discoveryTestServer(t, http.StatusOK, func(issuer string) string { - return `{ - "issuer": "` + issuer + `", - "authorization_endpoint": "https://idp.example.com/authorize", - "token_endpoint": "https://idp.example.com/token", - "userinfo_endpoint": "https://idp.example.com/userinfo" - }` - }) + server := discoveryTestServer(t, http.StatusOK, validDoc) - cfg := model.OAuthServiceConfig{Issuer: server.URL + "/"} + cfg := model.OAuthServiceConfig{Issuer: server.URL + "/", Insecure: true} got, err := resolveOIDCDiscovery(cfg, context.Background()) @@ -77,17 +73,11 @@ func TestResolveOIDCDiscovery(t *testing.T) { }) t.Run("does not overwrite explicitly configured endpoints", func(t *testing.T) { - server := discoveryTestServer(t, http.StatusOK, func(issuer string) string { - return `{ - "issuer": "` + issuer + `", - "authorization_endpoint": "https://idp.example.com/authorize", - "token_endpoint": "https://idp.example.com/token", - "userinfo_endpoint": "https://idp.example.com/userinfo" - }` - }) + server := discoveryTestServer(t, http.StatusOK, validDoc) cfg := model.OAuthServiceConfig{ Issuer: server.URL, + Insecure: true, AuthURL: "https://custom.example.com/auth", TokenURL: "https://custom.example.com/token", // UserinfoURL is left empty, so only it should be filled @@ -101,6 +91,21 @@ func TestResolveOIDCDiscovery(t *testing.T) { assert.Equal(t, "https://idp.example.com/userinfo", got.UserinfoURL) }) + t.Run("rejects a non-HTTPS issuer unless insecure is set", func(t *testing.T) { + // The server would answer, but discovery must refuse the cleartext issuer before fetching. + server := discoveryTestServer(t, http.StatusOK, validDoc) + + cfg := model.OAuthServiceConfig{Issuer: server.URL} // http://, Insecure defaults to false + + got, err := resolveOIDCDiscovery(cfg, context.Background()) + + require.Error(t, err) + assert.Contains(t, err.Error(), "non-HTTPS") + assert.Empty(t, got.AuthURL) + assert.Empty(t, got.TokenURL) + assert.Empty(t, got.UserinfoURL) + }) + t.Run("rejects a document whose issuer does not match", func(t *testing.T) { server := discoveryTestServer(t, http.StatusOK, func(issuer string) string { return `{ @@ -111,7 +116,7 @@ func TestResolveOIDCDiscovery(t *testing.T) { }` }) - cfg := model.OAuthServiceConfig{Issuer: server.URL} + cfg := model.OAuthServiceConfig{Issuer: server.URL, Insecure: true} got, err := resolveOIDCDiscovery(cfg, context.Background()) @@ -133,7 +138,7 @@ func TestResolveOIDCDiscovery(t *testing.T) { }` }) - cfg := model.OAuthServiceConfig{Issuer: server.URL} + cfg := model.OAuthServiceConfig{Issuer: server.URL, Insecure: true} got, err := resolveOIDCDiscovery(cfg, context.Background()) @@ -156,6 +161,7 @@ func TestResolveOIDCDiscovery(t *testing.T) { cfg := model.OAuthServiceConfig{ Issuer: server.URL, + Insecure: true, TokenURL: "https://custom.example.com/token", } @@ -191,7 +197,7 @@ func TestResolveOIDCDiscovery(t *testing.T) { return "not found" }) - cfg := model.OAuthServiceConfig{Issuer: server.URL} + cfg := model.OAuthServiceConfig{Issuer: server.URL, Insecure: true} got, err := resolveOIDCDiscovery(cfg, context.Background()) @@ -206,7 +212,7 @@ func TestResolveOIDCDiscovery(t *testing.T) { return "not json" }) - cfg := model.OAuthServiceConfig{Issuer: server.URL} + cfg := model.OAuthServiceConfig{Issuer: server.URL, Insecure: true} got, err := resolveOIDCDiscovery(cfg, context.Background()) @@ -222,7 +228,7 @@ func TestResolveOIDCDiscovery(t *testing.T) { return `{"issuer": "` + issuer + `", "authorization_endpoint": "https://idp.example.com/authorize"` + padding + `}` }) - cfg := model.OAuthServiceConfig{Issuer: server.URL} + cfg := model.OAuthServiceConfig{Issuer: server.URL, Insecure: true} got, err := resolveOIDCDiscovery(cfg, context.Background()) From a0bcd454992f9b394c9c918c438547ae6f5d05eb Mon Sep 17 00:00:00 2001 From: bsaurusrex <82356519+bsaurusrex@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:47:33 +0800 Subject: [PATCH 6/6] fix: require discovered OIDC endpoints to be HTTPS unless insecure An HTTPS issuer can still return http endpoints in its discovery document. Reject a discovered authorization, token or userinfo endpoint that is not HTTPS unless this provider's insecure option is set, so the client cannot send a user to a cleartext sign-in page or POST its secret to a cleartext token endpoint. Explicitly configured endpoints are left untouched. The document-processing logic is split into applyDiscoveryDocument for testing. Co-Authored-By: Claude Opus 5.5 --- internal/service/oauth_discovery.go | 41 +++++++++++++++++++-- internal/service/oauth_discovery_test.go | 46 ++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 3 deletions(-) diff --git a/internal/service/oauth_discovery.go b/internal/service/oauth_discovery.go index 556ae9640..0a3d77698 100644 --- a/internal/service/oauth_discovery.go +++ b/internal/service/oauth_discovery.go @@ -102,6 +102,14 @@ func resolveOIDCDiscovery(cfg model.OAuthServiceConfig, ctx context.Context) (mo return cfg, fmt.Errorf("failed to decode OIDC discovery document: %w", err) } + return applyDiscoveryDocument(cfg, doc) +} + +// applyDiscoveryDocument validates a fetched discovery document against the configured provider and +// returns the config with any missing endpoint filled in. It enforces the issuer match and the HTTPS +// requirement on discovered endpoints; on any failure it returns the original config unchanged so the +// caller can fail soft. +func applyDiscoveryDocument(cfg model.OAuthServiceConfig, doc oidcDiscoveryDocument) (model.OAuthServiceConfig, error) { // The issuer in the document MUST match the configured issuer (OIDC Discovery 1.0 section 4.3, // RFC 8414 section 3.3). Rejecting a mismatch prevents a substitution/mix-up attack from pointing // the endpoints (the token endpoint receives the client secret) at an unexpected provider. A @@ -111,21 +119,48 @@ func resolveOIDCDiscovery(cfg model.OAuthServiceConfig, ctx context.Context) (mo } // Determine the effective endpoints: an explicitly configured value always wins, otherwise the - // discovered one is used. The config is only mutated once every required endpoint is present, so a - // document that is valid JSON but omits an endpoint is rejected (and surfaces the fail-soft warning) - // instead of silently building a provider with an empty endpoint. + // discovered one is used. A discovered endpoint must be HTTPS unless insecure is set, otherwise a + // (possibly tampered) document could send the user to a cleartext authorization page or make the + // client POST its secret to a cleartext token endpoint. Explicitly configured values are the + // operator's own choice and are left as-is, matching the non-discovery config path. The config is + // only mutated once every required endpoint is present, so a document that is valid JSON but omits + // an endpoint is rejected (and surfaces the fail-soft warning) instead of silently building a + // provider with an empty endpoint. + secure := func(name, raw string) error { + if cfg.Insecure || raw == "" { + return nil + } + parsed, err := url.Parse(raw) + if err != nil { + return fmt.Errorf("invalid %s %q in OIDC discovery document: %w", name, raw, err) + } + if parsed.Scheme != "https" { + return fmt.Errorf("OIDC discovery %s %q is not HTTPS, set this provider's insecure option to allow it", name, raw) + } + return nil + } + authURL := cfg.AuthURL if authURL == "" { + if err := secure("authorization_endpoint", doc.AuthorizationEndpoint); err != nil { + return cfg, err + } authURL = doc.AuthorizationEndpoint } tokenURL := cfg.TokenURL if tokenURL == "" { + if err := secure("token_endpoint", doc.TokenEndpoint); err != nil { + return cfg, err + } tokenURL = doc.TokenEndpoint } userinfoURL := cfg.UserinfoURL if userinfoURL == "" { + if err := secure("userinfo_endpoint", doc.UserinfoEndpoint); err != nil { + return cfg, err + } userinfoURL = doc.UserinfoEndpoint } diff --git a/internal/service/oauth_discovery_test.go b/internal/service/oauth_discovery_test.go index 746523a3d..138a7c9ff 100644 --- a/internal/service/oauth_discovery_test.go +++ b/internal/service/oauth_discovery_test.go @@ -220,6 +220,52 @@ func TestResolveOIDCDiscovery(t *testing.T) { assert.Empty(t, got.AuthURL) }) + t.Run("rejects a cleartext discovered authorization endpoint when not insecure", func(t *testing.T) { + doc := oidcDiscoveryDocument{ + Issuer: "https://idp.example.com", + AuthorizationEndpoint: "http://idp.example.com/authorize", // cleartext + TokenEndpoint: "https://idp.example.com/token", + UserinfoEndpoint: "https://idp.example.com/userinfo", + } + + got, err := applyDiscoveryDocument(model.OAuthServiceConfig{Issuer: "https://idp.example.com"}, doc) + + require.Error(t, err) + assert.Contains(t, err.Error(), "authorization_endpoint") + assert.Contains(t, err.Error(), "not HTTPS") + assert.Empty(t, got.AuthURL) + }) + + t.Run("rejects a cleartext discovered token endpoint when not insecure", func(t *testing.T) { + doc := oidcDiscoveryDocument{ + Issuer: "https://idp.example.com", + AuthorizationEndpoint: "https://idp.example.com/authorize", + TokenEndpoint: "http://idp.example.com/token", // cleartext, would leak the client secret + UserinfoEndpoint: "https://idp.example.com/userinfo", + } + + got, err := applyDiscoveryDocument(model.OAuthServiceConfig{Issuer: "https://idp.example.com"}, doc) + + require.Error(t, err) + assert.Contains(t, err.Error(), "token_endpoint") + assert.Empty(t, got.TokenURL) + }) + + t.Run("allows a cleartext discovered endpoint when insecure is set", func(t *testing.T) { + doc := oidcDiscoveryDocument{ + Issuer: "http://idp.example.com", + AuthorizationEndpoint: "http://idp.example.com/authorize", + TokenEndpoint: "http://idp.example.com/token", + UserinfoEndpoint: "http://idp.example.com/userinfo", + } + + got, err := applyDiscoveryDocument(model.OAuthServiceConfig{Issuer: "http://idp.example.com", Insecure: true}, doc) + + require.NoError(t, err) + assert.Equal(t, "http://idp.example.com/authorize", got.AuthURL) + assert.Equal(t, "http://idp.example.com/token", got.TokenURL) + }) + t.Run("fails soft on an oversized body instead of exhausting memory", func(t *testing.T) { // Pad the document past the read cap so the body cannot be fully consumed; the truncated // read must surface as a decode error rather than an unbounded allocation.