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..f258329f 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 @@ -589,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 @@ -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: | @@ -1712,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 @@ -1755,6 +1778,8 @@ components: $ref: '#/components/schemas/ScopesMap' client: $ref: '#/components/schemas/ClientDetails' + resource: + $ref: '#/components/schemas/ResourceDetails' meta: $ref: '#/components/schemas/Meta' branding: @@ -1874,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/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/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/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/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 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"` }