From c796150c149d8dadcbc02d8fd34b344c033bd074 Mon Sep 17 00:00:00 2001 From: Simon Eisenmann Date: Wed, 14 Oct 2026 14:11:01 +0200 Subject: [PATCH 1/2] Add --client-id-metadata-document-host for Client ID Metadata Documents With Client ID Metadata Documents (draft-ietf-oauth-client-id-metadata-document-02) a client uses the https URL of a JSON document it publishes about itself as its client_id, and lico fetches that document instead of keeping a registration. The authorization server then needs neither a pre-registration nor dynamic client registration for such a client, and clients use it when the authorization server announces support. It removes the last thing a user has to enter when dynamic registration is off. The new repeatable --client-id-metadata-document-host enables it. An exact host, with its port if it has one, matches as listed, *.example.com matches subdomains and * any public host. Wildcards never match an address, a single-label name, an explicit port or a name only meaningful inside a local network, such as localhost, local, internal, home.arpa, lan or test. There is no separate switch: listing a host enables the feature, and the metadata then announces client_id_metadata_document_supported, which together with none among the token endpoint auth methods is what lets a public client choose it. identity/clients/cimd.go checks the client_id URL against section 3 of the draft before any network access, being https with a path and without userinfo, query, fragment, dot segments or a trailing dot, and against the host list. It then makes one GET with Accept application/json and a 5 second timeout, follows no redirect (section 5), takes only a 200 with a JSON content type and reads at most 5 KB (section 8.7). The dialer checks the address it actually connects to against the RFC 6890 and IANA special-use blocks (section 8.6), so a listed name resolving inward is refused as well. No proxy is used, because a proxy would dial past that check. The document's client_id must equal the URL, and it may carry neither client_secret nor client_secret_expires_at. token_endpoint_auth_method must be absent or none: no shared secret is possible and private_key_jwt is not implemented, so anything else is refused rather than downgraded. Clients list grant types lico does not implement, such as urn:ietf:params:oauth:grant-type:jwt-bearer, and refusing them would lock such clients out, so grant types are read as a request per RFC 7591 section 3.2.1: unknown ones are dropped and only a document without authorization_code is refused. response_types must include code when present. Every redirect URI which is not loopback must be https on the client_id's host. Loopback redirects alone make a native client, https alone a web client, and a mix is refused. The redirect URI checks of Register are extracted into prepareRedirectURIs and run here too. The resulting registration is public, requires PKCE and is never trusted. Its Name is the host of the client_id, which nobody can fake, and the document's client_name travels beside it as ClaimedName. Details gains unverified, for dynamic and metadata document clients alike, and claimed_name, for the consent screen to use in the next change. Documents are cached as their Cache-Control says, parsed with github.com/pquerna/cachecontrol, which was already in the module graph and is now a direct dependency. The default is 5 minutes, bounded to between 1 minute and 24 hours. Concurrent lookups share one fetch, detached from the caller's context. Errors and invalid documents are never cached (section 5.2). A definite answer, a 4xx or an invalid document, drops the cached entry. A network error, a 5xx or a 429 keeps serving the last good document for up to 24 hours past its expiry, because the token endpoint looks the client up on every refresh. Registry.Lookup and Get try the static registry first, so an operator can still pre-register a URL client_id, then a metadata document for an https client_id when enabled, then dynamic clients. A failed fetch is the answer, never the implicit trust of a redirect to the issuer. NewRegistry takes the host list. The README lists the draft among the standards and describes it in the client registration section, with app.example.com as the example host. The API spec lists the draft in its info section, documents client_id values which are metadata document URLs, adds such clients to those which must use PKCE, and gains the metadata field. cimd_test.go covers the client_id URL rules, the host list including wildcards and local names, and the address check. A web and a native client document, in the shapes public clients publish, are fixtures alongside documents to be refused. The fetcher runs against a TLS test server for caching, a shared concurrent fetch, stale serving while the host fails and its limit, a 404 dropping the entry, uncached errors, a redirect, an oversized and a non-JSON document and an unlisted host never fetched, and the production client is shown to refuse a loopback address. A registry test drives Lookup with loopback redirects of a native client. Verified against a live public metadata document host listed with --client-id-metadata-document-host: the metadata announces support, both of its client IDs pass client lookup at the token endpoint and fail only on the bogus code with invalid_grant, a foreign redirect URI and an unlisted host fail the client lookup, and the host's max-age=300 sets the cache lifetime. --- README.md | 18 + bootstrap/bootstrap.go | 5 + bootstrap/managers.go | 2 +- bootstrap/settings.go | 1 + cmd/licod/serve.go | 1 + config/config.go | 4 + docs/libregraph-connect-api-v1.yaml | 30 +- go.mod | 2 +- .../backends/libregraph/libregraph_test.go | 2 +- identity/clients/cimd.go | 710 ++++++++++++++++++ identity/clients/cimd_test.go | 609 +++++++++++++++ identity/clients/clients.go | 7 + identity/clients/models.go | 7 + identity/clients/registry.go | 70 +- identity/clients/registry_test.go | 4 +- oidc/provider/provider.go | 3 + oidc/provider/provider_test.go | 2 +- oidc/wellknown.go | 2 +- 18 files changed, 1460 insertions(+), 19 deletions(-) create mode 100644 identity/clients/cimd.go create mode 100644 identity/clients/cimd_test.go diff --git a/README.md b/README.md index 85bb2c60..dc4f3b84 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,9 @@ the limits worth knowing before you build on it. - [RFC 9207 - Authorization Server Issuer Identification](https://datatracker.ietf.org/doc/html/rfc9207) - every authorization response redirected to the client, errors included, carries `iss` +- [OAuth Client ID Metadata Document](https://datatracker.ietf.org/doc/html/draft-ietf-oauth-client-id-metadata-document-02) - + draft `-02`, for the hosts named with `--client-id-metadata-document-host`, + public clients only - [OAuth 2.0 Multiple Response Type Encoding Practices](https://openid.net/specs/oauth-v2-multiple-response-types-1_0.html) ### JOSE and cryptography @@ -158,6 +161,21 @@ to ask the same of every client, and `--dynamic-client-secret-expiration` to expire the credentials of a dynamic registration, which by default do not expire. +A client can also be identified by the `https` URL of a JSON document which +describes it, its Client ID Metadata Document. Lico fetches the document +instead of keeping a registration, for the hosts named with +`--client-id-metadata-document-host`, which can be given several times: +`app.example.com` allows that host as listed, `*.example.com` the subdomains of +`example.com`, and `*` any public host. A wildcard never matches an address, a +name without a dot or a name which only means something inside a local network, +such as `localhost`, `*.local`, `*.internal` or `*.home.arpa`, and no document is +ever fetched from a loopback, private or other special-use address. Such a +client is public and has to use PKCE, its non-loopback redirect URIs have to be +on the host of its `client_id`, and the consent screen names it by that host, +with the name the document claims beside it. A document is cached as its +`Cache-Control` says, between one minute and a day, and is used for up to a day +longer while its host cannot be reached. + ### Protected resources Lico can issue access tokens for an API which is not Lico itself, a diff --git a/bootstrap/bootstrap.go b/bootstrap/bootstrap.go index 6d56bdd0..f8353311 100644 --- a/bootstrap/bootstrap.go +++ b/bootstrap/bootstrap.go @@ -222,6 +222,11 @@ func (bs *bootstrap) initialize(settings *Settings) error { logger.Infoln("PKCE is required for all clients") } + bs.config.Config.ClientIDMetadataDocumentHosts = settings.ClientIDMetadataDocumentHosts + if len(bs.config.Config.ClientIDMetadataDocumentHosts) > 0 { + logger.WithField("hosts", bs.config.Config.ClientIDMetadataDocumentHosts).Infoln("client id metadata documents are enabled") + } + encryptionSecretFn := settings.EncryptionSecretFile if encryptionSecretFn != "" { diff --git a/bootstrap/managers.go b/bootstrap/managers.go index a98063b7..243e3eb5 100644 --- a/bootstrap/managers.go +++ b/bootstrap/managers.go @@ -72,7 +72,7 @@ func newManagers(ctx context.Context, bs *bootstrap) (*managers.Managers, error) mgrs.Set("code", code) // Identifier client registry manager. - clients, err := identityClients.NewRegistry(ctx, bs.config.IssuerIdentifierURI, bs.config.IdentifierRegistrationConf, bs.config.Config.AllowDynamicClientRegistration, time.Duration(bs.config.DyamicClientSecretDurationSeconds)*time.Second, logger) + clients, err := identityClients.NewRegistry(ctx, bs.config.IssuerIdentifierURI, bs.config.IdentifierRegistrationConf, bs.config.Config.AllowDynamicClientRegistration, time.Duration(bs.config.DyamicClientSecretDurationSeconds)*time.Second, bs.config.Config.ClientIDMetadataDocumentHosts, logger) if err != nil { return nil, fmt.Errorf("failed to create client registry: %v", err) } diff --git a/bootstrap/settings.go b/bootstrap/settings.go index 6f434cbd..0537cfc1 100644 --- a/bootstrap/settings.go +++ b/bootstrap/settings.go @@ -38,6 +38,7 @@ type Settings struct { AllowClientSignedLogins bool AllowDynamicClientRegistration bool RequirePKCE bool + ClientIDMetadataDocumentHosts []string EncryptionSecretFile string Listen string IdentifierClientDisabled bool diff --git a/cmd/licod/serve.go b/cmd/licod/serve.go index 2b06149e..5ebb8d12 100644 --- a/cmd/licod/serve.go +++ b/cmd/licod/serve.go @@ -101,6 +101,7 @@ func commandServe() *cobra.Command { serveCmd.Flags().BoolVar(&cfg.AllowClientSignedLogins, "allow-client-signed-logins", false, "Allow sign in of client controlled signed login users") serveCmd.Flags().BoolVar(&cfg.AllowDynamicClientRegistration, "allow-dynamic-client-registration", false, "Allow dynamic OAuth2 client registration") serveCmd.Flags().BoolVar(&cfg.RequirePKCE, "require-pkce", false, "Require PKCE for all clients using the authorization code flow, dynamically registered clients always require it") + serveCmd.Flags().StringArrayVar(&cfg.ClientIDMetadataDocumentHosts, "client-id-metadata-document-host", nil, "Fetch Client ID Metadata Documents from this host (repeatable, \"*\" for all public)") serveCmd.Flags().Uint64Var(&cfg.AccessTokenDurationSeconds, "access-token-expiration", 60*10, "Expiration time of access tokens in seconds since generated") // 10 Minutes. serveCmd.Flags().Uint64Var(&cfg.IDTokenDurationSeconds, "id-token-expiration", 60*60, "Expiration time of id tokens in seconds since generated") // 1 Hour. serveCmd.Flags().Uint64Var(&cfg.RefreshTokenDurationSeconds, "refresh-token-expiration", 60*60*24*365*3, "Expiration time of refresh tokens in seconds since generated") // 3 Years. diff --git a/config/config.go b/config/config.go index e5e75556..43c587ed 100644 --- a/config/config.go +++ b/config/config.go @@ -41,4 +41,8 @@ type Config struct { AllowClientSignedLogins bool AllowDynamicClientRegistration bool RequirePKCE bool + + // ClientIDMetadataDocumentHosts are the hosts whose Client ID Metadata + // Documents are fetched, "*" for any public host. None disables them. + ClientIDMetadataDocumentHosts []string } diff --git a/docs/libregraph-connect-api-v1.yaml b/docs/libregraph-connect-api-v1.yaml index bf9232eb..5a34ddcf 100644 --- a/docs/libregraph-connect-api-v1.yaml +++ b/docs/libregraph-connect-api-v1.yaml @@ -16,6 +16,7 @@ info: - Proof Key for Code Exchange (RFC 7636), `S256` only - Resource Indicators for OAuth 2.0 (RFC 8707) - OAuth 2.0 Authorization Server Issuer Identification (RFC 9207) + - OAuth Client ID Metadata Document (draft `-02`, only for configured hosts) - Dynamic Client Registration (only when explicitly enabled, see below) - SAML2 and OAuth2 integration with external identity providers @@ -196,6 +197,15 @@ paths: - name: client_id in: query required: true + description: | + A client registered in `--identifier-registration-conf` or dynamically, or the + `https` URL of the client's Client ID Metadata Document when its host is named + with `--client-id-metadata-document-host`. Such a document is fetched and + cached by this server and makes a public client which must use PKCE. Its + redirect URIs are either on the document's own host or loopback, never both. + + A `client_id` which does not resolve, including a document which is not + allowed, cannot be fetched or is invalid, is refused and never leads to a code. schema: type: string - name: redirect_uri @@ -267,8 +277,9 @@ paths: in: query description: | PKCE challenge (RFC 7636). Accepted on every code flow and verified at the - token endpoint. Required for dynamically registered clients, for static clients - with `require_pkce`, and for every client when the server runs with + token endpoint. Required for dynamically registered clients, for clients + identified by a Client ID Metadata Document, for static clients with + `require_pkce`, and for every client when the server runs with `--require-pkce`. schema: type: string @@ -1225,12 +1236,19 @@ components: type: array description: | Emitted as `["S256"]`. A code challenge is accepted on every code flow and, when - present, verified at the token endpoint. Dynamically registered clients and static - clients with `require_pkce` must send one, and every client must when the server - runs with `--require-pkce`. `plain` is rejected by the authorization endpoint and - is therefore not advertised. + present, verified at the token endpoint. Dynamically registered clients, clients + identified by a Client ID Metadata Document and static clients with + `require_pkce` must send one, and every client must when the server runs with + `--require-pkce`. `plain` is rejected by the authorization endpoint and is + therefore not advertised. items: type: string + client_id_metadata_document_supported: + type: boolean + description: | + Emitted as `true` when at least one `--client-id-metadata-document-host` is + configured, and omitted otherwise. A client may then use the `https` URL of its + Client ID Metadata Document as `client_id`. authorization_response_iss_parameter_supported: type: boolean description: | diff --git a/go.mod b/go.mod index d820a61e..c4774164 100644 --- a/go.mod +++ b/go.mod @@ -22,6 +22,7 @@ require ( github.com/longsleep/rndm v1.2.0 github.com/mendsley/gojwk v0.0.0-20141217222730-4d5ec6e58103 github.com/orcaman/concurrent-map v1.0.0 + github.com/pquerna/cachecontrol v0.2.0 github.com/prometheus/client_golang v1.23.2 github.com/rs/cors v1.11.1 github.com/russellhaering/goxmldsig v1.6.0 @@ -47,7 +48,6 @@ require ( github.com/mattermost/xml-roundtrip-validator v0.1.0 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pkg/errors v0.9.1 // indirect - github.com/pquerna/cachecontrol v0.2.0 // indirect github.com/prometheus/client_model v0.6.2 // indirect github.com/prometheus/common v0.66.1 // indirect github.com/prometheus/procfs v0.16.1 // indirect diff --git a/identifier/backends/libregraph/libregraph_test.go b/identifier/backends/libregraph/libregraph_test.go index 60e9963a..e39ecc04 100644 --- a/identifier/backends/libregraph/libregraph_test.go +++ b/identifier/backends/libregraph/libregraph_test.go @@ -154,7 +154,7 @@ func newTestBackendWithRegistry(t *testing.T, status int, body string) (*LibreGr logger := logrus.New() logger.SetLevel(logrus.PanicLevel) - registry, err := identityClients.NewRegistry(context.Background(), nil, "", false, 0, logger) + registry, err := identityClients.NewRegistry(context.Background(), nil, "", false, 0, nil, logger) if err != nil { httpServer.Close() t.Fatal(err) diff --git a/identity/clients/cimd.go b/identity/clients/cimd.go new file mode 100644 index 00000000..03fcbd1e --- /dev/null +++ b/identity/clients/cimd.go @@ -0,0 +1,710 @@ +/* + * Copyright 2026 Kopano and its licensors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package clients + +// Client ID Metadata Documents, as specified at +// https://datatracker.ietf.org/doc/html/draft-ietf-oauth-client-id-metadata-document-02. +// A client uses the https URL of a JSON document describing itself as its +// client_id, and the document is fetched instead of a registration being kept. + +import ( + "context" + "crypto/tls" + "encoding/json" + "errors" + "fmt" + "io" + "mime" + "net" + "net/http" + "net/netip" + "net/url" + "slices" + "strings" + "sync" + "syscall" + "time" + "unicode/utf8" + + "github.com/libregraph/oidc-go" + "github.com/pquerna/cachecontrol" + "github.com/sirupsen/logrus" +) + +const ( + // MetadataDocumentHostAny allows the metadata documents of any public host. + MetadataDocumentHostAny = "*" + + // The only scheme a metadata document is ever fetched with. + metadataDocumentScheme = "https" + + // The draft recommends reading no more than 5 KB of a document. + metadataDocumentSizeLimit = 5 * 1024 + + // Short, since a fetch can sit in the middle of an authorization or a + // token request, which the client is waiting on. + metadataDocumentFetchTimeout = 5 * time.Second + + // Lifetime of a document which states no caching of its own, and the bounds + // any stated lifetime is held to. + metadataDocumentDefaultTTL = 5 * time.Minute + metadataDocumentMinTTL = time.Minute + metadataDocumentMaxTTL = 24 * time.Hour + + // How long an expired document keeps being used while its host cannot be + // reached. The token endpoint looks the client up on every refresh, which + // must not depend on the client's host being up at that moment. + metadataDocumentStaleTTL = 24 * time.Hour + + // Longest claimed client name shown to a visitor. + metadataDocumentClaimedNameLimit = 100 +) + +// Domains which only resolve, or only mean something, inside a local network, +// and so never qualify for a wildcard host entry. +var metadataDocumentLocalDomains = []string{ + "arpa", + "corp", + "example", + "home", + "internal", + "intranet", + "invalid", + "lan", + "local", + "localdomain", + "localhost", + "onion", + "private", + "test", +} + +// Special-use address blocks of RFC 6890 and the IANA special-purpose address +// registries, which a metadata document fetch must never reach. +var metadataDocumentSpecialUsePrefixes = mustParsePrefixes( + "0.0.0.0/8", + "10.0.0.0/8", + "100.64.0.0/10", + "127.0.0.0/8", + "169.254.0.0/16", + "172.16.0.0/12", + "192.0.0.0/24", + "192.0.2.0/24", + "192.31.196.0/24", + "192.52.193.0/24", + "192.88.99.0/24", + "192.168.0.0/16", + "192.175.48.0/24", + "198.18.0.0/15", + "198.51.100.0/24", + "203.0.113.0/24", + "224.0.0.0/4", + "240.0.0.0/4", + "::/128", + "::1/128", + "::ffff:0:0/96", + "64:ff9b::/96", + "64:ff9b:1::/48", + "100::/64", + "2001::/23", + "2001:db8::/32", + "2002::/16", + "3fff::/20", + "5f00::/16", + "fc00::/7", + "fe80::/10", + "ff00::/8", +) + +func mustParsePrefixes(prefixes ...string) []netip.Prefix { + parsed := make([]netip.Prefix, 0, len(prefixes)) + for _, prefix := range prefixes { + parsed = append(parsed, netip.MustParsePrefix(prefix)) + } + return parsed +} + +// errMetadataDocumentUnavailable marks a fetch which failed for a reason that +// may go away by itself, as opposed to an answer which says the document is +// gone or invalid. +var errMetadataDocumentUnavailable = errors.New("client metadata document unavailable") + +// IsMetadataDocumentClientID returns true if the provided client ID has the +// shape of a Client ID Metadata Document URL. Only https is ever fetched. +func IsMetadataDocumentClientID(clientID string) bool { + return strings.HasPrefix(clientID, metadataDocumentScheme+"://") +} + +// metadataDocumentHosts is the operator's list of the hosts whose metadata +// documents may be fetched. +type metadataDocumentHosts struct { + any bool + exact map[string]bool + suffixes []string +} + +func newMetadataDocumentHosts(entries []string) (*metadataDocumentHosts, error) { + hosts := &metadataDocumentHosts{ + exact: make(map[string]bool), + } + for _, entry := range entries { + entry = strings.ToLower(strings.TrimSpace(entry)) + switch { + case entry == "": + continue + case entry == MetadataDocumentHostAny: + hosts.any = true + case strings.HasPrefix(entry, "*."): + suffix := entry[1:] + if !isPublicHostname(suffix[1:]) { + return nil, fmt.Errorf("invalid client metadata document host %q: a wildcard needs a public domain with at least two labels", entry) + } + hosts.suffixes = append(hosts.suffixes, suffix) + default: + if strings.ContainsAny(entry, "/@*?#") { + return nil, fmt.Errorf("invalid client metadata document host %q: expected a host name, optionally with a port", entry) + } + hosts.exact[entry] = true + } + } + + return hosts, nil +} + +// allows returns true if the metadata document at the provided URL may be +// fetched. An exact entry allows its host as listed. A wildcard allows public +// host names on the default port only, never an address, a single label name +// or a name which only means something inside a local network. +func (h *metadataDocumentHosts) allows(u *url.URL) bool { + if h.exact[strings.ToLower(u.Host)] { + return true + } + if !h.any && len(h.suffixes) == 0 { + return false + } + if u.Port() != "" { + return false + } + hostname := strings.ToLower(u.Hostname()) + if !isPublicHostname(hostname) { + return false + } + if h.any { + return true + } + for _, suffix := range h.suffixes { + if strings.HasSuffix(hostname, suffix) { + return true + } + } + + return false +} + +func (h *metadataDocumentHosts) empty() bool { + return !h.any && len(h.exact) == 0 && len(h.suffixes) == 0 +} + +// isPublicHostname returns true for a host name which can name a host on the +// internet: not an address, with at least two labels, and not below a domain +// which is only meaningful inside a local network. +func isPublicHostname(hostname string) bool { + if hostname == "" || strings.HasSuffix(hostname, ".") { + return false + } + if _, err := netip.ParseAddr(strings.Trim(hostname, "[]")); err == nil { + return false + } + if !strings.Contains(hostname, ".") { + return false + } + for _, domain := range metadataDocumentLocalDomains { + if hostname == domain || strings.HasSuffix(hostname, "."+domain) { + return false + } + } + + return true +} + +// checkMetadataDocumentAddress refuses a connection to a special-use address. +// It runs on the address actually dialed, after name resolution, so a listed +// name which resolves inward is refused just the same. +func checkMetadataDocumentAddress(address string) error { + host, _, err := net.SplitHostPort(address) + if err != nil { + return err + } + ip, err := netip.ParseAddr(host) + if err != nil { + return err + } + ip = ip.Unmap() + for _, prefix := range metadataDocumentSpecialUsePrefixes { + if prefix.Contains(ip) { + return fmt.Errorf("refusing to fetch client metadata document from special-use address %s", ip) + } + } + + return nil +} + +// parseMetadataDocumentClientID returns the URL of the provided client ID if +// it is a valid Client Identifier URL as specified in section 3 of the draft. +func parseMetadataDocumentClientID(clientID string) (*url.URL, error) { + u, err := url.Parse(clientID) + if err != nil { + return nil, fmt.Errorf("client_id is not a URL: %w", err) + } + switch { + case u.Scheme != metadataDocumentScheme: + return nil, errors.New("client_id URL must use https") + case u.Opaque != "" || u.Host == "": + return nil, errors.New("client_id URL has no host") + case u.User != nil: + return nil, errors.New("client_id URL must not contain user info") + case strings.ContainsAny(clientID, "#?"): + return nil, errors.New("client_id URL must not contain a query or a fragment") + case u.Path == "": + return nil, errors.New("client_id URL must contain a path") + case strings.HasSuffix(u.Hostname(), "."): + return nil, errors.New("client_id URL host must not end with a dot") + case hasDotSegment(u.Path): + return nil, errors.New("client_id URL must not contain dot segments") + } + + return u, nil +} + +func hasDotSegment(path string) bool { + for _, segment := range strings.Split(path, "/") { + if segment == "." || segment == ".." { + return true + } + } + return false +} + +// metadataDocument holds the members of a Client ID Metadata Document which +// are used here. Everything else in it is ignored. +type metadataDocument struct { + ClientID string `json:"client_id"` + ClientName string `json:"client_name"` + RedirectURIs []string `json:"redirect_uris"` + GrantTypes []string `json:"grant_types"` + ResponseTypes []string `json:"response_types"` + TokenEndpointAuthMethod string `json:"token_endpoint_auth_method"` +} + +// parseMetadataDocument validates the provided document fetched from the +// provided client ID URL and returns the client registration it describes. +func parseMetadataDocument(clientID string, u *url.URL, body []byte) (*ClientRegistration, error) { + var members map[string]json.RawMessage + if err := json.Unmarshal(body, &members); err != nil { + return nil, fmt.Errorf("client metadata document is not a JSON object: %w", err) + } + doc := &metadataDocument{} + if err := json.Unmarshal(body, doc); err != nil { + return nil, fmt.Errorf("client metadata document is invalid: %w", err) + } + + if doc.ClientID != clientID { + return nil, fmt.Errorf("client metadata document client_id %q does not match its URL", doc.ClientID) + } + + if err := checkMetadataDocumentCredentials(members, doc); err != nil { + return nil, err + } + grantTypes, err := metadataDocumentGrantTypes(doc) + if err != nil { + return nil, err + } + applicationType, err := metadataDocumentApplicationType(u, doc.RedirectURIs) + if err != nil { + return nil, err + } + + registration := &ClientRegistration{ + ID: clientID, + // The name shown for the client is the host vouching for it. The name + // the document gives is self-asserted and only travels beside it. + Name: u.Host, + ClaimedName: truncateRunes(strings.TrimSpace(doc.ClientName), metadataDocumentClaimedNameLimit), + ApplicationType: applicationType, + RedirectURIs: doc.RedirectURIs, + GrantTypes: grantTypes, + RawTokenEndpointAuthMethod: oidc.AuthMethodNone, + RequirePKCE: true, + MetadataDocument: true, + } + if err := prepareRedirectURIs(registration); err != nil { + return nil, fmt.Errorf("client metadata document: %w", err) + } + + return registration, nil +} + +// checkMetadataDocumentCredentials refuses a document which asks for anything +// but a public client. A document is public, so it can hold no shared secret, +// and a key based client authentication is not implemented. Anything but none +// is refused rather than silently downgraded. +func checkMetadataDocumentCredentials(members map[string]json.RawMessage, doc *metadataDocument) error { + for _, member := range []string{"client_secret", "client_secret_expires_at"} { + if _, ok := members[member]; ok { + return fmt.Errorf("client metadata document must not contain %s", member) + } + } + if doc.TokenEndpointAuthMethod != "" && doc.TokenEndpointAuthMethod != oidc.AuthMethodNone { + return fmt.Errorf("client metadata document token_endpoint_auth_method %q is not supported", doc.TokenEndpointAuthMethod) + } + + return nil +} + +// metadataDocumentGrantTypes returns the grant types of the provided document +// which are implemented here. Grant types are a request, not a contract, see +// RFC 7591 section 3.2.1, so the others are dropped, and only a document which +// cannot use the authorization code grant is refused. Their absence means +// authorization_code, as it does for a registration. +func metadataDocumentGrantTypes(doc *metadataDocument) ([]string, error) { + var grantTypes []string + if len(doc.GrantTypes) > 0 { + for _, grantType := range doc.GrantTypes { + switch grantType { + case oidc.GrantTypeAuthorizationCode, oidc.GrantTypeRefreshToken: + grantTypes = append(grantTypes, grantType) + } + } + if !slices.Contains(grantTypes, oidc.GrantTypeAuthorizationCode) { + return nil, errors.New("client metadata document does not use the authorization_code grant") + } + } + if len(doc.ResponseTypes) > 0 && !slices.Contains(doc.ResponseTypes, oidc.ResponseTypeCode) { + return nil, errors.New("client metadata document does not use the code response type") + } + + return grantTypes, nil +} + +// metadataDocumentApplicationType checks the redirect URIs of a document from +// the provided client ID URL and returns the application type they make. Every +// one which is not loopback has to be https on the host of the client ID, so a +// document cannot claim a callback on a host it does not speak for. The +// registry ties a client to a single application type, web returning to https +// only and native to loopback only, so a mix is refused. +func metadataDocumentApplicationType(u *url.URL, redirectURIs []string) (string, error) { + if len(redirectURIs) == 0 { + return "", errors.New("client metadata document has no redirect_uris") + } + + loopback := 0 + for _, redirectURIString := range redirectURIs { + redirectURI, err := url.Parse(redirectURIString) + if err != nil || !isPlainRedirectURI(redirectURI) { + return "", fmt.Errorf("client metadata document redirect_uri %q is invalid", redirectURIString) + } + switch { + case IsLocalNativeHTTPURI(redirectURI): + loopback++ + case redirectURI.Scheme == u.Scheme && strings.EqualFold(redirectURI.Host, u.Host): + default: + return "", fmt.Errorf("client metadata document redirect_uri %q is neither loopback nor on the host of the client_id", redirectURIString) + } + } + + switch loopback { + case 0: + return oidc.ApplicationTypeWeb, nil + case len(redirectURIs): + return oidc.ApplicationTypeNative, nil + default: + return "", errors.New("client metadata document mixes loopback and https redirect_uris") + } +} + +// isPlainRedirectURI returns true for a redirect URI with a host and without +// user info or a fragment. +func isPlainRedirectURI(uri *url.URL) bool { + return uri.Host != "" && uri.User == nil && uri.Fragment == "" +} + +func truncateRunes(value string, limit int) string { + if utf8.RuneCountInString(value) <= limit { + return value + } + runes := []rune(value) + return string(runes[:limit]) +} + +// metadataDocumentEntry is a validated document in the cache. +type metadataDocumentEntry struct { + registration *ClientRegistration + expires time.Time +} + +// metadataDocumentCall is a fetch in flight, shared by everyone asking for the +// same client ID meanwhile. +type metadataDocumentCall struct { + done chan struct{} + registration *ClientRegistration + err error +} + +// metadataDocumentFetcher fetches, validates and caches Client ID Metadata +// Documents from the hosts the operator allowed. +type metadataDocumentFetcher struct { + hosts *metadataDocumentHosts + client *http.Client + logger logrus.FieldLogger + now func() time.Time + + mutex sync.Mutex + entries map[string]*metadataDocumentEntry + calls map[string]*metadataDocumentCall +} + +func newMetadataDocumentFetcher(hostEntries []string, logger logrus.FieldLogger) (*metadataDocumentFetcher, error) { + hosts, err := newMetadataDocumentHosts(hostEntries) + if err != nil { + return nil, err + } + if hosts.empty() { + return nil, nil + } + + return &metadataDocumentFetcher{ + hosts: hosts, + client: newMetadataDocumentClient(checkMetadataDocumentAddress, nil), + logger: logger, + now: time.Now, + + entries: make(map[string]*metadataDocumentEntry), + calls: make(map[string]*metadataDocumentCall), + }, nil +} + +// newMetadataDocumentClient returns the HTTP client documents are fetched +// with. Every address it dials is checked with the provided function first. +func newMetadataDocumentClient(checkAddress func(address string) error, tlsConfig *tls.Config) *http.Client { + dialer := &net.Dialer{ + Timeout: metadataDocumentFetchTimeout, + Control: func(_, address string, _ syscall.RawConn) error { + return checkAddress(address) + }, + } + transport := &http.Transport{ + // No proxy: a proxy dials on our behalf, past the address check. + Proxy: nil, + DialContext: dialer.DialContext, + TLSClientConfig: tlsConfig, + ForceAttemptHTTP2: true, + MaxIdleConns: 10, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: metadataDocumentFetchTimeout, + ResponseHeaderTimeout: metadataDocumentFetchTimeout, + } + + return &http.Client{ + Transport: transport, + Timeout: metadataDocumentFetchTimeout, + // The draft forbids following redirects, and the answer to the + // request itself is then a non-200 status, which is an error. + CheckRedirect: func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + }, + } +} + +// Get returns the client registration described by the metadata document at +// the provided client ID. +func (f *metadataDocumentFetcher) Get(ctx context.Context, clientID string) (*ClientRegistration, error) { + u, err := parseMetadataDocumentClientID(clientID) + if err != nil { + return nil, err + } + if !f.hosts.allows(u) { + return nil, fmt.Errorf("client metadata documents of host %s are not allowed", u.Host) + } + + registration, call := f.cachedOrFetch(clientID, u) + if registration != nil { + return registration, nil + } + + select { + case <-call.done: + case <-ctx.Done(): + return nil, ctx.Err() + } + if call.err == nil { + return call.registration, nil + } + + if errors.Is(call.err, errMetadataDocumentUnavailable) { + if registration := f.stale(clientID); registration != nil { + f.logger.WithError(call.err).WithField("client_id", clientID).Warnln("using expired client metadata document") + return registration, nil + } + } + + return nil, call.err +} + +// cachedOrFetch returns the cached registration of the provided client ID if +// it has not expired, and the fetch to wait for otherwise, starting one unless +// there is one in flight already. +func (f *metadataDocumentFetcher) cachedOrFetch(clientID string, u *url.URL) (*ClientRegistration, *metadataDocumentCall) { + f.mutex.Lock() + defer f.mutex.Unlock() + + if entry, ok := f.entries[clientID]; ok && f.now().Before(entry.expires) { + return entry.registration, nil + } + call, ok := f.calls[clientID] + if !ok { + call = &metadataDocumentCall{ + done: make(chan struct{}), + } + f.calls[clientID] = call + // Detached from the request, so a caller which gives up does not fail + // the fetch for everyone else waiting on it. + go f.fetch(clientID, u, call) + } + + return nil, call +} + +// stale returns the cached registration of the provided client ID if it is +// expired by no more than the stale limit. +func (f *metadataDocumentFetcher) stale(clientID string) *ClientRegistration { + f.mutex.Lock() + defer f.mutex.Unlock() + + if entry, ok := f.entries[clientID]; ok && f.now().Before(entry.expires.Add(metadataDocumentStaleTTL)) { + return entry.registration + } + return nil +} + +func (f *metadataDocumentFetcher) fetch(clientID string, u *url.URL, call *metadataDocumentCall) { + registration, expires, err := f.request(clientID, u) + + f.mutex.Lock() + delete(f.calls, clientID) + switch { + case err == nil: + f.entries[clientID] = &metadataDocumentEntry{ + registration: registration, + expires: expires, + } + case !errors.Is(err, errMetadataDocumentUnavailable): + // The host answered, and the answer is that there is no valid + // document. Errors are never cached, and neither is the last good + // document once its host says it is gone. + delete(f.entries, clientID) + } + call.registration = registration + call.err = err + f.mutex.Unlock() + close(call.done) + + if err != nil { + f.logger.WithError(err).WithField("client_id", clientID).Debugln("client metadata document fetch failed") + } else { + f.logger.WithFields(logrus.Fields{ + "client_id": clientID, + "application_type": registration.ApplicationType, + "redirect_uris": registration.RedirectURIs, + "expires": expires, + }).Debugln("client metadata document fetched") + } +} + +func (f *metadataDocumentFetcher) request(clientID string, u *url.URL) (*ClientRegistration, time.Time, error) { + ctx, cancel := context.WithTimeout(context.Background(), metadataDocumentFetchTimeout) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, clientID, http.NoBody) + if err != nil { + return nil, time.Time{}, err + } + req.Header.Set("Accept", "application/json") + + resp, err := f.client.Do(req) + if err != nil { + return nil, time.Time{}, fmt.Errorf("%w: %w", errMetadataDocumentUnavailable, err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + err = fmt.Errorf("client metadata document answered with status %d", resp.StatusCode) + if resp.StatusCode >= http.StatusInternalServerError || resp.StatusCode == http.StatusTooManyRequests { + err = fmt.Errorf("%w: %w", errMetadataDocumentUnavailable, err) + } + return nil, time.Time{}, err + } + + body, err := readMetadataDocument(resp) + if err != nil { + return nil, time.Time{}, err + } + + registration, err := parseMetadataDocument(clientID, u, body) + if err != nil { + return nil, time.Time{}, err + } + + return registration, f.expires(req, resp), nil +} + +// readMetadataDocument returns the body of the provided response if it is a +// JSON document within the size limit. +func readMetadataDocument(resp *http.Response) ([]byte, error) { + mediaType, _, _ := mime.ParseMediaType(resp.Header.Get("Content-Type")) + if mediaType != "application/json" && !strings.HasSuffix(mediaType, "+json") { + return nil, fmt.Errorf("client metadata document has content type %q, not JSON", mediaType) + } + + body, err := io.ReadAll(io.LimitReader(resp.Body, metadataDocumentSizeLimit+1)) + if err != nil { + return nil, fmt.Errorf("%w: %w", errMetadataDocumentUnavailable, err) + } + if len(body) > metadataDocumentSizeLimit { + return nil, fmt.Errorf("client metadata document is larger than %d bytes", metadataDocumentSizeLimit) + } + + return body, nil +} + +// expires returns until when a document may be used without a refetch, from +// its caching headers and held to the bounds. +func (f *metadataDocumentFetcher) expires(req *http.Request, resp *http.Response) time.Time { + now := f.now() + ttl := metadataDocumentDefaultTTL + if _, expires, err := cachecontrol.CachableResponse(req, resp, cachecontrol.Options{}); err == nil && !expires.IsZero() { + ttl = expires.Sub(now) + } + if ttl < metadataDocumentMinTTL { + ttl = metadataDocumentMinTTL + } + if ttl > metadataDocumentMaxTTL { + ttl = metadataDocumentMaxTTL + } + + return now.Add(ttl) +} diff --git a/identity/clients/cimd_test.go b/identity/clients/cimd_test.go new file mode 100644 index 00000000..f392a3e0 --- /dev/null +++ b/identity/clients/cimd_test.go @@ -0,0 +1,609 @@ +/* + * Copyright 2026 Kopano and its licensors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package clients + +import ( + "context" + "crypto/tls" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/libregraph/oidc-go" + "github.com/sirupsen/logrus" +) + +// Two documents in the shapes public clients publish: a web client returning +// to its own host, asking for a grant type which is not implemented here, and +// a native client returning to loopback on any port. +const ( + webClientID = "https://app.example.com/oauth/client-metadata" + webClientDocument = `{ + "client_id": "https://app.example.com/oauth/client-metadata", + "client_name": "Example App", + "client_uri": "https://app.example.com", + "redirect_uris": [ + "https://app.example.com/oauth/callback" + ], + "grant_types": [ + "authorization_code", + "refresh_token", + "urn:ietf:params:oauth:grant-type:jwt-bearer" + ], + "response_types": [ + "code" + ], + "token_endpoint_auth_method": "none" +}` + + nativeClientID = "https://app.example.com/oauth/cli-client-metadata" + nativeClientDocument = `{ + "client_id": "https://app.example.com/oauth/cli-client-metadata", + "client_name": "Example CLI", + "client_uri": "https://app.example.com", + "redirect_uris": [ + "http://localhost/callback", + "http://127.0.0.1/callback" + ], + "grant_types": [ + "authorization_code", + "refresh_token" + ], + "response_types": [ + "code" + ], + "token_endpoint_auth_method": "none" +}` +) + +// testClaimedName is the client_name of the documents the test server serves. +const testClaimedName = "Test client" + +func TestParseMetadataDocumentClientID(t *testing.T) { + for _, tc := range []struct { + clientID string + valid bool + }{ + {webClientID, true}, + {"https://example.com:8443/client.json", true}, + {"https://example.com/", true}, + {"http://example.com/client.json", false}, + {"https://example.com", false}, + {"https://user@example.com/client.json", false}, + {"https://example.com/client.json#fragment", false}, + {"https://example.com/client.json?query", false}, + {"https://example.com/a/../client.json", false}, + {"https://example.com/./client.json", false}, + {"https://example.com./client.json", false}, + {"https:example.com/client.json", false}, + } { + _, err := parseMetadataDocumentClientID(tc.clientID) + if (err == nil) != tc.valid { + t.Errorf("%s: valid was %v, want %v, error %v", tc.clientID, err == nil, tc.valid, err) + } + } +} + +// metadataDocumentHostCases are host lists with the client IDs they allow +// and refuse. +var metadataDocumentHostCases = []struct { + name string + entries []string + allowed []string + refused []string +}{ + { + name: "exact host", + entries: []string{"app.example.com"}, + allowed: []string{"https://app.example.com/x", "https://APP.example.com/x"}, + refused: []string{"https://app.example.com:8443/x", "https://www.app.example.com/x", "https://example.com/x"}, + }, + { + name: "exact host with port", + entries: []string{"example.com:8443"}, + allowed: []string{"https://example.com:8443/x"}, + refused: []string{"https://example.com/x"}, + }, + { + // The address check refuses them when dialed all the same. + name: "exact entries are matched as listed", + entries: []string{"intranet.corp", "10.0.0.1"}, + allowed: []string{"https://intranet.corp/x", "https://10.0.0.1/x"}, + }, + { + name: "any public host", + entries: []string{MetadataDocumentHostAny}, + allowed: []string{"https://app.example.com/x", "https://app.example.org/x"}, + refused: []string{ + "https://example.org:8443/x", + "https://localhost/x", + "https://printer/x", + "https://10.0.0.1/x", + "https://[::1]/x", + "https://nas.local/x", + "https://wiki.internal/x", + "https://router.home.arpa/x", + "https://box.lan/x", + "https://foo.localhost/x", + "https://example.test/x", + }, + }, + { + name: "subdomains", + entries: []string{"*.example.com"}, + allowed: []string{"https://app.example.com/x", "https://a.b.example.com/x"}, + refused: []string{"https://example.com/x", "https://example.org/x", "https://app.example.com:8443/x"}, + }, +} + +func TestMetadataDocumentHosts(t *testing.T) { + for _, tc := range metadataDocumentHostCases { + t.Run(tc.name, func(t *testing.T) { + hosts, err := newMetadataDocumentHosts(tc.entries) + if err != nil { + t.Fatal(err) + } + for _, raw := range tc.allowed { + u, _ := url.Parse(raw) + if !hosts.allows(u) { + t.Errorf("%s was refused", raw) + } + } + for _, raw := range tc.refused { + u, _ := url.Parse(raw) + if hosts.allows(u) { + t.Errorf("%s was allowed", raw) + } + } + }) + } +} + +func TestMetadataDocumentHostsRefusesEntries(t *testing.T) { + for _, entry := range []string{"*.com", "*.local", "*.internal", "https://app.example.com", "app.example.com/path", "*app.example.com"} { + if _, err := newMetadataDocumentHosts([]string{entry}); err == nil { + t.Errorf("entry %q was accepted", entry) + } + } +} + +func TestCheckMetadataDocumentAddress(t *testing.T) { + for _, tc := range []struct { + address string + allowed bool + }{ + {"160.79.104.10:443", true}, + {"[2607:6bc0::10]:443", true}, + {"127.0.0.1:443", false}, + {"10.1.2.3:443", false}, + {"172.16.0.1:443", false}, + {"192.168.1.1:443", false}, + {"169.254.169.254:80", false}, + {"100.64.0.1:443", false}, + {"0.0.0.0:443", false}, + {"198.18.0.1:443", false}, + {"[::1]:443", false}, + {"[fd00::1]:443", false}, + {"[fe80::1]:443", false}, + {"[::ffff:127.0.0.1]:443", false}, + {"[64:ff9b::a00:1]:443", false}, + } { + err := checkMetadataDocumentAddress(tc.address) + if (err == nil) != tc.allowed { + t.Errorf("%s: allowed was %v, want %v, error %v", tc.address, err == nil, tc.allowed, err) + } + } +} + +func TestParseMetadataDocumentWebClient(t *testing.T) { + u, _ := url.Parse(webClientID) + registration, err := parseMetadataDocument(webClientID, u, []byte(webClientDocument)) + if err != nil { + t.Fatal(err) + } + if registration.ApplicationType != oidc.ApplicationTypeWeb { + t.Errorf("application type was %s, want web", registration.ApplicationType) + } + if registration.Name != "app.example.com" || registration.ClaimedName != "Example App" { + t.Errorf("names were %q and %q, want app.example.com and Example App", registration.Name, registration.ClaimedName) + } + // The jwt-bearer grant is not implemented, so it is dropped rather than a + // reason to refuse the client. + if strings.Join(registration.GrantTypes, " ") != "authorization_code refresh_token" { + t.Errorf("grant types were %v", registration.GrantTypes) + } + if !registration.RequirePKCE || registration.RawTokenEndpointAuthMethod != oidc.AuthMethodNone || !registration.MetadataDocument || registration.Trusted { + t.Errorf("registration is not an untrusted public client requiring PKCE: %+v", registration) + } +} + +func TestParseMetadataDocumentNativeClient(t *testing.T) { + u, _ := url.Parse(nativeClientID) + registration, err := parseMetadataDocument(nativeClientID, u, []byte(nativeClientDocument)) + if err != nil { + t.Fatal(err) + } + if registration.ApplicationType != oidc.ApplicationTypeNative { + t.Errorf("application type was %s, want native", registration.ApplicationType) + } +} + +func TestParseMetadataDocumentRefusals(t *testing.T) { + const clientID = "https://app.example.com/client.json" + document := func(members string) string { + return `{"client_id": "` + clientID + `", "client_name": "App"` + members + `}` + } + for _, tc := range []struct { + name string + document string + }{ + {"client_id mismatch", `{"client_id": "https://app.example.com/other.json", "redirect_uris": ["https://app.example.com/cb"]}`}, + {"no redirect_uris", document(``)}, + {"client_secret", document(`, "redirect_uris": ["https://app.example.com/cb"], "client_secret": "s"`)}, + {"client_secret_expires_at", document(`, "redirect_uris": ["https://app.example.com/cb"], "client_secret_expires_at": 0`)}, + {"client_secret_basic", document(`, "redirect_uris": ["https://app.example.com/cb"], "token_endpoint_auth_method": "client_secret_basic"`)}, + {"private_key_jwt", document(`, "redirect_uris": ["https://app.example.com/cb"], "token_endpoint_auth_method": "private_key_jwt"`)}, + {"foreign origin redirect", document(`, "redirect_uris": ["https://evil.example.org/cb"]`)}, + {"non-loopback http redirect", document(`, "redirect_uris": ["http://app.example.com/cb"]`)}, + {"private-use scheme redirect", document(`, "redirect_uris": ["com.example.app://cb"]`)}, + {"mixed redirects", document(`, "redirect_uris": ["https://app.example.com/cb", "http://localhost/cb"]`)}, + {"no authorization_code grant", document(`, "redirect_uris": ["https://app.example.com/cb"], "grant_types": ["client_credentials"]`)}, + {"no code response type", document(`, "redirect_uris": ["https://app.example.com/cb"], "response_types": ["token"]`)}, + {"not an object", `["https://app.example.com/client.json"]`}, + } { + t.Run(tc.name, func(t *testing.T) { + u, _ := url.Parse(clientID) + if _, err := parseMetadataDocument(clientID, u, []byte(tc.document)); err == nil { + t.Error("document was accepted") + } + }) + } +} + +// metadataDocumentServer serves documents from a TLS test server, with a +// handler the test can swap and a count of the requests it answered. +type metadataDocumentServer struct { + *httptest.Server + + mutex sync.Mutex + handler http.HandlerFunc + hits atomic.Int32 +} + +func newMetadataDocumentServer(t *testing.T) *metadataDocumentServer { + s := &metadataDocumentServer{} + s.Server = httptest.NewTLSServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + s.hits.Add(1) + s.mutex.Lock() + handler := s.handler + s.mutex.Unlock() + handler(rw, req) + })) + t.Cleanup(s.Close) + return s +} + +func (s *metadataDocumentServer) serve(handler http.HandlerFunc) { + s.mutex.Lock() + s.handler = handler + s.mutex.Unlock() +} + +// clientID returns a client ID on the test server, and a document for it. +func (s *metadataDocumentServer) clientID() (clientID, document string) { + clientID = s.URL + "/client.json" + document = fmt.Sprintf(`{"client_id": %q, "client_name": %q, "redirect_uris": ["http://localhost/callback"]}`, clientID, testClaimedName) + return clientID, document +} + +// newTestFetcher returns a fetcher allowed to reach the test server, which +// listens on loopback, with a clock the test controls. +func newTestFetcher(t *testing.T, s *metadataDocumentServer, clock *time.Time) *metadataDocumentFetcher { + t.Helper() + + u, _ := url.Parse(s.URL) + f, err := newMetadataDocumentFetcher([]string{u.Host}, logrus.New()) + if err != nil { + t.Fatal(err) + } + f.client = newMetadataDocumentClient(func(string) error { return nil }, &tls.Config{ + RootCAs: s.Client().Transport.(*http.Transport).TLSClientConfig.RootCAs, + MinVersion: tls.VersionTLS12, + }) + f.now = func() time.Time { return *clock } + return f +} + +func serveDocument(document, cacheControl string) http.HandlerFunc { + return func(rw http.ResponseWriter, _ *http.Request) { + rw.Header().Set("Content-Type", "application/json") + if cacheControl != "" { + rw.Header().Set("Cache-Control", cacheControl) + } + _, _ = rw.Write([]byte(document)) + } +} + +func TestMetadataDocumentFetcherCache(t *testing.T) { + ctx := context.Background() + s := newMetadataDocumentServer(t) + clock := time.Now() + f := newTestFetcher(t, s, &clock) + clientID, document := s.clientID() + s.serve(serveDocument(document, "max-age=300")) + + for range 3 { + registration, err := f.Get(ctx, clientID) + if err != nil { + t.Fatal(err) + } + if registration.ClaimedName != testClaimedName { + t.Errorf("claimed name was %q", registration.ClaimedName) + } + } + if hits := s.hits.Load(); hits != 1 { + t.Errorf("document was fetched %d times, want 1", hits) + } + + clock = clock.Add(301 * time.Second) + if _, err := f.Get(ctx, clientID); err != nil { + t.Fatal(err) + } + if hits := s.hits.Load(); hits != 2 { + t.Errorf("expired document was not fetched again, %d fetches", hits) + } +} + +func TestMetadataDocumentFetcherSharesFetch(t *testing.T) { + ctx := context.Background() + s := newMetadataDocumentServer(t) + clock := time.Now() + f := newTestFetcher(t, s, &clock) + clientID, document := s.clientID() + release := make(chan struct{}) + s.serve(func(rw http.ResponseWriter, req *http.Request) { + <-release + serveDocument(document, "")(rw, req) + }) + + var wg sync.WaitGroup + errs := make(chan error, 5) + for range 5 { + wg.Add(1) + go func() { + defer wg.Done() + _, err := f.Get(ctx, clientID) + errs <- err + }() + } + time.Sleep(50 * time.Millisecond) + close(release) + wg.Wait() + close(errs) + for err := range errs { + if err != nil { + t.Error(err) + } + } + if hits := s.hits.Load(); hits != 1 { + t.Errorf("document was fetched %d times, want 1", hits) + } +} + +func TestMetadataDocumentFetcherServesStale(t *testing.T) { + ctx := context.Background() + s := newMetadataDocumentServer(t) + clock := time.Now() + f := newTestFetcher(t, s, &clock) + clientID, document := s.clientID() + s.serve(serveDocument(document, "max-age=60")) + if _, err := f.Get(ctx, clientID); err != nil { + t.Fatal(err) + } + + s.serve(func(rw http.ResponseWriter, _ *http.Request) { + rw.WriteHeader(http.StatusServiceUnavailable) + }) + clock = clock.Add(2 * time.Minute) + if _, err := f.Get(ctx, clientID); err != nil { + t.Errorf("expired document was not served while its host failed: %v", err) + } + + clock = clock.Add(metadataDocumentStaleTTL) + if _, err := f.Get(ctx, clientID); err == nil { + t.Error("expired document was served beyond the stale limit") + } +} + +func TestMetadataDocumentFetcherDropsGone(t *testing.T) { + ctx := context.Background() + s := newMetadataDocumentServer(t) + clock := time.Now() + f := newTestFetcher(t, s, &clock) + clientID, document := s.clientID() + s.serve(serveDocument(document, "max-age=60")) + if _, err := f.Get(ctx, clientID); err != nil { + t.Fatal(err) + } + + s.serve(http.NotFound) + clock = clock.Add(2 * time.Minute) + if _, err := f.Get(ctx, clientID); err == nil { + t.Error("document was served after its host answered 404") + } + // Errors are not cached either, so the next lookup asks again. + hits := s.hits.Load() + if _, err := f.Get(ctx, clientID); err == nil { + t.Error("document was served after its host answered 404") + } + if s.hits.Load() != hits+1 { + t.Error("a failed fetch was cached") + } +} + +func TestMetadataDocumentFetcherRefusals(t *testing.T) { + ctx := context.Background() + for _, tc := range []struct { + name string + handler func(document string) http.HandlerFunc + }{ + {"a redirect is not followed", func(document string) http.HandlerFunc { + return func(rw http.ResponseWriter, req *http.Request) { + if req.URL.Path == "/moved.json" { + serveDocument(document, "")(rw, req) + return + } + http.Redirect(rw, req, "/moved.json", http.StatusFound) + } + }}, + {"an oversized document is refused", func(document string) http.HandlerFunc { + padded := strings.Replace(document, `"client_name"`, `"padding": "`+strings.Repeat("x", metadataDocumentSizeLimit)+`", "client_name"`, 1) + return serveDocument(padded, "") + }}, + {"a document which is not JSON is refused", func(document string) http.HandlerFunc { + return func(rw http.ResponseWriter, _ *http.Request) { + rw.Header().Set("Content-Type", "text/html") + _, _ = rw.Write([]byte(document)) + } + }}, + } { + t.Run(tc.name, func(t *testing.T) { + s := newMetadataDocumentServer(t) + clock := time.Now() + f := newTestFetcher(t, s, &clock) + clientID, document := s.clientID() + s.serve(tc.handler(document)) + if _, err := f.Get(ctx, clientID); err == nil { + t.Error("document was accepted") + } + }) + } +} + +func TestMetadataDocumentFetcherUnlistedHost(t *testing.T) { + s := newMetadataDocumentServer(t) + clock := time.Now() + f := newTestFetcher(t, s, &clock) + _, document := s.clientID() + s.serve(serveDocument(document, "")) + if _, err := f.Get(context.Background(), nativeClientID); err == nil { + t.Error("document of a host which is not listed was accepted") + } + if hits := s.hits.Load(); hits != 0 { + t.Errorf("a host which is not listed was fetched %d times", hits) + } +} + +func TestMetadataDocumentFetcherRefusesLoopback(t *testing.T) { + // The fetcher as built for production, whose address check refuses the + // loopback address the test server listens on. + s := newMetadataDocumentServer(t) + u, _ := url.Parse(s.URL) + f, err := newMetadataDocumentFetcher([]string{u.Host}, logrus.New()) + if err != nil { + t.Fatal(err) + } + clientID, document := s.clientID() + s.serve(serveDocument(document, "")) + _, err = f.Get(context.Background(), clientID) + if err == nil || !errors.Is(err, errMetadataDocumentUnavailable) { + t.Errorf("fetch from loopback was not refused, got %v", err) + } + if hits := s.hits.Load(); hits != 0 { + t.Errorf("loopback was fetched %d times", hits) + } +} + +// newTestMetadataDocumentRegistry returns a registry which allows the metadata +// documents of a test server serving one, and the client ID of that document. +func newTestMetadataDocumentRegistry(t *testing.T) (registry *Registry, clientID, host string) { + t.Helper() + + s := newMetadataDocumentServer(t) + clientID, document := s.clientID() + s.serve(serveDocument(document, "")) + + u, _ := url.Parse(s.URL) + registry, err := NewRegistry(context.Background(), nil, "", false, 0, []string{u.Host}, logrus.New()) + if err != nil { + t.Fatal(err) + } + clock := time.Now() + registry.metadataDocuments.client = newTestFetcher(t, s, &clock).client + + return registry, clientID, u.Host +} + +// TestRegistryLookupMetadataDocument covers a client ID which is the URL of a +// metadata document, looked up through the registry as every handler does. +func TestRegistryLookupMetadataDocument(t *testing.T) { + ctx := context.Background() + registry, clientID, host := newTestMetadataDocumentRegistry(t) + if !registry.MetadataDocumentsSupported() { + t.Fatal("metadata documents are not supported") + } + + for _, tc := range []struct { + redirectURI string + valid bool + }{ + {"http://localhost:53682/callback", true}, + {"http://127.0.0.1:41000/callback", true}, + {"http://localhost:53682/other", false}, + {"https://localhost/callback", false}, + } { + redirectURI, _ := url.Parse(tc.redirectURI) + details, err := registry.Lookup(ctx, clientID, "", redirectURI, "", false) + if (err == nil) != tc.valid { + t.Errorf("%s: valid was %v, want %v, error %v", tc.redirectURI, err == nil, tc.valid, err) + continue + } + if err == nil && (details.Trusted || !details.Unverified || details.DisplayName != host || details.ClaimedName != testClaimedName) { + t.Errorf("details were not the untrusted, unverified host %s claiming %q: %+v", host, testClaimedName, details) + } + } +} + +func TestRegistryGetMetadataDocument(t *testing.T) { + registry, clientID, _ := newTestMetadataDocumentRegistry(t) + if registration, ok := registry.Get(context.Background(), clientID); !ok || !registration.MetadataDocument { + t.Errorf("Get did not resolve the metadata document, got %v", registration) + } +} + +func TestRegistryLookupMetadataDocumentDisabled(t *testing.T) { + ctx := context.Background() + registry, err := NewRegistry(ctx, nil, "", false, 0, nil, logrus.New()) + if err != nil { + t.Fatal(err) + } + redirectURI, _ := url.Parse("http://localhost:53682/callback") + if _, err := registry.Lookup(ctx, nativeClientID, "", redirectURI, "", false); err == nil { + t.Error("client ID was accepted without metadata document hosts") + } + if registry.MetadataDocumentsSupported() { + t.Error("metadata documents are supported without hosts") + } +} diff --git a/identity/clients/clients.go b/identity/clients/clients.go index fe0b8928..d171bf98 100644 --- a/identity/clients/clients.go +++ b/identity/clients/clients.go @@ -29,6 +29,13 @@ type Details struct { RedirectURI string `json:"redirect_uri"` Trusted bool `json:"trusted"` + // Unverified is true for a client nobody vouches for, one which registered + // itself or which is described by a metadata document at its client ID. + // For the latter DisplayName is the host of that client ID, and + // ClaimedName is the name the document gives. + Unverified bool `json:"unverified"` + ClaimedName string `json:"claimed_name,omitempty"` + Registration *ClientRegistration `json:"-"` } diff --git a/identity/clients/models.go b/identity/clients/models.go index 791a4d6b..d829c422 100644 --- a/identity/clients/models.go +++ b/identity/clients/models.go @@ -68,6 +68,13 @@ type ClientRegistration struct { IDIssuedAt time.Time `yaml:"-" json:"-"` SecretExpiresAt time.Time `yaml:"-" json:"-"` + // MetadataDocument is true for a client described by the Client ID + // Metadata Document at its client ID URL. Its Name is then the host of + // that URL, and ClaimedName the client_name the document gives, which + // nobody vouches for. + MetadataDocument bool `yaml:"-" json:"-"` + ClaimedName string `yaml:"-" json:"-"` + Contacts []string `yaml:"contacts,flow" json:"contacts,omitempty"` Name string `yaml:"name" json:"name,omitempty"` URI string `yaml:"uri" json:"uri,omitempty"` diff --git a/identity/clients/registry.go b/identity/clients/registry.go index b380e598..46599191 100644 --- a/identity/clients/registry.go +++ b/identity/clients/registry.go @@ -43,6 +43,10 @@ type Registry struct { allowDynamicClientRegistration bool dynamicClientSecretDuration time.Duration + // metadataDocuments resolves client IDs which are the URL of a Client ID + // Metadata Document. Nil unless the operator allowed at least one host. + metadataDocuments *metadataDocumentFetcher + StatelessCreator func(ctx context.Context, signingMethod jwt.SigningMethod, claims jwt.Claims) (string, error) StatelessValidator func(token *jwt.Token) (interface{}, error) @@ -59,7 +63,9 @@ type contextKey int var registryKey contextKey // NewRegistry created a new client Registry with the provided parameters. -func NewRegistry(ctx context.Context, trustedURI *url.URL, registrationConfFilepath string, allowDynamicClientRegistration bool, dynamicClientSecretDuration time.Duration, logger logrus.FieldLogger) (*Registry, error) { +// Client IDs which are the URL of a Client ID Metadata Document are accepted +// when they are on one of the provided metadata document hosts. +func NewRegistry(ctx context.Context, trustedURI *url.URL, registrationConfFilepath string, allowDynamicClientRegistration bool, dynamicClientSecretDuration time.Duration, metadataDocumentHosts []string, logger logrus.FieldLogger) (*Registry, error) { registryData := &RegistryData{} if registrationConfFilepath != "" { @@ -75,6 +81,11 @@ func NewRegistry(ctx context.Context, trustedURI *url.URL, registrationConfFilep } } + metadataDocuments, err := newMetadataDocumentFetcher(metadataDocumentHosts, logger) + if err != nil { + return nil, err + } + r := &Registry{ trustedURI: trustedURI, clients: make(map[string]*ClientRegistration), @@ -82,6 +93,8 @@ func NewRegistry(ctx context.Context, trustedURI *url.URL, registrationConfFilep allowDynamicClientRegistration: allowDynamicClientRegistration, dynamicClientSecretDuration: dynamicClientSecretDuration, + metadataDocuments: metadataDocuments, + logger: logger, } @@ -131,6 +144,20 @@ func (r *Registry) Register(client *ClientRegistration) error { return errors.New("invalid client_id") } + if err := prepareRedirectURIs(client); err != nil { + return err + } + + r.mutex.Lock() + defer r.mutex.Unlock() + r.clients[client.ID] = client + return nil +} + +// prepareRedirectURIs checks the redirect URIs of the provided registration +// against its application type, and fills in the application type and the +// origins where they are left to their defaults. +func prepareRedirectURIs(client *ClientRegistration) error { if !client.Insecure && len(client.RedirectURIs) == 0 { return errors.New("no redirect_uris") } @@ -181,9 +208,6 @@ func (r *Registry) Register(client *ClientRegistration) error { return fmt.Errorf("unknown application_type: %v", client.ApplicationType) } - r.mutex.Lock() - defer r.mutex.Unlock() - r.clients[client.ID] = client return nil } @@ -290,6 +314,17 @@ func (r *Registry) Lookup(ctx context.Context, clientID string, clientSecret str registration, _ = r.getDynamicClient(clientID) } + // Lookup clients described by a metadata document at their client ID. A + // document which cannot be had is the answer, never the implicit trust of + // a redirect to ourselves. + if registration == nil && r.metadataDocuments != nil && IsMetadataDocumentClientID(clientID) { + trusted = false + registration, err = r.metadataDocuments.Get(ctx, clientID) + if err != nil { + return nil, fmt.Errorf("unknown client_id: %w", err) + } + } + if registration != nil { redirectURIBase := &url.URL{ Scheme: redirectURI.Scheme, @@ -321,14 +356,28 @@ func (r *Registry) Lookup(ctx context.Context, clientID string, clientSecret str "known": registration != nil, }).Debugln("identifier client lookup") - return &Details{ + details := &Details{ ID: clientID, RedirectURI: redirecURIString, DisplayName: displayName, Trusted: trusted, Registration: registration, - }, nil + } + if registration != nil { + // Nobody vouches for a client which registered itself or which + // describes itself, so the consent screen has to say so. + details.Unverified = registration.Dynamic || registration.MetadataDocument + details.ClaimedName = registration.ClaimedName + } + + return details, nil +} + +// MetadataDocumentsSupported returns true if client IDs which are the URL of a +// Client ID Metadata Document are accepted. +func (r *Registry) MetadataDocumentsSupported() bool { + return r.metadataDocuments != nil } // Get returns the registered clients registration for the provided client ID. @@ -341,6 +390,15 @@ func (r *Registry) Get(ctx context.Context, clientID string) (*ClientRegistratio return registration, true } + if r.metadataDocuments != nil && IsMetadataDocumentClientID(clientID) { + registration, err := r.metadataDocuments.Get(ctx, clientID) + if err != nil { + r.logger.WithError(err).WithField("client_id", clientID).Debugln("client metadata document lookup failed") + return nil, false + } + return registration, true + } + return r.getDynamicClient(clientID) } diff --git a/identity/clients/registry_test.go b/identity/clients/registry_test.go index 89f7f7bd..e3f0e7c5 100644 --- a/identity/clients/registry_test.go +++ b/identity/clients/registry_test.go @@ -24,7 +24,7 @@ func TestRedirectUriWithDynamicPort(t *testing.T) { {"https://localhost:123/callback", true}, } - registry, _ := NewRegistry(context.Background(), nil, "", true, 0, nil) + registry, _ := NewRegistry(context.Background(), nil, "", true, 0, nil, nil) clientRegistration := ClientRegistration{ ID: "native", Secret: "secret", @@ -69,7 +69,7 @@ func TestRedirectUriWithSpecificPath(t *testing.T) { {"http://localhost:8080/other-callback", false}, } - registry, _ := NewRegistry(context.Background(), nil, "", true, 0, nil) + registry, _ := NewRegistry(context.Background(), nil, "", true, 0, nil, nil) clientRegistration := ClientRegistration{ ID: "native", Secret: "secret", diff --git a/oidc/provider/provider.go b/oidc/provider/provider.go index d1445882..b44bce3c 100644 --- a/oidc/provider/provider.go +++ b/oidc/provider/provider.go @@ -488,6 +488,9 @@ func (p *Provider) InitializeMetadata() error { // payload.AuthenticationRequest. Clients reject a response without it // once this is set, see RFC 9207 section 2.4. AuthorizationResponseIssParameterSupported: true, + // Clients pick a Client ID Metadata Document only when this is set + // and none is among the token endpoint auth methods, which it is. + ClientIDMetadataDocumentSupported: p.clients.MetadataDocumentsSupported(), } return nil diff --git a/oidc/provider/provider_test.go b/oidc/provider/provider_test.go index 67b6f42b..078c7ccc 100644 --- a/oidc/provider/provider_test.go +++ b/oidc/provider/provider_test.go @@ -132,7 +132,7 @@ func newTestResourceRegistry(ctx context.Context, t *testing.T) *resources.Regis func newTestClientRegistry(ctx context.Context, t *testing.T) *clients.Registry { trustedURI, _ := url.Parse("http://localhost:8777") // Dynamic registration enabled, with no secret expiry, as it defaults to. - registry, err := clients.NewRegistry(ctx, trustedURI, "", true, 0, logger) + registry, err := clients.NewRegistry(ctx, trustedURI, "", true, 0, nil, logger) if err != nil { t.Fatal(err) } diff --git a/oidc/wellknown.go b/oidc/wellknown.go index dc31d24c..bf01d562 100644 --- a/oidc/wellknown.go +++ b/oidc/wellknown.go @@ -57,6 +57,6 @@ type ProviderMetadata struct { // ClientIDMetadataDocumentSupported announces that a client may use the // HTTPS URL of its own metadata document as client_id, as specified at - // https://datatracker.ietf.org/doc/html/draft-ietf-oauth-client-id-metadata-document-00. + // https://datatracker.ietf.org/doc/html/draft-ietf-oauth-client-id-metadata-document-02. ClientIDMetadataDocumentSupported bool `json:"client_id_metadata_document_supported,omitempty"` } From f6e093da229a6f80e2b7dfe29b29ff67be06e84e Mon Sep 17 00:00:00 2001 From: Simon Eisenmann Date: Wed, 14 Oct 2026 14:11:25 +0200 Subject: [PATCH 2/2] Name the hello resource and unverified client host in Consent The previous change made two kinds of client possible which nobody vouches for, dynamically registered ones and those described by a Client ID Metadata Document, and requests can be bound to a protected resource. The consent screen did not say who was asking or for what, so it now does. The web app passes the authorization request's resource to hello. The identifier looks it up in the resource registry, which it now takes as an optional manager, and answers resource with id and name on a consent answer. The sentence becomes "app.example.com wants to use to:" through the new key konnect.consent.messageWithResource, falling back to the id when the registration has no name. That name comes from the operator's registration, so unlike the client's it is one the server vouches for. An unknown resource never reaches the screen, because the authorization endpoint refuses it with invalid_target first. A client nobody vouches for chooses its own name, so that name alone tells the user nothing. A client marked unverified is therefore named by its display_name, which for a metadata document client is the host of its client_id, the host its metadata document and its redirect URIs live on, as section 8.5 of the draft asks. The document's self-asserted claimed_name is shown beside it as "It identifies itself as Example App.", never instead of it, and not at all when it only repeats the host. For such clients the redirect destination is stated on the page rather than behind the info icon. A local destination, loopback or the private-use scheme of a dynamically registered native client, is stated as an application on this device and comes with a warning to continue only when the user started the sign-in from that application, since any application on the device can listen on localhost. Registered clients keep the previous screen exactly. The consent sentence naming the client is rendered with react-i18next's component, whose translations address the client name element by position. Both variants of it, with and without a resource, are now built in one const ahead of the JSX, so a single biome-ignore keeps the formatter from reflowing them. The resource variant carries explicit defaults, because i18next-parser would otherwise extract the JSX expression literally instead of the {{resource}} interpolation. All six shipped locales are translated, and the catalogs were regenerated with make pot and merge, leaving no fuzzy entries. The API spec documents hello's resource request member, the resource answer member as ResourceDetails, and unverified and claimed_name on ClientDetails, and says that dynamically registered clients are shown as unverified on the consent screen. TestHelloNamesTheResource expects the resource named in canonical form, and none without a resource or with an unknown one; it fails without the change. Vitest cases cover the unverified host and claimed name, the stated destination, the local warning for loopback and private-use schemes, and the resource sentence with and without a name. Every locale renders the resource and unverified notices without unmapped indices or unresolved placeholders. The built web app served by licod was also checked with stubbed hello answers for a web and a native unverified client. --- docs/libregraph-connect-api-v1.yaml | 33 ++++- identifier/api.go | 12 ++ identifier/api_test.go | 135 ++++++++++++++++++ identifier/i18n/de.po | 22 ++- identifier/i18n/es.po | 22 ++- identifier/i18n/fr.po | 22 ++- identifier/i18n/it.po | 22 ++- identifier/i18n/nl.po | 22 ++- identifier/i18n/translation.pot | 28 +++- identifier/i18n/zh-CN.po | 22 ++- identifier/identifier.go | 6 + identifier/models.go | 8 ++ .../src/containers/Login/Consent.test.tsx | 69 +++++++++ identifier/src/containers/Login/Consent.tsx | 106 +++++++++++--- identifier/src/locales/translations.test.tsx | 45 ++++++ identifier/src/models/api.ts | 12 ++ identifier/src/models/hello.ts | 4 + 17 files changed, 556 insertions(+), 34 deletions(-) create mode 100644 identifier/api_test.go diff --git a/docs/libregraph-connect-api-v1.yaml b/docs/libregraph-connect-api-v1.yaml index 5a34ddcf..f258329f 100644 --- a/docs/libregraph-connect-api-v1.yaml +++ b/docs/libregraph-connect-api-v1.yaml @@ -600,7 +600,7 @@ paths: A client registering with `token_endpoint_auth_method: none` is public and gets no secret. Every dynamically registered client must use PKCE, since nobody vouches - for it. + for it, and is shown as unverified on the consent screen. operationId: registerClient requestBody: required: true @@ -1730,6 +1730,11 @@ components: max_age: type: string description: Seconds, as a string. Must parse as an integer if present. + resource: + type: string + description: | + The `resource` parameter of the authorization request (RFC 8707), passed on so + the consent screen can name the protected resource the request is for. HelloResponse: type: object @@ -1773,6 +1778,8 @@ components: $ref: '#/components/schemas/ScopesMap' client: $ref: '#/components/schemas/ClientDetails' + resource: + $ref: '#/components/schemas/ResourceDetails' meta: $ref: '#/components/schemas/Meta' branding: @@ -1892,6 +1899,30 @@ components: format: uri trusted: type: boolean + unverified: + type: boolean + description: | + True for a client nobody vouches for: one which registered itself, or one whose + `client_id` is the URL of its Client ID Metadata Document. For the latter + `display_name` is the host of that URL. + claimed_name: + type: string + description: | + The `client_name` a Client ID Metadata Document gives. Self-asserted, so it is + only shown beside the host. Omitted when there is none. + + ResourceDetails: + type: object + description: | + The protected resource a consent is for, as registered with this server. Present + on a consent answer only when the request named a registered resource. + required: [id] + properties: + id: + type: string + format: uri + name: + type: string Meta: type: object diff --git a/identifier/api.go b/identifier/api.go index 6a849b28..0dad77e0 100644 --- a/identifier/api.go +++ b/identifier/api.go @@ -136,6 +136,18 @@ handleHelloLoop: response.Meta = &meta.Meta{ Scopes: scopes.NewScopesFromIDs(r.Scopes, i.meta.Scopes), } + // The resource is named by the operator's registration, unlike + // the client, so it is what the consent screen can vouch for. An + // unknown one never gets here, the authorization endpoint refuses + // it first. + if r.RawResource != "" && i.resources != nil { + if resource, ok := i.resources.Get(r.RawResource); ok { + response.Resource = &ResourceDetails{ + ID: resource.ID, + Name: resource.Name, + } + } + } } // Add authorize endpoint URI as continue URI. diff --git a/identifier/api_test.go b/identifier/api_test.go new file mode 100644 index 00000000..a4ff5628 --- /dev/null +++ b/identifier/api_test.go @@ -0,0 +1,135 @@ +/* + * Copyright 2026 Kopano and its licensors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package identifier + +import ( + "context" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "testing" + + "github.com/libregraph/oidc-go" + "github.com/sirupsen/logrus" + + "github.com/libregraph/lico/identifier/meta" + "github.com/libregraph/lico/identifier/meta/scopes" + "github.com/libregraph/lico/identity/clients" + "github.com/libregraph/lico/identity/resources" +) + +// TestHelloNamesTheResource covers the consent answer of the hello endpoint, +// which names the protected resource a request is for by its registration, +// since that name, unlike the client's, is one the operator vouches for. +func TestHelloNamesTheResource(t *testing.T) { + i := newTestHelloIdentifier(t) + user := &IdentifiedUser{username: "user1@example.com"} + + hello := func(t *testing.T, resource string) *HelloResponse { + t.Helper() + + r := &HelloRequest{ + Flow: FlowOAuth, + ClientID: "unittestclient", + RawRedirectURI: "http://localhost/callback", + RawScope: "openid", + RawResource: resource, + } + if err := r.parse(); err != nil { + t.Fatal(err) + } + response, err := i.writeHelloResponse(nil, httptest.NewRequest("GET", "/", http.NoBody), r, user) + if err != nil { + t.Fatal(err) + } + if response.Next != FlowConsent { + t.Fatalf("hello did not ask for consent, got %q", response.Next) + } + return response + } + + t.Run("a registered resource is named", func(t *testing.T) { + // Not the canonical form, which the registry resolves. + response := hello(t, "https://MCP.example.com/mcp/") + if response.Resource == nil { + t.Fatal("hello named no resource") + } + if response.Resource.ID != "https://mcp.example.com/mcp" || response.Resource.Name != "Example MCP server" { + t.Errorf("resource was incorrect, got %+v", response.Resource) + } + }) + + t.Run("no resource names none", func(t *testing.T) { + if response := hello(t, ""); response.Resource != nil { + t.Errorf("hello named a resource, got %+v", response.Resource) + } + }) + + t.Run("an unknown resource names none", func(t *testing.T) { + if response := hello(t, "https://other.example.com/mcp"); response.Resource != nil { + t.Errorf("hello named a resource, got %+v", response.Resource) + } + }) +} + +// newTestHelloIdentifier returns an identifier with a registered client and a +// registered resource, enough to answer hello for a consent. +func newTestHelloIdentifier(t *testing.T) *Identifier { + t.Helper() + + ctx := context.Background() + logger := logrus.New() + logger.SetLevel(logrus.PanicLevel) + + clientRegistry, err := clients.NewRegistry(ctx, nil, "", false, 0, nil, logger) + if err != nil { + t.Fatal(err) + } + if err := clientRegistry.Register(&clients.ClientRegistration{ + ID: "unittestclient", + Name: "Unit test client", + ApplicationType: oidc.ApplicationTypeNative, + RedirectURIs: []string{"http://localhost/callback"}, + }); err != nil { + t.Fatal(err) + } + + fn := filepath.Join(t.TempDir(), "identifier-registration.yaml") + conf := "resources:\n" + + " - id: https://mcp.example.com/mcp\n" + + " name: Example MCP server\n" + if err := os.WriteFile(fn, []byte(conf), 0o600); err != nil { + t.Fatal(err) + } + resourceRegistry, err := resources.NewRegistry(ctx, fn, logger) + if err != nil { + t.Fatal(err) + } + + authorizationEndpointURI, _ := url.Parse("https://example.com/signin/v1/identifier/_/authorize") + return &Identifier{ + Config: &Config{}, + clients: clientRegistry, + resources: resourceRegistry, + meta: &meta.Meta{Scopes: &scopes.Scopes{}}, + authorizationEndpointURI: authorizationEndpointURI, + logger: logger, + } +} diff --git a/identifier/i18n/de.po b/identifier/i18n/de.po index 9e8d4486..6655ccfe 100644 --- a/identifier/i18n/de.po +++ b/identifier/i18n/de.po @@ -1,7 +1,7 @@ msgid "" msgstr "" "Project-Id-Version: LibreGraph Connect Identifier\n" -"POT-Creation-Date: 2026-09-04T18:39:04.647Z\n" +"POT-Creation-Date: 2026-10-13T12:33:59.002Z\n" "PO-Revision-Date: 2026-09-04 19:02+0200\n" "Last-Translator: Simon Eisenmann \n" "Language-Team: none\n" @@ -11,6 +11,11 @@ msgstr "" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=2; plural=n != 1;\n" +#. From: konnect##consent##messageWithResource +#: konnect##consent##messageWithResource +msgid "<0><0> wants to use <2>{{resource}} to:" +msgstr "<0><0> möchte über <2>{{resource}}:" + #. From: konnect##consent##message #: konnect##consent##message msgid "<0><0> wants to:" @@ -36,6 +41,11 @@ msgstr "Abbrechen" msgid "Choose an account" msgstr "Konto auswählen" +#. From: konnect##consent##redirectLocal +#: konnect##consent##redirectLocal +msgid "Clicking \"Allow\" will redirect you to an application on this device ({{host}})" +msgstr "Wenn Sie \"Einverstanden\" klicken werden Sie zu einer Anwendung auf diesem Gerät weitergeleitet ({{host}})" + #. From: konnect##consent##redirect #: konnect##consent##redirect msgid "Clicking \"Allow\" will redirect you to {{host}}" @@ -81,6 +91,11 @@ msgstr "Hallo {{displayName}}" msgid "Identity" msgstr "Identität" +#. From: konnect##consent##claimedName +#: konnect##consent##claimedName +msgid "It identifies itself as {{name}}." +msgstr "Die Anwendung gibt an, {{name}} zu sein." + #. From: konnect##error##login##failed #: konnect##error##login##failed msgid "Logon failed. Please verify your credentials and try again." @@ -106,6 +121,11 @@ msgstr "Weiter" msgid "Nothing is shared until you click Allow." msgstr "Es wird nichts weitergegeben, bis Sie auf Einverstanden klicken." +#. From: konnect##consent##localWarning +#: konnect##consent##localWarning +msgid "Only continue if you started this sign-in from that application yourself." +msgstr "Fahren Sie nur fort, wenn Sie diese Anmeldung selbst aus dieser Anwendung gestartet haben." + #. From: konnect##login##passwordField##label #: konnect##login##passwordField##label msgid "Password" diff --git a/identifier/i18n/es.po b/identifier/i18n/es.po index c68a90a3..12d6b2aa 100644 --- a/identifier/i18n/es.po +++ b/identifier/i18n/es.po @@ -1,7 +1,7 @@ msgid "" msgstr "" "Project-Id-Version: LibreGraph Connect Identifier\n" -"POT-Creation-Date: 2026-09-04T18:39:04.647Z\n" +"POT-Creation-Date: 2026-10-13T12:33:59.002Z\n" "PO-Revision-Date: 2026-09-04 19:02+0200\n" "Last-Translator: Simon Eisenmann \n" "Language-Team: none\n" @@ -11,6 +11,11 @@ msgstr "" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=2; plural=(n != 1);\n" +#. From: konnect##consent##messageWithResource +#: konnect##consent##messageWithResource +msgid "<0><0> wants to use <2>{{resource}} to:" +msgstr "<0><0> quiere, a través de <2>{{resource}}:" + #. From: konnect##consent##message #: konnect##consent##message msgid "<0><0> wants to:" @@ -36,6 +41,11 @@ msgstr "Cancelar" msgid "Choose an account" msgstr "Elija una cuenta" +#. From: konnect##consent##redirectLocal +#: konnect##consent##redirectLocal +msgid "Clicking \"Allow\" will redirect you to an application on this device ({{host}})" +msgstr "Al hacer clic en \"Permitir\" se le redirigirá a una aplicación de este dispositivo: {{host}}" + #. From: konnect##consent##redirect #: konnect##consent##redirect msgid "Clicking \"Allow\" will redirect you to {{host}}" @@ -81,6 +91,11 @@ msgstr "Hola {{displayName}}" msgid "Identity" msgstr "Identidad" +#. From: konnect##consent##claimedName +#: konnect##consent##claimedName +msgid "It identifies itself as {{name}}." +msgstr "La aplicación se identifica como {{name}}." + #. From: konnect##error##login##failed #: konnect##error##login##failed msgid "Logon failed. Please verify your credentials and try again." @@ -106,6 +121,11 @@ msgstr "Siguiente" msgid "Nothing is shared until you click Allow." msgstr "No se comparte nada hasta que haga clic en Permitir." +#. From: konnect##consent##localWarning +#: konnect##consent##localWarning +msgid "Only continue if you started this sign-in from that application yourself." +msgstr "Continúe solo si usted mismo inició este inicio de sesión desde esa aplicación." + #. From: konnect##login##passwordField##label #: konnect##login##passwordField##label msgid "Password" diff --git a/identifier/i18n/fr.po b/identifier/i18n/fr.po index c4a4945d..dd16abf3 100644 --- a/identifier/i18n/fr.po +++ b/identifier/i18n/fr.po @@ -1,7 +1,7 @@ msgid "" msgstr "" "Project-Id-Version: LibreGraph Connect Identifier\n" -"POT-Creation-Date: 2026-09-04T18:39:04.647Z\n" +"POT-Creation-Date: 2026-10-13T12:33:59.002Z\n" "PO-Revision-Date: 2026-09-04 19:02+0200\n" "Last-Translator: Simon Eisenmann \n" "Language-Team: none\n" @@ -11,6 +11,11 @@ msgstr "" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=2; plural=n > 1;\n" +#. From: konnect##consent##messageWithResource +#: konnect##consent##messageWithResource +msgid "<0><0> wants to use <2>{{resource}} to:" +msgstr "<0><0> souhaite, via <2>{{resource}} :" + #. From: konnect##consent##message #: konnect##consent##message msgid "<0><0> wants to:" @@ -36,6 +41,11 @@ msgstr "Annuler" msgid "Choose an account" msgstr "Choisir un compte" +#. From: konnect##consent##redirectLocal +#: konnect##consent##redirectLocal +msgid "Clicking \"Allow\" will redirect you to an application on this device ({{host}})" +msgstr "En cliquant \"Autoriser\" vous serez redirigé vers une application de cet appareil : {{host}}" + #. From: konnect##consent##redirect #: konnect##consent##redirect msgid "Clicking \"Allow\" will redirect you to {{host}}" @@ -81,6 +91,11 @@ msgstr "Bonjour {{displayName}}" msgid "Identity" msgstr "Identité" +#. From: konnect##consent##claimedName +#: konnect##consent##claimedName +msgid "It identifies itself as {{name}}." +msgstr "L'application se présente comme {{name}}." + #. From: konnect##error##login##failed #: konnect##error##login##failed msgid "Logon failed. Please verify your credentials and try again." @@ -106,6 +121,11 @@ msgstr "Suivant" msgid "Nothing is shared until you click Allow." msgstr "Rien n'est partagé tant que vous n'avez pas cliqué sur Autoriser." +#. From: konnect##consent##localWarning +#: konnect##consent##localWarning +msgid "Only continue if you started this sign-in from that application yourself." +msgstr "Ne continuez que si vous avez lancé vous-même cette connexion depuis cette application." + #. From: konnect##login##passwordField##label #: konnect##login##passwordField##label msgid "Password" diff --git a/identifier/i18n/it.po b/identifier/i18n/it.po index ea54afa3..4c465c13 100644 --- a/identifier/i18n/it.po +++ b/identifier/i18n/it.po @@ -1,7 +1,7 @@ msgid "" msgstr "" "Project-Id-Version: LibreGraph Connect Identifier\n" -"POT-Creation-Date: 2026-09-04T18:39:04.647Z\n" +"POT-Creation-Date: 2026-10-13T12:33:59.002Z\n" "PO-Revision-Date: 2026-09-04 19:02+0200\n" "Last-Translator: Simon Eisenmann \n" "Language-Team: none\n" @@ -11,6 +11,11 @@ msgstr "" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=2; plural=(n != 1);\n" +#. From: konnect##consent##messageWithResource +#: konnect##consent##messageWithResource +msgid "<0><0> wants to use <2>{{resource}} to:" +msgstr "<0><0> desidera, tramite <2>{{resource}}:" + #. From: konnect##consent##message #: konnect##consent##message msgid "<0><0> wants to:" @@ -36,6 +41,11 @@ msgstr "Annulla" msgid "Choose an account" msgstr "Scegliere un account" +#. From: konnect##consent##redirectLocal +#: konnect##consent##redirectLocal +msgid "Clicking \"Allow\" will redirect you to an application on this device ({{host}})" +msgstr "Facendo clic su \"Acconsento\" verrà reindirizzato a un'applicazione su questo dispositivo: {{host}}" + #. From: konnect##consent##redirect #: konnect##consent##redirect msgid "Clicking \"Allow\" will redirect you to {{host}}" @@ -81,6 +91,11 @@ msgstr "Salve {{displayName}}" msgid "Identity" msgstr "Identità" +#. From: konnect##consent##claimedName +#: konnect##consent##claimedName +msgid "It identifies itself as {{name}}." +msgstr "L'applicazione si presenta come {{name}}." + #. From: konnect##error##login##failed #: konnect##error##login##failed msgid "Logon failed. Please verify your credentials and try again." @@ -106,6 +121,11 @@ msgstr "Avanti" msgid "Nothing is shared until you click Allow." msgstr "Non viene condiviso nulla finché non fa clic su Acconsento." +#. From: konnect##consent##localWarning +#: konnect##consent##localWarning +msgid "Only continue if you started this sign-in from that application yourself." +msgstr "Continui solo se ha avviato lei stesso questo accesso da quell'applicazione." + #. From: konnect##login##passwordField##label #: konnect##login##passwordField##label msgid "Password" diff --git a/identifier/i18n/nl.po b/identifier/i18n/nl.po index b74f7273..9e202094 100644 --- a/identifier/i18n/nl.po +++ b/identifier/i18n/nl.po @@ -1,7 +1,7 @@ msgid "" msgstr "" "Project-Id-Version: LibreGraph Connect Identifier\n" -"POT-Creation-Date: 2026-09-04T18:39:04.647Z\n" +"POT-Creation-Date: 2026-10-13T12:33:59.002Z\n" "PO-Revision-Date: 2026-09-04 19:02+0200\n" "Last-Translator: Simon Eisenmann \n" "Language-Team: none\n" @@ -11,6 +11,11 @@ msgstr "" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=2; plural=n != 1;\n" +#. From: konnect##consent##messageWithResource +#: konnect##consent##messageWithResource +msgid "<0><0> wants to use <2>{{resource}} to:" +msgstr "<0><0> wil via <2>{{resource}}:" + #. From: konnect##consent##message #: konnect##consent##message msgid "<0><0> wants to:" @@ -36,6 +41,11 @@ msgstr "Annuleren" msgid "Choose an account" msgstr "Account kiezen" +#. From: konnect##consent##redirectLocal +#: konnect##consent##redirectLocal +msgid "Clicking \"Allow\" will redirect you to an application on this device ({{host}})" +msgstr "Door op \"Toestaan\" te klikken word je doorverwezen naar een toepassing op dit apparaat: {{host}}" + #. From: konnect##consent##redirect #: konnect##consent##redirect msgid "Clicking \"Allow\" will redirect you to {{host}}" @@ -81,6 +91,11 @@ msgstr "Hoi {{displayName}}" msgid "Identity" msgstr "Identiteit" +#. From: konnect##consent##claimedName +#: konnect##consent##claimedName +msgid "It identifies itself as {{name}}." +msgstr "De toepassing noemt zichzelf {{name}}." + #. From: konnect##error##login##failed #: konnect##error##login##failed msgid "Logon failed. Please verify your credentials and try again." @@ -106,6 +121,11 @@ msgstr "Volgende" msgid "Nothing is shared until you click Allow." msgstr "Er wordt niets gedeeld totdat je op Toestaan klikt." +#. From: konnect##consent##localWarning +#: konnect##consent##localWarning +msgid "Only continue if you started this sign-in from that application yourself." +msgstr "Ga alleen verder als je deze aanmelding zelf vanuit die toepassing hebt gestart." + #. From: konnect##login##passwordField##label #: konnect##login##passwordField##label msgid "Password" diff --git a/identifier/i18n/translation.pot b/identifier/i18n/translation.pot index 552dae52..bda17d08 100644 --- a/identifier/i18n/translation.pot +++ b/identifier/i18n/translation.pot @@ -3,7 +3,7 @@ msgstr "" "Project-Id-Version: LibreGraph Connect Identifier\n" "Content-Type: text/plain; charset=utf-8\n" "Content-Transfer-Encoding: 8bit\n" -"POT-Creation-Date: 2026-09-04T18:39:04.647Z\n" +"POT-Creation-Date: 2026-10-13T12:36:16.761Z\n" "MIME-Version: 1.0\n" #. From: konnect##loading##error##headline @@ -113,9 +113,9 @@ msgstr "" msgid "Use another account" msgstr "" -#. From: konnect##consent##headline -#: konnect##consent##headline -msgid "Hi {{displayName}}" +#. From: konnect##consent##messageWithResource +#: konnect##consent##messageWithResource +msgid "<0><0> wants to use <2>{{resource}} to:" msgstr "" #. From: konnect##consent##message @@ -123,6 +123,16 @@ msgstr "" msgid "<0><0> wants to:" msgstr "" +#. From: konnect##consent##headline +#: konnect##consent##headline +msgid "Hi {{displayName}}" +msgstr "" + +#. From: konnect##consent##claimedName +#: konnect##consent##claimedName +msgid "It identifies itself as {{name}}." +msgstr "" + #. From: konnect##consent##nothingShared #: konnect##consent##nothingShared msgid "Nothing is shared until you click Allow." @@ -133,6 +143,16 @@ msgstr "" msgid "Clicking \"Allow\" will redirect you to {{host}}" msgstr "" +#. From: konnect##consent##redirectLocal +#: konnect##consent##redirectLocal +msgid "Clicking \"Allow\" will redirect you to an application on this device ({{host}})" +msgstr "" + +#. From: konnect##consent##localWarning +#: konnect##consent##localWarning +msgid "Only continue if you started this sign-in from that application yourself." +msgstr "" + #. From: konnect##consent##cancelButton##label #: konnect##consent##cancelButton##label msgid "Cancel" diff --git a/identifier/i18n/zh-CN.po b/identifier/i18n/zh-CN.po index 3fbe71cf..02059576 100644 --- a/identifier/i18n/zh-CN.po +++ b/identifier/i18n/zh-CN.po @@ -1,7 +1,7 @@ msgid "" msgstr "" "Project-Id-Version: LibreGraph Connect Identifier\n" -"POT-Creation-Date: 2026-09-04T18:39:04.647Z\n" +"POT-Creation-Date: 2026-10-13T12:33:59.002Z\n" "PO-Revision-Date: 2026-09-04 19:02+0200\n" "Last-Translator: Simon Eisenmann \n" "Language-Team: none\n" @@ -11,6 +11,11 @@ msgstr "" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=1; plural=0;\n" +#. From: konnect##consent##messageWithResource +#: konnect##consent##messageWithResource +msgid "<0><0> wants to use <2>{{resource}} to:" +msgstr "<0><0> 希望通过 <2>{{resource}} 执行以下操作：" + #. From: konnect##consent##message #: konnect##consent##message msgid "<0><0> wants to:" @@ -36,6 +41,11 @@ msgstr "取消" msgid "Choose an account" msgstr "选择账户" +#. From: konnect##consent##redirectLocal +#: konnect##consent##redirectLocal +msgid "Clicking \"Allow\" will redirect you to an application on this device ({{host}})" +msgstr "点击\"允许\"后，您将被重定向至此设备上的应用程序：{{host}}" + #. From: konnect##consent##redirect #: konnect##consent##redirect msgid "Clicking \"Allow\" will redirect you to {{host}}" @@ -81,6 +91,11 @@ msgstr "您好，{{displayName}}" msgid "Identity" msgstr "身份标识" +#. From: konnect##consent##claimedName +#: konnect##consent##claimedName +msgid "It identifies itself as {{name}}." +msgstr "该应用程序自称为 {{name}}。" + #. From: konnect##error##login##failed #: konnect##error##login##failed msgid "Logon failed. Please verify your credentials and try again." @@ -106,6 +121,11 @@ msgstr "下一步" msgid "Nothing is shared until you click Allow." msgstr "在您点击“允许”之前，不会共享任何内容。" +#. From: konnect##consent##localWarning +#: konnect##consent##localWarning +msgid "Only continue if you started this sign-in from that application yourself." +msgstr "请仅在您亲自从该应用程序发起此次登录时继续。" + #. From: konnect##login##passwordField##label #: konnect##login##passwordField##label msgid "Password" diff --git a/identifier/identifier.go b/identifier/identifier.go index 17faabd1..226dd7ff 100644 --- a/identifier/identifier.go +++ b/identifier/identifier.go @@ -44,6 +44,7 @@ import ( "github.com/libregraph/lico/identity" "github.com/libregraph/lico/identity/authorities" "github.com/libregraph/lico/identity/clients" + "github.com/libregraph/lico/identity/resources" "github.com/libregraph/lico/managers" "github.com/libregraph/lico/utils" ) @@ -82,6 +83,7 @@ type Identifier struct { backend backends.Backend clients *clients.Registry authorities *authorities.Registry + resources *resources.Registry meta *meta.Meta @@ -196,6 +198,10 @@ func NewIdentifier(c *Config) (*Identifier, error) { func (i *Identifier) RegisterManagers(mgrs *managers.Managers) error { i.clients = mgrs.Must("clients").(*clients.Registry) i.authorities = mgrs.Must("authorities").(*authorities.Registry) + // Optional, it only names the resource a consent is for. + if resourceRegistry, _ := mgrs.Get("resources"); resourceRegistry != nil { + i.resources = resourceRegistry.(*resources.Registry) + } if service, ok := i.backend.(managers.ServiceUsesManagers); ok { err := service.RegisterManagers(mgrs) diff --git a/identifier/models.go b/identifier/models.go index 6cfb310f..04c47665 100644 --- a/identifier/models.go +++ b/identifier/models.go @@ -55,6 +55,7 @@ type HelloRequest struct { RawRedirectURI string `json:"redirect_uri"` RawIDTokenHint string `json:"id_token_hint"` RawMaxAge string `json:"max_age"` + RawResource string `json:"resource"` Scopes map[string]bool `json:"-"` Prompts map[string]bool `json:"-"` @@ -105,10 +106,17 @@ type HelloResponse struct { ContinueURI string `json:"continue_uri,omitempty"` Scopes map[string]bool `json:"scopes,omitempty"` ClientDetails *clients.Details `json:"client,omitempty"` + Resource *ResourceDetails `json:"resource,omitempty"` Meta *meta.Meta `json:"meta,omitempty"` Branding *meta.Branding `json:"branding,omitempty"` } +// ResourceDetails names the protected resource a consent is for. +type ResourceDetails struct { + ID string `json:"id"` + Name string `json:"name,omitempty"` +} + // A StateRequest is a general request with a state. type StateRequest struct { State string diff --git a/identifier/src/containers/Login/Consent.test.tsx b/identifier/src/containers/Login/Consent.test.tsx index 613686ab..974e59cf 100644 --- a/identifier/src/containers/Login/Consent.test.tsx +++ b/identifier/src/containers/Login/Consent.test.tsx @@ -213,6 +213,75 @@ describe("Consent", () => { expect(screen.getByText(/Nothing is shared until you click Allow\./)).toBeInTheDocument(); }); + describe("for an unverified client", () => { + // A client whose id is the URL of its own metadata document: the server + // names it by the host of that URL and passes the name it gives beside it. + const unverified: ClientDetails = { + id: "https://app.example.test/oauth/client-metadata", + display_name: "app.example.test", + redirect_uri: "https://app.example.test/oauth/callback", + trusted: false, + unverified: true, + claimed_name: "Example App", + }; + + it("names it by its host and states the name it claims beside it", () => { + renderConsent(helloResponse({ client: unverified })); + + expect(screen.getByText(/wants to/)).toHaveTextContent("app.example.test wants to:"); + expect(screen.getByText("It identifies itself as Example App.")).toBeInTheDocument(); + }); + + it("states no claimed name which only repeats the host", () => { + renderConsent(helloResponse({ client: { ...unverified, claimed_name: "app.example.test" } })); + + expect(screen.queryByText(/identifies itself/)).not.toBeInTheDocument(); + }); + + it("states the destination on the page rather than behind the icon", () => { + renderConsent(helloResponse({ client: unverified })); + + // Nobody vouches for this client, so where Allow leads is the one thing + // a visitor can check, and the specification requires it to be shown. + expect(screen.getByText('Clicking "Allow" will redirect you to app.example.test')).toBeInTheDocument(); + expect(screen.queryByTitle(/redirect you to/)).not.toBeInTheDocument(); + expect(screen.queryByText(/Only continue/)).not.toBeInTheDocument(); + }); + + it.each([ + ["http://localhost:53682/callback", "localhost:53682"], + ["http://127.0.0.1:41000/callback", "127.0.0.1:41000"], + ["vscode://vscode.github-authentication/did-authenticate", "vscode://vscode.github-authentication"], + ])("warns before returning to an application on this device, %s", (redirect_uri, host) => { + renderConsent(helloResponse({ client: { ...unverified, redirect_uri } })); + + // Any application on the device can listen there, so the visitor is told + // and asked to have started the sign-in themselves. + expect( + screen.getByText(`Clicking "Allow" will redirect you to an application on this device (${host})`), + ).toBeInTheDocument(); + expect( + screen.getByText("Only continue if you started this sign-in from that application yourself."), + ).toBeInTheDocument(); + }); + }); + + describe("for a request naming a protected resource", () => { + it("names the resource in the sentence", () => { + renderConsent(helloResponse({ resource: { id: "https://mcp.example.test/mcp", name: "Example MCP server" } })); + + expect(screen.getByText(/wants to use/)).toHaveTextContent("OIDC Playground wants to use Example MCP server to:"); + }); + + it("falls back to the resource id when it has no name", () => { + renderConsent(helloResponse({ resource: { id: "https://mcp.example.test/mcp" } })); + + expect(screen.getByText(/wants to use/)).toHaveTextContent( + "OIDC Playground wants to use https://mcp.example.test/mcp to:", + ); + }); + }); + it("grants the requested scopes when allow is clicked", async () => { renderConsent(); diff --git a/identifier/src/containers/Login/Consent.tsx b/identifier/src/containers/Login/Consent.tsx index 2b80e9f4..a5ad0082 100644 --- a/identifier/src/containers/Login/Consent.tsx +++ b/identifier/src/containers/Login/Consent.tsx @@ -2,6 +2,7 @@ import { useEffect } from "react"; import { Trans, useTranslation } from "react-i18next"; import InfoOutlined from "@mui/icons-material/InfoOutlined"; +import WarningAmberOutlined from "@mui/icons-material/WarningAmberOutlined"; import Box from "@mui/material/Box"; import DialogActions from "@mui/material/DialogActions"; import DialogContent from "@mui/material/DialogContent"; @@ -78,27 +79,68 @@ function Consent() { const meta = hello.details.meta?.scopes ?? { mapping: {}, definitions: {} }; /* - * The domain behind the icon, and only when it is one a visitor could - * recognise. The registry decides which is which and this mirrors it: a web - * client's redirect URI is https and never loopback, while a native client's - * is a custom scheme or http on localhost - * (identity/clients/registry.go:138-177). A client returning to - * `msauth://com.microsoft...` or `localhost:53682` has no destination worth - * naming, so it gets no icon either. + * Where Allow sends the visitor, split by whether it is a domain a visitor + * could recognise. The registry decides which is which and this mirrors it: a + * web client's redirect URI is https and never loopback, while a native + * client's is a custom scheme or http on localhost + * (identity/clients/registry.go, prepareRedirectURIs). + * + * For a registered client the domain is detail, so it sits behind the icon, + * and a client returning to `msauth://com.microsoft...` or `localhost:53682` + * gets no icon at all. For an unverified client, one nobody vouches for, the + * destination is the one thing the visitor can check, so it is stated on the + * page, a local one included, and a local one comes with a warning: the MCP + * authorization specification requires the first and asks for the second + * (security considerations, Client ID Metadata Document Security), since any + * application on the device can listen on localhost. */ + const unverified = !!client.unverified; let redirectHost = ""; + let redirectLocal = ""; if (client.redirect_uri) { try { const uri = new URL(client.redirect_uri); const loopback = ["localhost", "127.0.0.1", "::1", "[::1]"].includes(uri.hostname); if ((uri.protocol === "https:" || uri.protocol === "http:") && !loopback) { redirectHost = uri.host; + } else if (loopback) { + redirectLocal = uri.host; + } else if (uri.host) { + redirectLocal = `${uri.protocol}//${uri.host}`; } } catch { // Not a URI this browser can read, so there is no host to promise. } } + // The name an unverified client gives itself, shown beside the host it is + // named by and never instead of it. Repeating the same name says nothing. + const claimedName = + unverified && client.claimed_name && client.claimed_name !== client.display_name ? client.claimed_name : ""; + + // The resource is named by the operator's registration, so unlike the + // client's it is a name this server vouches for. + const resourceName = hello.details.resource ? hello.details.resource.name || hello.details.resource.id : ""; + + /* + * The translations address these children by position: konnect.consent.message + * is "<0> wants to:". Reflowing the JSX inserts a {" "} child before it, + * which shifts the index and silently wraps the wrong node, so each stays on + * the lines it was written on. The spans carry no styling of their own now + * that the whole line is set in the heading weight; they exist because the + * sentence structure is the translators' contract. + */ + // biome-ignore format: children are index-mapped by the translations + const message = resourceName ? ( + + wants to use {resourceName} to: + + ) : ( + + wants to: + + ); + return ( @@ -115,20 +157,14 @@ function Consent() { * the spacing scale rather than gutterBottom's 0.35em, which landed on 7px * here and read as an accident beside the 16px between every other block. */} - - {/* - * The translations address this child by position: konnect.consent.message - * is "<0> wants to:". Reflowing the JSX inserts a {" "} child before - * it, which shifts the index and silently wraps the wrong node, so this - * stays on the lines it was written on. The span carries no styling of - * its own now that the whole line is set in the heading weight; it - * exists because the sentence structure is the translators' contract. - */} - {/* biome-ignore format: children are index-mapped by the translations */} - - wants to: - + + {message} + {claimedName ? ( + + {t("konnect.consent.claimedName", "It identifies itself as {{name}}.", { name: claimedName })} + + ) : null} {/* @@ -141,8 +177,9 @@ function Consent() { * The whole redirect URI used to be here, then just its host. Neither * belongs on the page: none of the sign-in pages this was measured * against shows a redirect target, and for a native client it is a custom - * scheme or a loopback port, which reads like a fault. The domain is - * detail, so it sits behind the icon. + * scheme or a loopback port, which reads like a fault. For a registered + * client the domain is detail, so it sits behind the icon. An unverified + * client's destination is stated below instead, see redirectLocal above. */} {/* * text.secondary rather than the secondary palette colour, which lands on @@ -151,7 +188,7 @@ function Consent() { */} {t("konnect.consent.nothingShared", "Nothing is shared until you click Allow.")} - {redirectHost ? ( + {redirectHost && !unverified ? ( /* Above, because this line sits directly over Allow and Cancel. */ ) : null} + {unverified && (redirectHost || redirectLocal) ? ( + + {redirectHost + ? t("konnect.consent.redirect", 'Clicking "Allow" will redirect you to {{host}}', { host: redirectHost }) + : t( + "konnect.consent.redirectLocal", + 'Clicking "Allow" will redirect you to an application on this device ({{host}})', + { host: redirectLocal }, + )} + + ) : null} + {unverified && redirectLocal ? ( + + {/* The amber is the icon's alone, the sentence keeps the text colour it needs to be read. */} + + + {t( + "konnect.consent.localWarning", + "Only continue if you started this sign-in from that application yourself.", + )} + + + ) : null}
{/* diff --git a/identifier/src/locales/translations.test.tsx b/identifier/src/locales/translations.test.tsx index 14f3f034..3bdf79ca 100644 --- a/identifier/src/locales/translations.test.tsx +++ b/identifier/src/locales/translations.test.tsx @@ -121,4 +121,49 @@ for (const lng of Object.keys(locales)) { expect(screen.getAllByRole("button").length).toBeGreaterThanOrEqual(2); expect(text).toContain("User One"); }); + + it(`renders the resource and the unverified client notices in ${lng}`, async () => { + const i18n = getTestI18n(); + i18n.addResourceBundle(lng, "translation", locales[lng], true, true); + await i18n.changeLanguage(lng); + + const details: HelloResponse = { + ...hello, + client: { + ...client, + display_name: "app.example.test", + redirect_uri: "http://localhost:53682/callback", + unverified: true, + claimed_name: "Example CLI", + }, + resource: { id: "https://mcp.example.test/mcp", name: "Example MCP server" }, + }; + const { container } = renderWithProviders(, { + ...arriveAt("/consent"), + request: { query }, + preloaded: { + common: { + hello: { + state: details.success, + username: details.username, + displayName: details.displayName, + details, + }, + }, + }, + }); + + const text = container.textContent ?? ""; + expect(text).not.toMatch(/<\d+>/); + expect(text).not.toMatch(/\{\{/); + + // Both names are wrapped by the sentence, so both have to survive it. + const wrapped = [...container.querySelectorAll("span")].map((e) => e.textContent); + expect(wrapped).toContain("app.example.test"); + expect(wrapped).toContain("Example MCP server"); + + // Interpolated rather than wrapped, so only their presence can be checked. + expect(text).toContain("Example CLI"); + expect(text).toContain("localhost:53682"); + }); } diff --git a/identifier/src/models/api.ts b/identifier/src/models/api.ts index f99fc893..f1ff6f4e 100644 --- a/identifier/src/models/api.ts +++ b/identifier/src/models/api.ts @@ -20,6 +20,17 @@ export interface ClientDetails { display_name: string; redirect_uri: string; trusted: boolean; + // Nobody vouches for this client: it registered itself, or its id is the URL + // of a metadata document and display_name is the host of that URL. + unverified?: boolean; + // The name such a document gives, self-asserted. + claimed_name?: string; +} + +// The protected resource a consent is for, named by the server's registration. +export interface ResourceDetails { + id: string; + name?: string; } export interface ScopeDefinition { @@ -47,6 +58,7 @@ export interface HelloResponse { continue_uri?: string; scopes?: Record; client?: ClientDetails; + resource?: ResourceDetails; meta?: { scopes: ScopesMeta }; branding?: Branding; } diff --git a/identifier/src/models/hello.ts b/identifier/src/models/hello.ts index def50a7b..20347fbd 100644 --- a/identifier/src/models/hello.ts +++ b/identifier/src/models/hello.ts @@ -11,6 +11,7 @@ export interface HelloRequest { redirect_uri?: string; id_token_hint?: string; max_age?: string; + resource?: string; } export function newHelloRequest(flow: string | null | undefined, query: Record): HelloRequest { @@ -35,6 +36,9 @@ export function newHelloRequest(flow: string | null | undefined, query: Record