diff --git a/internal/model/config.go b/internal/model/config.go index 9d514390..c7cc309b 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 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_broker_service.go b/internal/service/oauth_broker_service.go index 4df0e825..8f38cb01 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 00000000..0a3d7769 --- /dev/null +++ b/internal/service/oauth_discovery.go @@ -0,0 +1,190 @@ +package service + +import ( + "context" + "crypto/tls" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "time" + + "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 { + Issuer string `json:"issuer"` + 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 + } + + // 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, + Transport: &http.Transport{ + Proxy: http.ProxyFromEnvironment, + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: cfg.Insecure, + 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, discoveryURL, 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(io.LimitReader(resp.Body, maxDiscoveryBodyBytes)).Decode(&doc); err != nil { + 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 + // 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) + } + + // Determine the effective endpoints: an explicitly configured value always wins, otherwise the + // 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 + } + + 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 new file mode 100644 index 00000000..138a7c9f --- /dev/null +++ b/internal/service/oauth_discovery_test.go @@ -0,0 +1,284 @@ +package service + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/tinyauthapp/tinyauth/internal/model" +) + +// 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. 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 + 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(docFn(server.URL))) + }) + server = httptest.NewServer(mux) + t.Cleanup(server.Close) + 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"} + + 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, validDoc) + + cfg := model.OAuthServiceConfig{Issuer: server.URL, Insecure: true} + + 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("accepts an issuer that differs only by a trailing slash", func(t *testing.T) { + server := discoveryTestServer(t, http.StatusOK, validDoc) + + cfg := model.OAuthServiceConfig{Issuer: server.URL + "/", Insecure: true} + + 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, 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 + } + + 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("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 `{ + "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, Insecure: true} + + 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("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, Insecure: true} + + 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, + Insecure: true, + 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 { + return "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, func(issuer string) string { + return "not found" + }) + + cfg := model.OAuthServiceConfig{Issuer: server.URL, Insecure: true} + + 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, func(issuer string) string { + return "not json" + }) + + cfg := model.OAuthServiceConfig{Issuer: server.URL, Insecure: true} + + got, err := resolveOIDCDiscovery(cfg, context.Background()) + + require.Error(t, err) + 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. + padding := strings.Repeat(" ", (2<<20)+1) + 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, Insecure: true} + + got, err := resolveOIDCDiscovery(cfg, context.Background()) + + require.Error(t, err) + assert.Empty(t, got.AuthURL) + }) +}