diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 38f515b..eebe2fd 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -134,12 +134,15 @@ identically: `MarshalText()` all redact; `Reveal()` is the only way to get the raw value out. `client.TokenSet`, `server.TokenResult` and any resource claim that carries a raw token value all use it. -- **`URL`** (constructed via `ParseIssuerURL` / `ParseEndpointURL`, not a - bare `string`) — enforces absolute, HTTPS (except an explicitly - enabled loopback development mode), no fragment, no embedded +- **`URL`** (constructed via `ParseIssuerURL` / `ParseEndpointURL` / + `ParseRedirectURL`, not a bare `string`) — enforces absolute, HTTPS + (except an explicitly enabled loopback exception, or, for a native + app's redirect, a private-use scheme), no fragment, no embedded credentials, normalized host. Registered redirect URIs are compared under OAuth registration semantics, never generic URL equivalence or - automatic normalization — see `fapi.RegisteredRedirectURI`. + automatic normalization — see `fapi.RegisteredRedirectURI` — but for + a native client's loopback redirect URI, which matches on any port + (RFC 8252 §7.3, §8.4). `SignatureAlgorithm` is a closed enum (`ES256`, `PS256`, ...), never a bare string accepted from a caller or read directly out of a JWT header diff --git a/GETTING_STARTED.md b/GETTING_STARTED.md index 27761d0..9368347 100644 --- a/GETTING_STARTED.md +++ b/GETTING_STARTED.md @@ -115,8 +115,11 @@ A mobile or desktop app — a wallet, say — registers with redirect URIs RFC 8252 gives native apps, in production too: a private-use scheme in reverse-domain form (`com.example.wallet:/callback`), or loopback http to `127.0.0.1` or -`[::1]`, which matches on whatever port the app listens on. A web -client can use neither. A native client still authenticates like any +`[::1]`, which matches on whatever port the app listens on. Write the +private-use form with a single slash, as RFC 8252 §7.1 does: +`com.example.wallet:/callback`, not `com.example.wallet://callback`. In +production a web client can use neither; under development assurance +it may use loopback http, matched exactly. A native client still authenticates like any other: give each app instance its own credentials, as attestation-based client authentication does. diff --git a/README.md b/README.md index 6110980..b63ce33 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,7 @@ variants and OAuth 2.0 attestation-based client authentication. - JAR / JARM · RAR (RFC 9396) · CIBA (poll & ping delivery) - Refresh tokens (not rotated, per FAPI 2.0) and whole-grant revocation - OpenID Connect: the `claims` parameter with per-claim consent, `acr_values` and enforced `max_age`, signed and encrypted ID tokens and UserInfo +- Native apps (RFC 8252): private-use URI scheme and any-port loopback redirect URIs, for clients registered as native - Grants you serve yourself at the token endpoint (OpenID4VCI's `pre-authorized_code`, say), with the server's own client authentication and DPoP/mTLS checks - OpenID Federation 1.0 (trust chains, automatic client registration, trust marks) - OpenID Certified™ for OP, RP and FAPI-CIBA OP conformance profiles — see below @@ -276,10 +277,12 @@ role is tested against the OpenID Foundation conformance suite. - [RFC 8705 — Mutual TLS Client Authentication and Certificate-Bound Access Tokens][mtls] - [OpenID Connect Client-Initiated Backchannel Authentication (CIBA) Core 1.0][ciba] - [RFC 9396 — Rich Authorization Requests][rar] +- [RFC 8252 — OAuth 2.0 for Native Apps][native] [fapi2]: https://openid.net/specs/fapi-security-profile-2_0-final.html [fapi2-sign]: https://openid.net/specs/fapi-2_0-message-signing.html [par]: https://www.rfc-editor.org/info/rfc9126 +[native]: https://www.rfc-editor.org/info/rfc8252 [dpop]: https://www.rfc-editor.org/info/rfc9449 [mtls]: https://www.rfc-editor.org/info/rfc8705 [ciba]: https://openid.net/specs/openid-client-initiated-backchannel-authentication-core-1_0.html diff --git a/redirect_url_test.go b/redirect_url_test.go index df8fb22..d2c9508 100644 --- a/redirect_url_test.go +++ b/redirect_url_test.go @@ -1,6 +1,7 @@ package fapi_test import ( + "strings" "testing" fapi "github.com/idfoundry/fapigo" @@ -17,6 +18,8 @@ func TestParseRedirectURL(t *testing.T) { "myapp:/cb": false, // not a reverse domain (RFC 8252 §8.4) "com.example.app:cb": false, // opaque, no path "com.example.app://host/cb": false, // an authority + "com.example.app:///cb": false, // an empty authority + "com.example.app:////evil.example/cb": false, "com..app:/cb": false, "com.-example.app:/cb": false, "com.example.app:/cb#frag": false, @@ -58,3 +61,17 @@ func FuzzParseRedirectURL(f *testing.F) { } }) } + +// TestParseRedirectURLSaysWhatToChange covers the two refusals an +// integrator meets: the common two-slash form, and a private-use URI +// passed without the option. +func TestParseRedirectURLSaysWhatToChange(t *testing.T) { + _, err := fapi.ParseRedirectURL("com.example.app://callback", fapi.AllowPrivateUseScheme()) + if err == nil || !strings.Contains(err.Error(), "com.example.app:/path") { + t.Errorf("ParseRedirectURL(two slashes) = %v, want it to show the single-slash form", err) + } + _, err = fapi.ParseRedirectURL("com.example.app:/callback") + if err == nil || strings.Contains(err.Error(), "must be absolute") { + t.Errorf("ParseRedirectURL(no host, no option) = %v, want it to say the host is missing, not that it isn't absolute", err) + } +} diff --git a/server/assurance.go b/server/assurance.go index 46ffeda..eaf0831 100644 --- a/server/assurance.go +++ b/server/assurance.go @@ -19,9 +19,11 @@ const ( // AssuranceDevelopment permits a configuration meant for local // development only — most importantly, it does not require an // AuditSink or a store that declares storage.StoreAssurance - // capabilities. It also accepts loopback http redirect URIs - // (RFC 8252 §7.3, e.g. "http://localhost:8080/callback"), which - // AssuranceProduction refuses at the pushed authorization request. + // capabilities. It also accepts a web client's loopback http redirect + // URIs (RFC 8252 §7.3, e.g. "http://localhost:8080/callback"), which + // AssuranceProduction refuses at the pushed authorization request. A + // client registered as storage.ApplicationTypeNative may use loopback + // http, and a private-use scheme, under either level. AssuranceDevelopment // AssuranceProduction rejects a configuration missing anything this @@ -66,10 +68,11 @@ const ( // token, jti and DPoP nonce this server issues is only as // unguessable as that reader, and nothing about an io.Reader says // whether it is a CSPRNG. - // Unlike every check above, which New performs once, a client's + // Unlike every check above, which New performs once, a web client's // loopback http redirect URI is refused per request, at the pushed // authorization request, as invalid_request — redirect URIs belong - // to client registrations, which New never sees. + // to client registrations, which New never sees. A native client's + // (storage.ApplicationTypeNative) is accepted, as FAPI 2.0 allows. AssuranceProduction ) diff --git a/server/authorization.go b/server/authorization.go index 12c64ae..c77da7c 100644 --- a/server/authorization.go +++ b/server/authorization.go @@ -207,6 +207,11 @@ func (s *Server) BuildAuthorizationErrorRedirect(ctx context.Context, client sto if !client.HasRedirectURI(redirectURI) { return fapi.URL{}, newError(ErrorInvalidRequest, 400, "redirect_uri is not registered for this client", nil) } + // What the pushed authorization request would have held it to: this + // redirect never went through one. + if _, err := s.parseRedirectURI(client, redirectURI); err != nil { + return fapi.URL{}, newError(ErrorInvalidRequest, 400, "redirect_uri is not an acceptable redirect destination", err) + } dest, buildErr := s.buildAuthorizationResponse(ctx, client.ID(), redirectURI, map[string]string{ "error": errorCode, "state": state, "error_description": description, }) diff --git a/server/complete_authorization.go b/server/complete_authorization.go index 56bcce9..52aa178 100644 --- a/server/complete_authorization.go +++ b/server/complete_authorization.go @@ -232,9 +232,10 @@ func (s *Server) buildAuthorizationResponse(ctx context.Context, clientID fapi.C base.RawQuery = q.Encode() } - // The pushed authorization request already held redirect_uri to the - // client's registered type (parseRedirectURI); this parse only turns - // the stored value, with the response parameters added, into a URL. + // The caller already held redirect_uri to the client's registered type + // (parseRedirectURI): the pushed authorization request, or + // BuildAuthorizationErrorRedirect. This parse only turns the value, + // with the response parameters added, into a URL. destination, err := fapi.ParseRedirectURL(base.String(), fapi.AllowLoopbackHTTP(), fapi.AllowPrivateUseScheme()) if err != nil { return fapi.URL{}, newError(ErrorServerError, 500, "failed to construct redirect destination", err) @@ -259,10 +260,21 @@ func (s *Server) parseRedirectURI(client storage.RegisteredClient, raw string) ( if client.ApplicationType() == storage.ApplicationTypeNative { return fapi.ParseRedirectURL(raw, fapi.AllowLoopbackHTTP(), fapi.AllowPrivateUseScheme()) } + var u fapi.URL + var err error if s.cfg.Assurance == AssuranceProduction { - return fapi.ParseRedirectURL(raw) + u, err = fapi.ParseRedirectURL(raw) + } else { + u, err = fapi.ParseRedirectURL(raw, fapi.AllowLoopbackHTTP()) + } + if err != nil { + if _, nativeErr := fapi.ParseRedirectURL(raw, fapi.AllowLoopbackHTTP(), fapi.AllowPrivateUseScheme()); nativeErr == nil { + // The mistake a native app's integrator makes first: say + // which registration would admit it. + return fapi.URL{}, fmt.Errorf("%w (a native app's redirect URI: acceptable for a client registered with ApplicationType storage.ApplicationTypeNative)", err) + } } - return fapi.ParseRedirectURL(raw, fapi.AllowLoopbackHTTP()) + return u, err } func validateGrantedScopeSubset(granted []string, requestedSpaceDelimited string) error { diff --git a/server/native_redirect_test.go b/server/native_redirect_test.go index a647fa5..ba1732c 100644 --- a/server/native_redirect_test.go +++ b/server/native_redirect_test.go @@ -2,8 +2,11 @@ package server_test import ( "context" + "strings" "testing" + fapi "github.com/idfoundry/fapigo" + "github.com/idfoundry/fapigo/internal/clientassertion" "github.com/idfoundry/fapigo/server" "github.com/idfoundry/fapigo/storage" @@ -119,3 +122,55 @@ func TestNativeClientPrivateUseRedirectWithJARM(t *testing.T) { t.Errorf("response query = %v, want a JARM response", q) } } + +// TestBuildAuthorizationErrorRedirectHoldsTheRedirectPolicy covers the +// error redirect a caller builds without a pushed authorization request: +// it gets the same redirect policy PAR applies. Under production, a web +// client's registered loopback or private-use URI is refused; a native +// client's is accepted. +func TestBuildAuthorizationErrorRedirectHoldsTheRedirectPolicy(t *testing.T) { + for name, tc := range map[string]struct { + uri string + appType storage.ApplicationType + ok bool + }{ + "web, loopback": {"http://127.0.0.1/callback", storage.ApplicationTypeWeb, false}, + "web, localhost": {"http://localhost/callback", storage.ApplicationTypeWeb, false}, + "web, private-use": {testPrivateUseRedirectURI, storage.ApplicationTypeWeb, false}, + "native, private-use": {testPrivateUseRedirectURI, storage.ApplicationTypeNative, true}, + "native, loopback": {"http://127.0.0.1/callback", storage.ApplicationTypeNative, true}, + } { + t.Run(name, func(t *testing.T) { + h := newHarnessWithApplicationType(t, server.ProfileFAPISecurity, true, testRedirectURI, server.AssuranceProduction, storage.ApplicationTypeWeb) + client, err := storage.NewRegisteredClient(storage.RegisteredClientConfig{ + ID: testClientID, RedirectURIs: []fapi.RegisteredRedirectURI{fapi.RegisteredRedirectURI(tc.uri)}, + ClientAssertionAlgorithm: fapi.ES256, AllowedScopes: []string{"openid"}, ApplicationType: tc.appType, + }) + if err != nil { + t.Fatalf("NewRegisteredClient: %v", err) + } + _, err = h.server.BuildAuthorizationErrorRedirect(context.Background(), client, tc.uri, "s", "access_denied", "") + if got := err == nil; got != tc.ok { + t.Errorf("BuildAuthorizationErrorRedirect(%q) = %v, want accepted %v", tc.uri, err, tc.ok) + } + }) + } +} + +// TestPARNamesTheNativeRegistration covers the refusal a native app's +// integrator meets first — a native redirect URI on a client registered +// as web — naming the registration that admits it, for the logs. +func TestPARNamesTheNativeRegistration(t *testing.T) { + h := newHarnessWithApplicationType(t, server.ProfileFAPISecurity, true, testPrivateUseRedirectURI, server.AssuranceProduction, storage.ApplicationTypeWeb) + _, err := pushWithRedirectURI(t, h, testPrivateUseRedirectURI) + if code := serverErrorCode(t, err); code != server.ErrorInvalidRequest { + t.Fatalf("error code = %q, want %q", code, server.ErrorInvalidRequest) + } + if !strings.Contains(err.Error(), "ApplicationTypeNative") { + t.Errorf("cause = %v, want it to name ApplicationTypeNative", err) + } + h = newHarnessWithApplicationType(t, server.ProfileFAPISecurity, true, "http://rp.example/callback", server.AssuranceProduction, storage.ApplicationTypeWeb) + if _, err := pushWithRedirectURI(t, h, "http://rp.example/callback"); err == nil || strings.Contains(err.Error(), "ApplicationTypeNative") { + t.Errorf("cause = %v, want no native hint for a URI no registration admits", err) + } +} diff --git a/storage/client_repository.go b/storage/client_repository.go index 2d0ff7a..cf9a947 100644 --- a/storage/client_repository.go +++ b/storage/client_repository.go @@ -51,7 +51,8 @@ const ( // RFC 8252's native-app redirect URIs, in production as well: // // - a private-use URI scheme (RFC 8252 §7.1), a domain name in - // reverse order: com.example.app:/callback; + // reverse order, then a single slash: com.example.app:/callback, + // not com.example.app://callback; // - loopback http (§7.3) to the IP literal 127.0.0.1 or [::1], never // "localhost" (§8.3), matched on any port at request time; // - a claimed https URI (§7.2), as for a web application. @@ -791,7 +792,8 @@ func (c RegisteredClient) AllowsAuthorizationCodeGrant() bool { return len(c.red // HasRedirectURI reports whether candidate is exactly one of this // client's registered redirect URIs (RegisteredRedirectURI.Equal -// semantics — exact match, no normalization). +// semantics — exact match, no normalization), but for a native client's +// loopback redirect URI, which matches on any port (RFC 8252 §7.3). func (c RegisteredClient) HasRedirectURI(candidate string) bool { for _, u := range c.redirectURIs { if u.Equal(candidate) { @@ -839,8 +841,9 @@ func checkApplicationType(cfg RegisteredClientConfig) error { // isLoopbackLiteral reports whether host is the IP literal 127.0.0.1 or // ::1, the loopback addresses RFC 8252 §7.3 names. func isLoopbackLiteral(host string) bool { - ip := net.ParseIP(host) - return ip != nil && (ip.Equal(net.IPv4(127, 0, 0, 1)) || ip.Equal(net.IPv6loopback)) + // Spelled exactly: net.IP.Equal would also take the IPv4-mapped + // ::ffff:127.0.0.1. + return host == "127.0.0.1" || host == "::1" } // loopbackMatchesAnyPort reports whether candidate is the loopback http diff --git a/storage/enums.go b/storage/enums.go index 0312a1a..5a95177 100644 --- a/storage/enums.go +++ b/storage/enums.go @@ -171,3 +171,38 @@ func ParseBackchannelTokenDeliveryMode(s string) (BackchannelTokenDeliveryMode, return 0, fmt.Errorf("storage: unrecognized backchannel token delivery mode %q", s) } } + +// String returns the canonical wire value for t — OpenID Connect Dynamic +// Client Registration's application_type, "web" or "native" — or "" if t +// is not one of this package's recognized values. +func (t ApplicationType) String() string { + switch t { + case ApplicationTypeWeb: + return "web" + case ApplicationTypeNative: + return "native" + default: + return "" + } +} + +// IsValid reports whether t is one of this package's recognized +// ApplicationType values, including the zero value ApplicationTypeWeb. +func (t ApplicationType) IsValid() bool { + return t == ApplicationTypeWeb || t == ApplicationTypeNative +} + +// ParseApplicationType maps a wire value (see String) to its +// ApplicationType. It rejects every other string, including "": OpenID +// Connect's own default when the field is absent is "web", but that is +// the caller's decision, as for ParseClientAuthMethod. +func ParseApplicationType(s string) (ApplicationType, error) { + switch s { + case "web": + return ApplicationTypeWeb, nil + case "native": + return ApplicationTypeNative, nil + default: + return 0, fmt.Errorf("storage: unrecognized application type %q", s) + } +} diff --git a/storage/native_app_test.go b/storage/native_app_test.go index 79e47c7..95aa397 100644 --- a/storage/native_app_test.go +++ b/storage/native_app_test.go @@ -23,6 +23,7 @@ func TestNativeRedirectURIsAtRegistration(t *testing.T) { "myapp:/callback": false, // RFC 8252 §8.4: no "." "http://localhost/callback": false, // §8.3: not a name "http://127.0.0.2/callback": false, + "http://[::ffff:127.0.0.1]/callback": false, // IPv4-mapped: not the literal "http://wallet.example/callback": false, "com.example.app://host/callback": false, } { @@ -77,3 +78,20 @@ func TestNativeLoopbackMatchesAnyPort(t *testing.T) { t.Error("ApplicationType() doesn't report the registration") } } + +func TestApplicationTypeWireValues(t *testing.T) { + for _, typ := range []ApplicationType{ApplicationTypeWeb, ApplicationTypeNative} { + got, err := ParseApplicationType(typ.String()) + if err != nil || got != typ || !typ.IsValid() { + t.Errorf("ParseApplicationType(%q) = %v, %v; want %v", typ.String(), got, err, typ) + } + } + for _, s := range []string{"", "Native", "mobile"} { + if _, err := ParseApplicationType(s); err == nil { + t.Errorf("ParseApplicationType(%q) = nil error", s) + } + } + if ApplicationType(9).IsValid() || ApplicationType(9).String() != "" { + t.Error("an unknown ApplicationType is valid, or has a wire value") + } +} diff --git a/url.go b/url.go index a68f344..c40bd46 100644 --- a/url.go +++ b/url.go @@ -24,13 +24,17 @@ type urlOptions struct { allowPrivateUseScheme bool } -// URLOption configures ParseIssuerURL or ParseEndpointURL. +// URLOption configures ParseIssuerURL, ParseEndpointURL or +// ParseRedirectURL. AllowPrivateUseScheme applies to ParseRedirectURL +// only; the other two ignore it, and still refuse a URI with no host. type URLOption func(*urlOptions) // AllowLoopbackHTTP permits an http:// scheme when the host is a -// loopback address ("localhost", 127.0.0.0/8, or ::1). It exists for -// local development only and must never be enabled from configuration -// that could reach a production deployment by accident. +// loopback address ("localhost", 127.0.0.0/8, or ::1). For an issuer or +// endpoint it exists for local development only and must never be +// enabled from configuration that could reach a production deployment +// by accident. For a native app's redirect URI (RFC 8252 §7.3) it is +// what FAPI 2.0 allows in production too. func AllowLoopbackHTTP() URLOption { return func(o *urlOptions) { o.allowLoopbackHTTP = true } } @@ -38,8 +42,9 @@ func AllowLoopbackHTTP() URLOption { // AllowPrivateUseScheme permits, for ParseRedirectURL only, a native // app's private-use URI scheme redirect (RFC 8252 §7.1), such as // "com.example.app:/oauth2redirect": a scheme that is a domain name in -// reverse order, so it contains a ".", followed by a path and no -// authority, as RFC 8252 §7.1 writes it. Enable it only for a client +// reverse order, so it contains a ".", then a single slash and the path, +// as RFC 8252 §7.1 writes it — not "com.example.app://oauth2redirect", +// whose "oauth2redirect" would be an authority. Enable it only for a client // registered as a native app. func AllowPrivateUseScheme() URLOption { return func(o *urlOptions) { o.allowPrivateUseScheme = true } @@ -76,8 +81,11 @@ func parsePrivateUseURL(parsed *url.URL) (URL, error) { if !isReverseDomainScheme(parsed.Scheme) { return URL{}, fmt.Errorf("private-use scheme %q must be a reverse-order domain name, such as com.example.app", parsed.Scheme) } - if parsed.Opaque != "" || parsed.Host != "" || parsed.User != nil || !strings.HasPrefix(parsed.Path, "/") { - return URL{}, fmt.Errorf("private-use redirect URI must be scheme:/path, with no authority") + // RFC 8252 §7.1: "only a single slash ("/") appears after the scheme + // component" — com.example.app:/callback, not + // com.example.app://callback, where "callback" would be an authority. + if parsed.Opaque != "" || parsed.Host != "" || parsed.User != nil || !strings.HasPrefix(parsed.Path, "/") || strings.HasPrefix(parsed.Path, "//") || strings.HasPrefix(parsed.String(), parsed.Scheme+"://") { + return URL{}, fmt.Errorf("private-use redirect URI must be %s:/path, with a single slash after the scheme (RFC 8252 §7.1), not %s://…", parsed.Scheme, parsed.Scheme) } if parsed.Fragment != "" { return URL{}, fmt.Errorf("URL must not contain a fragment") @@ -145,9 +153,12 @@ func parseSecureURL(raw string, opts []URLOption) (URL, error) { if err != nil { return URL{}, fmt.Errorf("invalid URL: %w", err) } - if !parsed.IsAbs() || parsed.Host == "" { + if !parsed.IsAbs() { return URL{}, fmt.Errorf("URL must be absolute") } + if parsed.Host == "" { + return URL{}, fmt.Errorf("URL must have a host (scheme %q)", parsed.Scheme) + } if parsed.User != nil { return URL{}, fmt.Errorf("URL must not contain embedded credentials") }