diff --git a/GETTING_STARTED.md b/GETTING_STARTED.md index a2b31088..3deb6239 100644 --- a/GETTING_STARTED.md +++ b/GETTING_STARTED.md @@ -1053,8 +1053,9 @@ func (p credentialIssuerPolicy) AllowResources(ctx context.Context, client stora A resource is checked against, in order: the client's own `storage.RegisteredClientConfig.AllowedResources`, if it registered any -(a client registered automatically through OpenID Federation has none, -so it may name any resource the server accepts); then +(a client registered automatically through OpenID Federation gets +`AutomaticRegistrationConfig.AllowedResources`, the same for all of +them, and without that may name any resource the server accepts); then `Config.Resources`; then, only for those the list doesn't name, `ResourcePolicy`. Anything refused is `invalid_target`. The client names resources at PAR — repeated `resource` form parameters, or a @@ -1065,7 +1066,18 @@ refresh; a refresh token always keeps the grant's whole set. The access token's `aud` is then the resource (a JSON string), or, for several, an array of them. A request naming no resource still gets the issuer as its audience, and with neither `Config.Resources` nor `ResourcePolicy` -set, `resource` is ignored as it always was. For a grant you serve +set, `resource` is ignored as it always was. + +A client confined by `AllowedResources` never gets an issuer-audience +token, though: every resource server accepting the issuer as an +audience would take it, whatever the list says. If it lists exactly one +resource, a request naming none — at PAR, in a CIBA request, for +client credentials, or to `IssueAccessToken`/`IssueRefreshToken` — is +for that one, recorded with the grant as if the client had named it and +checked against `Config.Resources` and `ResourcePolicy` the same way; +if it lists several, the request must name one, or it's +`invalid_target`. A grant made without resources before the client was +confined gets the same treatment at its next token request. For a grant you serve yourself (`Config.AdditionalGrantTypes`), pass `TokenEndpointRequest.Resources()` as `IssueAccessTokenRequest.Resources` and `IssueRefreshTokenRequest.Resources`, and check the resources your @@ -1112,10 +1124,14 @@ present it to another. Every FAPIgo access token is sender-constrained that doesn't want its resource servers to share a token narrows each token request to the one resource it's for. -The consent screen isn't told which resources a request names -(`InteractionRequest` doesn't carry them), so it can neither show them -to the user nor narrow them: they're the client's to choose and the -server's to allow. +The consent screen is told which resources the tokens would be for — +`InteractionRequest.Resources` (and `BackchannelInteractionRequest`'s +for CIBA), the client's default included — so where several resource +servers share the AS, show them: the user can then tell which one a +token is for. Approving, `GrantedAuthorization.Resources` can narrow +them to a subset of those; leaving it nil grants all of them, and one +that wasn't requested makes the completion fail, as an unrequested +scope does. While clients move to sending `resource`, a resource server can also accept tokens issued without one: `AcceptIssuerAudience: true` in @@ -1125,7 +1141,10 @@ server verifying the issuer audience accepts, works here too. A UserInfo endpoint keeps the default (no `Resource`): it accepts only tokens issued without a resource, so a client that also calls UserInfo names the AS's issuer as one of its resources (list it in `Config.Resources`) -and narrows each token request to the resource it's for. +and narrows each token request to the resource it's for. A client +confined by `AllowedResources` must list the issuer there too to call +UserInfo at all, and then, listing more than one, name a resource in +every request. ### Verify a request diff --git a/UPGRADING.md b/UPGRADING.md index d9e0a6ea..171b3d3e 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -17,6 +17,44 @@ and `keys/ephemeral` needs only the steps not marked *production only*. ## v0.61.0 +### A client confined by `AllowedResources` no longer gets issuer-audience tokens (server, storage) + +**Affects:** a server with resource indicators enabled (`Config.Resources` +or `Dependencies.ResourcePolicy`) where a client registered with +`storage.RegisteredClientConfig.AllowedResources` makes a request that +names no resource: at PAR, in a CIBA request, for client credentials, or +through `IssueAccessToken`/`IssueRefreshToken`. Also such a client's +grants made without resources, at their next code exchange, refresh or +CIBA token request. + +**Why:** a request naming no resource got an access token addressed to +the issuer, which `AllowedResources` was never checked against. Every +resource server accepting the issuer as an audience took it — one +migrating with `serverresource.Options.AcceptIssuerAudience` or the +issuer in `AdditionalAudiences`, and the UserInfo endpoint — so a client +refused a resource could get a token it accepts simply by naming none. + +**What fails:** for a client with exactly one `AllowedResources` entry, a +request naming no resource is now for that resource: the grant records +it, and its access tokens are addressed to it, not the issuer. It's +checked against `Config.Resources` and the `ResourcePolicy` as a named +one is, so if the server doesn't accept it the request fails with +`invalid_target`. For a client with several entries, a request naming +none fails with `invalid_target` ("a resource is required"). A grant made +without resources before the client was confined gets the same: its +single resource, or `invalid_target`, so the client must authorize +again. Nothing changes for clients without `AllowedResources`, or while +resource indicators are disabled. + +**What to change:** a confined client that also calls the UserInfo +endpoint, which accepts only issuer-audience tokens, lists the issuer +identifier in its `AllowedResources` (and the server accepts it, in +`Config.Resources`, say); having more than one entry, it then names a +resource in every request. A confined client with several entries names +the resource it wants on every request. To confine clients registered +through OpenID Federation too, set the new +`AutomaticRegistrationConfig.AllowedResources`. + ### A statically registered client authenticates with its static keys only (federation, server) **Affects:** a server with OpenID Federation automatic registration (or diff --git a/fapitest/resource_indicators_allowed_test.go b/fapitest/resource_indicators_allowed_test.go new file mode 100644 index 00000000..4bbb7389 --- /dev/null +++ b/fapitest/resource_indicators_allowed_test.go @@ -0,0 +1,91 @@ +package fapitest_test + +import ( + "context" + "fmt" + "testing" + + "github.com/idfoundry/fapigo/client" + "github.com/idfoundry/fapigo/fapitest" + "github.com/idfoundry/fapigo/server" + "github.com/idfoundry/fapigo/serverresource" +) + +// TestResourceIndicatorsAllowedResourcesDefault: a client confined by +// AllowedResources to one resource server that names no resource gets +// tokens for that resource, end to end, never ones addressed to the +// issuer, which another resource server migrating with +// AcceptIssuerAudience (or the issuer's own, UserInfo say) would accept. +func TestResourceIndicatorsAllowedResourcesDefault(t *testing.T) { + for _, opaque := range []bool{false, true} { + t.Run(fmt.Sprintf("opaque=%v", opaque), func(t *testing.T) { + h := fapitest.New(t, fapitest.Config{ + Profile: server.ProfileFAPISecurity, Resources: servedResources, + AllowedResources: []string{credentials}, OpaqueAccessTokens: opaque, + }) + ctx := context.Background() + _, err := h.RunAuthorizationCodeFlowWithRequest(ctx, client.BeginAuthorizationRequest{Scope: offlineScope, Resource: []string{payments}}) + wantServerError(t, "RunAuthorizationCodeFlow(payments)", err, "invalid_target") + + tokens, err := h.RunAuthorizationCodeFlow(ctx, offlineScope) + if err != nil { + t.Fatalf("RunAuthorizationCodeFlow: %v", err) + } + checkConfinedToCredentials(t, h, tokens, opaque) + + // A refresh naming no resource gets the grant's: the default. + refreshed, err := h.Client.RefreshTokens(ctx, client.RefreshTokenRequest{Tokens: tokens}) + if err != nil { + t.Fatalf("RefreshTokens: %v", err) + } + checkConfinedToCredentials(t, h, refreshed, opaque) + }) + } +} + +// TestResourceIndicatorsAllowedResourcesDefaultClientCredentials is +// TestResourceIndicatorsAllowedResourcesDefault with no user involved. +func TestResourceIndicatorsAllowedResourcesDefaultClientCredentials(t *testing.T) { + h := fapitest.New(t, fapitest.Config{ + Profile: server.ProfileFAPISecurity, ClientCredentialsGrant: true, + Resources: servedResources, AllowedResources: []string{credentials}, + }) + result, err := h.Client.RequestClientCredentialsToken(context.Background(), client.ClientCredentialsTokenRequest{Scope: []string{"accounts"}}) + if err != nil { + t.Fatalf("RequestClientCredentialsToken: %v", err) + } + checkConfinedToCredentials(t, h, client.TokenSet{AccessToken: result.AccessToken}, false) +} + +// TestResourceIndicatorsAllowedResourcesRequired: a client confined to +// several resource servers must name which a request is for. +func TestResourceIndicatorsAllowedResourcesRequired(t *testing.T) { + h := fapitest.New(t, fapitest.Config{ + Profile: server.ProfileFAPISecurity, ClientCredentialsGrant: true, + Resources: servedResources, AllowedResources: []string{credentials, payments}, + }) + ctx := context.Background() + _, err := h.RunAuthorizationCodeFlow(ctx, offlineScope) + wantServerError(t, "RunAuthorizationCodeFlow", err, "invalid_target") + _, err = h.Client.RequestClientCredentialsToken(ctx, client.ClientCredentialsTokenRequest{Scope: []string{"accounts"}}) + wantServerError(t, "RequestClientCredentialsToken", err, "invalid_target") + + tokens, err := h.RunAuthorizationCodeFlowWithRequest(ctx, client.BeginAuthorizationRequest{Scope: offlineScope, Resource: []string{payments}}) + if err != nil { + t.Fatalf("RunAuthorizationCodeFlow(payments): %v", err) + } + checkVerifierAudience(t, h, tokens, payments, []string{payments}) +} + +// checkConfinedToCredentials checks tokens are for credentials alone: +// accepted there, and refused by payments even while it accepts the +// issuer as an audience, and by the issuer's own resource server. +func checkConfinedToCredentials(t *testing.T, h *fapitest.Harness, tokens client.TokenSet, opaque bool) { + t.Helper() + if !opaque { + checkJWTAudience(t, tokens, `"`+credentials+`"`) + } + checkVerifierAudience(t, h, tokens, credentials, []string{credentials}) + refuseAtResource(t, h, tokens, serverresource.Options{Resource: payments, AcceptIssuerAudience: true}) + refuseAt(t, h, h.Resource, tokens) +} diff --git a/fapitest/resource_indicators_test.go b/fapitest/resource_indicators_test.go index 467a1545..972d3f74 100644 --- a/fapitest/resource_indicators_test.go +++ b/fapitest/resource_indicators_test.go @@ -422,13 +422,12 @@ func TestResourceIndicatorsClientCredentials(t *testing.T) { refuseAtResource(t, h, tokens, serverresource.Options{Resource: payments}) }) + // Confined to several resources, the client must name + // which: a token addressed to the issuer would be accepted + // wherever the issuer is. t.Run("no resource", func(t *testing.T) { - tokens, err := request(nil) - if err != nil { - t.Fatalf("RequestClientCredentialsToken: %v", err) - } - verifyAt(t, h, h.Resource, tokens) - refuseAtResource(t, h, tokens, serverresource.Options{Resource: accounts}) + _, err := request(nil) + wantServerError(t, "RequestClientCredentialsToken", err, "invalid_target") }) for _, tc := range []struct { diff --git a/federation/automatic_registration.go b/federation/automatic_registration.go index c1a33219..f4387816 100644 --- a/federation/automatic_registration.go +++ b/federation/automatic_registration.go @@ -19,6 +19,7 @@ import ( intfed "github.com/idfoundry/fapigo/internal/federation" "github.com/idfoundry/fapigo/internal/jose" "github.com/idfoundry/fapigo/internal/mtls" + "github.com/idfoundry/fapigo/internal/resourceindicator" "github.com/idfoundry/fapigo/internal/strictjson" "github.com/idfoundry/fapigo/keys" "github.com/idfoundry/fapigo/storage" @@ -51,6 +52,19 @@ type AutomaticRegistrationConfig struct { // self-published authorization_details_types. Empty means none. AuthorizationDetailsTypes []string + // AllowedResources confines every automatically-registered client to + // these RFC 8707 resource indicators + // (storage.RegisteredClientConfig.AllowedResources) — for the same + // reason as AllowedScopes, never anything an RP publishes about + // itself: which resource servers a client may get tokens for is the + // operator's decision. Each an absolute URI without a fragment, none + // repeated. Empty means unconfined: an RP may name any resource the + // server accepts, and one naming none gets tokens addressed to the + // issuer. Set, a request naming none is for its single entry, or, + // with several, invalid_target (see + // storage.RegisteredClientConfig.AllowedResources). + AllowedResources []string + // MaxCacheAge bounds how long a resolved client's Trust Chain is // reused before it is resolved again — the actual cache lifetime // for a given client is min(MaxCacheAge, its own Trust Chain's @@ -373,6 +387,10 @@ func NewAutomaticClientRepository(underlying storage.ClientRepository, resolver if err := validateSubjectTypes(cfg.SubjectTypes, cfg.DefaultSubjectType); err != nil { return nil, err } + if err := validateAllowedResources(cfg.AllowedResources); err != nil { + return nil, err + } + cfg.AllowedResources = slices.Clone(cfg.AllowedResources) if clock == nil { return nil, fmt.Errorf("federation: clock is required") } @@ -384,6 +402,20 @@ func NewAutomaticClientRepository(underlying storage.ClientRepository, resolver }, nil } +// validateAllowedResources checks AutomaticRegistrationConfig.AllowedResources: +// each a resource indicator RFC 8707 §2 allows, none repeated. +func validateAllowedResources(resources []string) error { + for i, r := range resources { + if err := resourceindicator.Check(r); err != nil { + return fmt.Errorf("federation: config: allowed_resources[%d]: %w", i, err) + } + if slices.Contains(resources[:i], r) { + return fmt.Errorf("federation: config: allowed_resources[%d]: %q is repeated", i, r) + } + } + return nil +} + // ResolveClient implements storage.ClientRepository. func (a *AutomaticClientRepository) ResolveClient(ctx context.Context, id fapi.ClientID) (storage.RegisteredClient, error) { client, err := a.underlying.ResolveClient(ctx, id) @@ -755,7 +787,7 @@ type relyingPartyMetadata struct { // choiceLimits). // // a.cfg supplies the operator-level capability grants (AllowedScopes, -// AllowsClientCredentialsGrant, AllowsCIBA) that must never be inferred +// AllowedResources, AllowsClientCredentialsGrant, AllowsCIBA) that must never be inferred // from raw itself, plus the AllowedClientAuthMethods restriction raw's // own token_endpoint_auth_method is checked against — see AutomaticRegistrationConfig's own doc comments // for why. @@ -810,6 +842,7 @@ func (a *AutomaticClientRepository) registeredClientConfigFromMetadata(ctx conte SenderConstrain: senderConstrain, AllowedScopes: a.cfg.AllowedScopes, AuthorizationDetailsTypes: a.cfg.AuthorizationDetailsTypes, + AllowedResources: a.cfg.AllowedResources, AutomaticFederationRegistration: true, AllowsClientCredentialsGrant: a.cfg.AllowsClientCredentialsGrant, } diff --git a/federation/automatic_registration_test.go b/federation/automatic_registration_test.go index 68888949..cb47fee8 100644 --- a/federation/automatic_registration_test.go +++ b/federation/automatic_registration_test.go @@ -292,6 +292,16 @@ func TestNewAutomaticClientRepositoryRejectsInvalidConfig(t *testing.T) { cfg.FailureCacheAge = -time.Second return alwaysFailsRepository{}, resolver, f.fetcher, cfg, fixedClock{now: f.now} }, + "relative allowed resource": func() (storage.ClientRepository, *federation.Resolver, *fapihttp.Client, federation.AutomaticRegistrationConfig, federation.Clock) { + cfg := validAutomaticRegistrationConfig() + cfg.AllowedResources = []string{"/api"} + return alwaysFailsRepository{}, resolver, f.fetcher, cfg, fixedClock{now: f.now} + }, + "repeated allowed resource": func() (storage.ClientRepository, *federation.Resolver, *fapihttp.Client, federation.AutomaticRegistrationConfig, federation.Clock) { + cfg := validAutomaticRegistrationConfig() + cfg.AllowedResources = []string{"https://rs.example", "https://rs.example"} + return alwaysFailsRepository{}, resolver, f.fetcher, cfg, fixedClock{now: f.now} + }, "invalid allowed client auth method": func() (storage.ClientRepository, *federation.Resolver, *fapihttp.Client, federation.AutomaticRegistrationConfig, federation.Clock) { cfg := validAutomaticRegistrationConfig() cfg.AllowedClientAuthMethods = []storage.ClientAuthMethod{storage.ClientAuthMethod(255)} @@ -377,6 +387,9 @@ func TestAutomaticClientRepositoryResolveClientFallsBackToFederation(t *testing. if got.AllowsAuthorizationDetailsType("payment") { t.Error("AllowsAuthorizationDetailsType(payment) = true with no AutomaticRegistrationConfig.AuthorizationDetailsTypes") } + if got.AllowedResources() != nil { + t.Errorf("AllowedResources() = %q with no AutomaticRegistrationConfig.AllowedResources", got.AllowedResources()) + } } // unavailableRepository is a storage.ClientRepository whose backend @@ -428,6 +441,31 @@ func TestAutomaticClientRepositoryAuthorizationDetailsTypesComeFromConfig(t *tes } } +// TestAutomaticClientRepositoryAllowedResourcesComeFromConfig covers an +// automatically registered client being confined to the operator's +// resources, never ones from the RP's own metadata, and the config's +// slice being copied. +func TestAutomaticClientRepositoryAllowedResourcesComeFromConfig(t *testing.T) { + f := setupAutomaticRegistrationFixture(t, rpMetadataBuilder(t)) + cfg := validAutomaticRegistrationConfig() + cfg.AllowedResources = []string{"https://rs.example", "https://other.example"} + repo, err := federation.NewAutomaticClientRepository(alwaysFailsRepository{}, f.newResolver(t), f.fetcher, cfg, fixedClock{now: f.now}) + if err != nil { + t.Fatalf("NewAutomaticClientRepository: %v", err) + } + cfg.AllowedResources[0] = "https://changed.example" + got, err := repo.ResolveClient(context.Background(), fapi.ClientID(f.rpID)) + if err != nil { + t.Fatalf("ResolveClient: %v", err) + } + if want := []string{"https://other.example", "https://rs.example"}; !slices.Equal(got.AllowedResources(), want) { + t.Errorf("AllowedResources() = %q, want %q", got.AllowedResources(), want) + } + if got.AllowsResource("https://changed.example") { + t.Error("a change to the config's slice after construction reached the client") + } +} + func TestAutomaticClientRepositoryResolveClientDoesNotAllowClientCredentialsByDefault(t *testing.T) { f := setupAutomaticRegistrationFixture(t, rpMetadataBuilder(t)) resolver := f.newResolver(t) diff --git a/server/authorization.go b/server/authorization.go index ca45bac9..5215048b 100644 --- a/server/authorization.go +++ b/server/authorization.go @@ -3,6 +3,7 @@ package server import ( "context" "encoding/json" + "slices" "strings" "time" @@ -152,6 +153,7 @@ func (s *Server) BeginAuthorization(ctx context.Context, req BeginAuthorizationR interaction.RequiredSubject = request.requiredSubjectForApplication() interaction.ClientDisplay = client.Display() interaction.Extensions = extensions + interaction.Resources = slices.Clone(request.Resources) action := InteractionRequired{ Handle: InteractionHandle{value: handle}, diff --git a/server/automatic_registration_test.go b/server/automatic_registration_test.go index abfd3780..2a8c47b7 100644 --- a/server/automatic_registration_test.go +++ b/server/automatic_registration_test.go @@ -388,6 +388,48 @@ func TestNewWiresAutomaticRegistrationAllowedClientAuthMethods(t *testing.T) { } } +// TestNewWiresAutomaticRegistrationAllowedResources proves server.New +// passes AutomaticRegistrationConfig.AllowedResources through: the RP +// may name only those, and, confined to several, must name one. +func TestNewWiresAutomaticRegistrationAllowedResources(t *testing.T) { + f := setupAutomaticRegistrationFixture(t, testRedirectURI) + srv := newAutomaticRegistrationTestServer(t, f, func(cfg *server.Config, deps *server.Dependencies) { + cfg.Resources = []string{resourceA, resourceB, resourceC} + cfg.AutomaticRegistration.AllowedResources = []string{resourceA, resourceB} + }) + for name, tc := range map[string]struct { + extra map[string]string + want server.ErrorCode + }{ + "allowed": {extra: map[string]string{"resource": resourceB}}, + "not allowed": {extra: map[string]string{"resource": resourceC}, want: server.ErrorInvalidTarget}, + "none named": {want: server.ErrorInvalidTarget}, + } { + t.Run(name, func(t *testing.T) { + assertion, err := clientassertion.CreateAssertion(clientassertion.AssertionRequest{ + Signer: f.rpOIDCKey, Algorithm: fapi.ES256, KeyID: "rp-oidc", + ClientID: f.rpID, Audience: testIssuer, + Now: f.now, Lifetime: 30 * time.Second, + }) + if err != nil { + t.Fatalf("CreateAssertion: %v", err) + } + _, err = srv.PushAuthorizationRequest(context.Background(), server.PushAuthorizationRequest{ + HTTP: server.FormRequest{Parameters: plainFormParameters(t, assertion, tc.extra)}, + }) + if tc.want == "" { + if err != nil { + t.Fatalf("PushAuthorizationRequest: %v", err) + } + return + } + if code := serverErrorCode(t, err); code != tc.want { + t.Fatalf("error code = %q, want %q", code, tc.want) + } + }) + } +} + // TestNewWiresAutomaticRegistrationOnResolutionFailure proves server.New // passes AutomaticRegistrationConfig.OnResolutionFailure through: the // operator's hook hears why the RP was refused, while the RP itself diff --git a/server/backchannel_authentication.go b/server/backchannel_authentication.go index 50c31e32..8106ce8b 100644 --- a/server/backchannel_authentication.go +++ b/server/backchannel_authentication.go @@ -190,7 +190,7 @@ func (s *Server) BeginBackchannelAuthentication(ctx context.Context, req BeginBa return s.backchannelBeginFail(ctx, client.ID(), dpopErr), nil } - interaction, err := s.backchannelInteraction(client, validated.params) + interaction, err := s.backchannelInteraction(client, validated.params, validated.resources) if err != nil { return s.backchannelBeginFail(ctx, client.ID(), newError(ErrorServerError, 500, "validated extension parameters could not be read back", err)), nil } @@ -420,6 +420,30 @@ func (s *Server) validateBackchannelAuthenticationParameters(verified verifiedBa return verifiedBackchannelRequest{}, newError(ErrorInvalidScope, 400, "scope must include openid", nil) } + if hintErr := validateBackchannelHints(params); hintErr != nil { + return verifiedBackchannelRequest{}, hintErr + } + + if validateErr := validateClientNotificationToken(params, client); validateErr != nil { + return verifiedBackchannelRequest{}, validateErr + } + if err := validateClaimsParameter(params); err != nil { + return verifiedBackchannelRequest{}, newError(ErrorInvalidRequest, 400, err.Error(), nil) + } + if _, err := essentialACRValues(params); err != nil { + return verifiedBackchannelRequest{}, newError(ErrorInvalidRequest, 400, err.Error(), nil) + } + + if bindingErr := validateBindingMessage(params); bindingErr != nil { + return verifiedBackchannelRequest{}, bindingErr + } + + return verified, nil +} + +// validateBackchannelHints checks params carries exactly one identity +// hint (CIBA §7.1): login_hint, login_hint_token or id_token_hint. +func validateBackchannelHints(params map[string]json.RawMessage) *Error { hints := 0 for _, name := range [...]string{"login_hint", "login_hint_token", "id_token_hint"} { raw, ok := params[name] @@ -431,35 +455,31 @@ func (s *Server) validateBackchannelAuthenticationParameters(verified verifiedBa // reach the application as no hint at all // (backchannelInteractionRequestFrom keeps only string values). if v, err := jsonStringValue(raw); err != nil || v == "" { - return verifiedBackchannelRequest{}, newError(ErrorInvalidRequest, 400, name+" must be a non-empty string", err) + return newError(ErrorInvalidRequest, 400, name+" must be a non-empty string", err) } hints++ } if hints != 1 { - return verifiedBackchannelRequest{}, newError(ErrorInvalidRequest, 400, "exactly one of login_hint, login_hint_token, or id_token_hint is required", nil) + return newError(ErrorInvalidRequest, 400, "exactly one of login_hint, login_hint_token, or id_token_hint is required", nil) } + return nil +} - if validateErr := validateClientNotificationToken(params, client); validateErr != nil { - return verifiedBackchannelRequest{}, validateErr - } - if err := validateClaimsParameter(params); err != nil { - return verifiedBackchannelRequest{}, newError(ErrorInvalidRequest, 400, err.Error(), nil) +// validateBindingMessage checks params' binding_message, if any, is a +// string the authentication device can show (CIBA §7.1). +func validateBindingMessage(params map[string]json.RawMessage) *Error { + raw, ok := params["binding_message"] + if !ok { + return nil } - if _, err := essentialACRValues(params); err != nil { - return verifiedBackchannelRequest{}, newError(ErrorInvalidRequest, 400, err.Error(), nil) + bindingMessage, err := jsonStringValue(raw) + if err != nil { + return newError(ErrorInvalidRequest, 400, "binding_message must be a string", err) } - - if raw, ok := params["binding_message"]; ok { - bindingMessage, err := jsonStringValue(raw) - if err != nil { - return verifiedBackchannelRequest{}, newError(ErrorInvalidRequest, 400, "binding_message must be a string", err) - } - if !isAcceptableBindingMessage(bindingMessage) { - return verifiedBackchannelRequest{}, newError(ErrorInvalidBindingMessage, 400, "binding_message is not acceptable", nil) - } + if !isAcceptableBindingMessage(bindingMessage) { + return newError(ErrorInvalidBindingMessage, 400, "binding_message is not acceptable", nil) } - - return verified, nil + return nil } // validateClientNotificationToken checks client_notification_token's @@ -684,11 +704,13 @@ func (s *Server) backchannelBeginFail(ctx context.Context, clientID fapi.ClientI } // backchannelInteraction is the BackchannelInteractionRequest for a -// request from client with params, as BeginBackchannelAuthentication and -// LookupBackchannelInteraction return it. -func (s *Server) backchannelInteraction(client storage.RegisteredClient, params map[string]json.RawMessage) (BackchannelInteractionRequest, error) { +// request from client with params, for resources, as +// BeginBackchannelAuthentication and LookupBackchannelInteraction return +// it. +func (s *Server) backchannelInteraction(client storage.RegisteredClient, params map[string]json.RawMessage, resources []string) (BackchannelInteractionRequest, error) { interaction := s.backchannelInteractionRequestFrom(client.ID(), params) interaction.ClientDisplay = client.Display() + interaction.Resources = slices.Clone(resources) extensions, err := s.interactionExtensions(params, coreBackchannelAuthenticationParameters, extension.SourceRequestObject) if err != nil { return BackchannelInteractionRequest{}, err @@ -725,7 +747,7 @@ func (s *Server) LookupBackchannelInteraction(ctx context.Context, handle Backch if err != nil { return BackchannelInteractionRequest{}, newError(ErrorServerError, 500, "the request's client could not be resolved", err) } - interaction, err := s.backchannelInteraction(client, request.Parameters) + interaction, err := s.backchannelInteraction(client, request.Parameters, request.Resources) if err != nil { return BackchannelInteractionRequest{}, newError(ErrorServerError, 500, "the request's extension parameters could not be read back", err) } diff --git a/server/backchannel_interaction.go b/server/backchannel_interaction.go index a9840427..069dcf89 100644 --- a/server/backchannel_interaction.go +++ b/server/backchannel_interaction.go @@ -146,6 +146,11 @@ type BackchannelInteractionRequest struct { // exactly, for the CIBA flow. AuthorizationDetails extension.RARValues + // Resources mirrors InteractionRequest.Resources for the CIBA flow: + // the resource servers the tokens would be for, which + // GrantedAuthorization.Resources can narrow. + Resources []string + // RequestedClaims is the identity claims the client asked for with // the "claims" parameter — see InteractionRequest.RequestedClaims. RequestedClaims RequestedClaims diff --git a/server/backchannel_test.go b/server/backchannel_test.go index ad70fe1a..38601096 100644 --- a/server/backchannel_test.go +++ b/server/backchannel_test.go @@ -1331,6 +1331,27 @@ func TestBeginBackchannelAuthenticationRejectsBindingMessageWithControlCharacter } } +func TestBeginBackchannelAuthenticationRejectsNonStringBindingMessage(t *testing.T) { + h, _ := newHarnessWithBackchannel(t) + params := standardBackchannelParams(t) + params["binding_message"] = jsonRaw(t, 1234) + requestObj := h.backchannelRequestObject(t, params) + + action, err := h.server.BeginBackchannelAuthentication(context.Background(), server.BeginBackchannelAuthenticationRequest{ + HTTP: server.FormRequest{Parameters: backchannelFormParams(h.clientAssertion(t), requestObj)}, + }) + if err != nil { + t.Fatalf("BeginBackchannelAuthentication: %v", err) + } + localErr, ok := action.(server.BackchannelAuthenticationLocalError) + if !ok { + t.Fatalf("action = %T, want server.BackchannelAuthenticationLocalError", action) + } + if localErr.Error.Code() != server.ErrorInvalidRequest { + t.Fatalf("Code = %q, want %q", localErr.Error.Code(), server.ErrorInvalidRequest) + } +} + func TestBeginBackchannelAuthenticationRejectsWhenCIBADisabled(t *testing.T) { h := newHarness(t, server.ProfileFAPISecurity, true) // CIBA not configured diff --git a/server/complete_authorization.go b/server/complete_authorization.go index 2b204729..9a34dc94 100644 --- a/server/complete_authorization.go +++ b/server/complete_authorization.go @@ -3,6 +3,7 @@ package server import ( "context" "crypto/sha256" + "encoding/json" "fmt" "net/url" "strings" @@ -126,61 +127,13 @@ func (s *Server) completeAuthorize(ctx context.Context, clientID fapi.ClientID, return s.completeLocalFail(ctx, clientID, authorizeErr), nil } - // OIDC Core §3.1.2.1: with max_age, an authentication older than - // max_age must be repeated, and if it isn't, the client gets an - // error, "typically login_required" (§3.1.2.1, prompt=login). - if maxAge, ok, _ := requestedMaxAge(request.Parameters); ok { - oldest := s.deps.Clock.Now().Add(-maxAge - s.cfg.Limits.MaxClockSkew) - if result.auth.authTime.Before(oldest) { - return s.completeErrorRedirect(ctx, clientID, redirectURI, state, "login_required", "the user's authentication is older than the requested max_age", AuditOutcomeFailure) - } - } - - // OIDC Core §3.1.2.1: prompt=login asks the OP to "reauthenticate the - // End-User even if the End-User is already authenticated", and if it - // can't, to return login_required. An authentication from before the - // request was pushed, allowing the same clock skew as max_age, didn't - // happen for this request. A record without PushedAt (written before - // it existed) isn't checked. - if prompt, _ := requestedPrompt(request.Parameters); prompt.Has(PromptLogin) && request.PushedAt != nil { - if result.auth.authTime.Before(request.PushedAt.Add(-s.cfg.Limits.MaxClockSkew)) { - return s.completeErrorRedirect(ctx, clientID, redirectURI, state, "login_required", "the user didn't authenticate again for prompt=login", AuditOutcomeFailure) - } + if problem := s.loginRequiredProblem(request, result.auth, clientSub); problem != "" { + return s.completeErrorRedirect(ctx, clientID, redirectURI, state, "login_required", problem, AuditOutcomeFailure) } - // OIDC Core §5.5.1.1: an essential "acr" request with values must be - // met, or the outcome treated as a failed authentication. - if !meetsEssentialACR(request.EssentialACRValues, result.auth.acr) { - return s.completeErrorRedirect(ctx, clientID, redirectURI, state, "login_required", "the user's authentication doesn't meet the requested essential acr", AuditOutcomeFailure) - } - - // OIDC Core §5.5.1: a request naming its end user, by id_token_hint - // or a "sub" value, must not be answered for a different one; that - // user didn't authenticate, so it's login_required. - if !meetsRequiredSubject(request.RequiredSubject, clientSub) { - return s.completeErrorRedirect(ctx, clientID, redirectURI, state, "login_required", "the authenticated user isn't the one the request named", AuditOutcomeFailure) - } - - requestedScope, _ := jsonString(request.Parameters, "scope") - if err := validateGrantedScopeSubset(result.grant.Scope, requestedScope); err != nil { - return s.completeLocalFail(ctx, clientID, newError(ErrorInvalidRequest, 400, "granted scope exceeds requested scope", err)), nil - } - - grantedAuthorizationDetails, err := s.validateGrantedAuthorizationDetails(request.Parameters[authorizationDetailsParameter], result.grant.AuthorizationDetails) - if err != nil { - return s.completeLocalFail(ctx, clientID, newError(ErrorInvalidRequest, 400, "granted authorization_details exceeds what was requested", err)), nil - } - - if err := validateGrantedIDTokenClaims(result.grant.IDTokenClaims, s.cfg.Limits.MaxIDTokenClaimsBytes); err != nil { - return s.completeLocalFail(ctx, clientID, newError(ErrorInvalidRequest, 400, "granted ID token claims are not valid", err)), nil - } - if err := s.validateGrantID(result.grant.GrantID); err != nil { - return s.completeLocalFail(ctx, clientID, newError(ErrorInvalidRequest, 400, "the grant ID is not valid", err)), nil - } - - idTokenClaims, userinfoClaims, err := approvedClaimNames(request.Parameters, result.grant.ApprovedIdentityClaims) - if err != nil { - return s.completeLocalFail(ctx, clientID, newError(ErrorInvalidRequest, 400, "approved identity claims exceed what was requested", err)), nil + approved, approvedErr := s.checkApprovedGrant(request, result.grant) + if approvedErr != nil { + return s.completeLocalFail(ctx, clientID, approvedErr), nil } codeChallenge, err := jsonString(request.Parameters, "code_challenge") @@ -210,11 +163,11 @@ func (s *Server) completeAuthorize(ctx context.Context, clientID fapi.ClientID, AuthTime: result.auth.authTime, ACR: result.auth.acr, AMR: result.auth.amr, - AuthorizationDetails: grantedAuthorizationDetails, - Resources: request.Resources, + AuthorizationDetails: approved.authorizationDetails, + Resources: approved.resources, TokenClaims: request.TokenClaims, - RequestedIDTokenClaims: idTokenClaims, - RequestedUserinfoClaims: userinfoClaims, + RequestedIDTokenClaims: approved.idTokenClaims, + RequestedUserinfoClaims: approved.userinfoClaims, IDTokenClaims: result.grant.IDTokenClaims, GrantID: result.grant.GrantID, }) @@ -242,6 +195,85 @@ func (s *Server) completeAuthorize(ctx context.Context, clientID fapi.ClientID, return AuthorizationRedirect{destination: destination}, nil } +// loginRequiredProblem describes why auth, the authentication of the +// user whose sub at the client is clientSub, doesn't meet request, so +// the client gets login_required, or returns "" when it does. +func (s *Server) loginRequiredProblem(request requestRecord, auth AuthenticationContext, clientSub string) string { + // OIDC Core §3.1.2.1: with max_age, an authentication older than + // max_age must be repeated, and if it isn't, the client gets an + // error, "typically login_required" (§3.1.2.1, prompt=login). + if maxAge, ok, _ := requestedMaxAge(request.Parameters); ok { + oldest := s.deps.Clock.Now().Add(-maxAge - s.cfg.Limits.MaxClockSkew) + if auth.authTime.Before(oldest) { + return "the user's authentication is older than the requested max_age" + } + } + + // OIDC Core §3.1.2.1: prompt=login asks the OP to "reauthenticate the + // End-User even if the End-User is already authenticated", and if it + // can't, to return login_required. An authentication from before the + // request was pushed, allowing the same clock skew as max_age, didn't + // happen for this request. A record without PushedAt (written before + // it existed) isn't checked. + if prompt, _ := requestedPrompt(request.Parameters); prompt.Has(PromptLogin) && request.PushedAt != nil { + if auth.authTime.Before(request.PushedAt.Add(-s.cfg.Limits.MaxClockSkew)) { + return "the user didn't authenticate again for prompt=login" + } + } + + // OIDC Core §5.5.1.1: an essential "acr" request with values must be + // met, or the outcome treated as a failed authentication. + if !meetsEssentialACR(request.EssentialACRValues, auth.acr) { + return "the user's authentication doesn't meet the requested essential acr" + } + + // OIDC Core §5.5.1: a request naming its end user, by id_token_hint + // or a "sub" value, must not be answered for a different one; that + // user didn't authenticate, so it's login_required. + if !meetsRequiredSubject(request.RequiredSubject, clientSub) { + return "the authenticated user isn't the one the request named" + } + return "" +} + +// approvedGrant is what an application's GrantedAuthorization approves +// of a pending authorization request, checked against it. +type approvedGrant struct { + authorizationDetails json.RawMessage + resources []string + idTokenClaims, userinfoClaims []string +} + +// checkApprovedGrant checks grant against request, what the client +// asked for: its scope, authorization details, resources and identity +// claims may only narrow the request's, and its ID token claims and +// grant ID must be valid. Any other grant is invalid_request. +func (s *Server) checkApprovedGrant(request requestRecord, grant GrantedAuthorization) (approvedGrant, *Error) { + requestedScope, _ := jsonString(request.Parameters, "scope") + if err := validateGrantedScopeSubset(grant.Scope, requestedScope); err != nil { + return approvedGrant{}, newError(ErrorInvalidRequest, 400, "granted scope exceeds requested scope", err) + } + details, err := s.validateGrantedAuthorizationDetails(request.Parameters[authorizationDetailsParameter], grant.AuthorizationDetails) + if err != nil { + return approvedGrant{}, newError(ErrorInvalidRequest, 400, "granted authorization_details exceeds what was requested", err) + } + resources, resourcesErr := grantedResources(request.Resources, grant.Resources) + if resourcesErr != nil { + return approvedGrant{}, resourcesErr + } + if err := validateGrantedIDTokenClaims(grant.IDTokenClaims, s.cfg.Limits.MaxIDTokenClaimsBytes); err != nil { + return approvedGrant{}, newError(ErrorInvalidRequest, 400, "granted ID token claims are not valid", err) + } + if err := s.validateGrantID(grant.GrantID); err != nil { + return approvedGrant{}, newError(ErrorInvalidRequest, 400, "the grant ID is not valid", err) + } + idTokenClaims, userinfoClaims, err := approvedClaimNames(request.Parameters, grant.ApprovedIdentityClaims) + if err != nil { + return approvedGrant{}, newError(ErrorInvalidRequest, 400, "approved identity claims exceed what was requested", err) + } + return approvedGrant{authorizationDetails: details, resources: resources, idTokenClaims: idTokenClaims, userinfoClaims: userinfoClaims}, nil +} + func (s *Server) completeErrorRedirect(ctx context.Context, clientID fapi.ClientID, redirectURI, state, errorCode, description string, outcome AuditOutcome) (AuthorizationResult, error) { params := map[string]string{"error": errorCode, "state": state} // description may be the application's own reason (Deny, diff --git a/server/complete_backchannel_authentication.go b/server/complete_backchannel_authentication.go index 1458be82..f89931db 100644 --- a/server/complete_backchannel_authentication.go +++ b/server/complete_backchannel_authentication.go @@ -176,6 +176,12 @@ func (s *Server) approveBackchannelAuthentication(ctx context.Context, clientID return wrapped } + resources, resourcesErr := grantedResources(request.Resources, result.grant.Resources) + if resourcesErr != nil { + s.audit(ctx, AuditEventCompleteBackchannelAuthentication, clientID, AuditOutcomeFailure, string(resourcesErr.Code())) + return resourcesErr + } + if claimsErr := validateGrantedIDTokenClaims(result.grant.IDTokenClaims, s.cfg.Limits.MaxIDTokenClaimsBytes); claimsErr != nil { wrapped := newError(ErrorInvalidRequest, 400, "granted ID token claims are not valid", claimsErr) s.audit(ctx, AuditEventCompleteBackchannelAuthentication, clientID, AuditOutcomeFailure, string(wrapped.Code())) @@ -216,7 +222,7 @@ func (s *Server) approveBackchannelAuthentication(ctx context.Context, clientID ACR: result.auth.acr, AMR: result.auth.amr, AuthorizationDetails: grantedAuthorizationDetails, - Resources: request.Resources, + Resources: resources, TokenClaims: request.TokenClaims, RequestedIDTokenClaims: idTokenClaims, RequestedUserinfoClaims: userinfoClaims, diff --git a/server/config.go b/server/config.go index da8c821e..87e31252 100644 --- a/server/config.go +++ b/server/config.go @@ -362,7 +362,7 @@ type Limits struct { type Config struct { // Issuer is this server's issuer identifier — the audience client // assertions and request objects must be addressed to, and every - // access token's audience unless the client named resources (RFC + // access token's audience unless the token is for resources (RFC // 8707 resource indicators; see Resources). Issuer fapi.URL @@ -487,7 +487,9 @@ type Config struct { // neither set, the "resource" parameter is ignored, as any // unregistered parameter is, and every access token's audience is // Issuer. A request naming no resource also gets Issuer as its - // audience. + // audience — unless its client is confined by AllowedResources, + // whose single entry it is then for, or which, listing several, + // make naming one required (invalid_target otherwise). Resources []string // AttestationBasedClientAuthentication enables @@ -600,6 +602,17 @@ type AutomaticRegistrationConfig struct { // refuses one that isn't. AuthorizationDetailsTypes []string + // AllowedResources confines every automatically-registered client to + // these RFC 8707 resource indicators, as + // storage.RegisteredClientConfig.AllowedResources confines a + // statically registered one — see + // federation.AutomaticRegistrationConfig.AllowedResources for why + // this never comes from an RP's own metadata. Each an absolute URI + // without a fragment, none repeated. Empty (the default) leaves them + // unconfined: any resource Config.Resources or the ResourcePolicy + // accepts, and the issuer as audience when they name none. + AllowedResources []string + // MaxPathLength/MaxAuthorityHints/MaxStatementLifetime/MaxClockSkew // bound Trust Chain resolution itself — see federation.Limits, which these configure // directly. All required when TrustAnchors is set. diff --git a/server/embedder_access_token.go b/server/embedder_access_token.go index 747bda2a..08ce9881 100644 --- a/server/embedder_access_token.go +++ b/server/embedder_access_token.go @@ -43,9 +43,10 @@ type IssueAccessTokenRequest struct { AuthorizationDetails []json.RawMessage // Resources are the RFC 8707 resource indicators the access token is - // restricted to, checked as IssueRefreshTokenRequest.Resources are: - // its "aud" is them instead of the issuer. Give a refresh token - // issued for the same grant the same Resources. + // restricted to, checked, and defaulted when empty, as + // IssueRefreshTokenRequest.Resources are: its "aud" is them instead + // of the issuer. Give a refresh token issued for the same grant the + // same Resources. Resources []string // GrantID, if set, names the grant, as IssueRefreshTokenRequest's @@ -59,7 +60,7 @@ type IssueAccessTokenRequest struct { // IssueAccessToken issues an access token for a grant the embedder // served itself (Config.AdditionalGrantTypes), as this server's own // token endpoint would: through Dependencies.AccessTokens, with this -// server's issuer, its audience (req.Resources, or the issuer) and +// server's issuer, its audience (req.Resources, or their default) and // Limits.AccessTokenLifetime, bound to // req.Binding, and carrying the same claims as the access tokens a // refresh token for the grant (IssueRefreshToken) later issues. Use it diff --git a/server/embedder_refresh.go b/server/embedder_refresh.go index d00850d1..62720399 100644 --- a/server/embedder_refresh.go +++ b/server/embedder_refresh.go @@ -56,7 +56,11 @@ type IssueRefreshTokenRequest struct { // access token a refresh issues is restricted to them, or to the // subset the refresh request names. Typically the "resource" // parameters of the token request the embedder served (see - // TokenEndpointRequest.Resources). Empty, the audience is the issuer. + // TokenEndpointRequest.Resources). Empty, the audience is the + // issuer, unless the client is confined by AllowedResources: then + // it's the client's single allowed resource, or, when it has + // several, invalid_target (see + // storage.RegisteredClientConfig.AllowedResources). Resources []string // GrantID, if set, names the grant so RevokeGrant can revoke it, as diff --git a/server/interaction.go b/server/interaction.go index 56150c17..fc235976 100644 --- a/server/interaction.go +++ b/server/interaction.go @@ -230,6 +230,16 @@ type InteractionRequest struct { // of Tax Return TX-12345") instead of a bare scope string. AuthorizationDetails extension.RARValues + // Resources are the RFC 8707 resource indicators the access tokens + // from this authorization would be restricted to — the resource + // servers they can be used at: those the request named, or, for a + // client confined by storage.RegisteredClientConfig.AllowedResources + // that named none, its single allowed one. Nil when the tokens would + // be addressed to the issuer. Already validated; show them where + // the resource servers are several, so the user can tell which a + // token is for, and narrow them with GrantedAuthorization.Resources. + Resources []string + // RequestedClaims is the identity claims the client asked for with // the "claims" parameter. None is released unless the application // approves it in GrantedAuthorization.ApprovedIdentityClaims. diff --git a/server/interaction_encoding.go b/server/interaction_encoding.go index 79d86038..5d65ec2e 100644 --- a/server/interaction_encoding.go +++ b/server/interaction_encoding.go @@ -40,6 +40,7 @@ type encodedInteractionRequest struct { PolicyURI string `json:"policy_uri,omitempty"` TermsOfServiceURI string `json:"tos_uri,omitempty"` AuthorizationDetails extension.RARValues `json:"authorization_details"` + Resources []string `json:"resources,omitempty"` IDTokenClaims []string `json:"id_token_claims,omitempty"` UserInfoClaims []string `json:"userinfo_claims,omitempty"` Extensions extension.Values `json:"extensions"` @@ -57,10 +58,10 @@ type encodedInteractionRequest struct { // you would the rest of a session, never where the client or the user // agent can read it. It carries no integrity protection and needs none for // the grant's sake: CompleteAuthorization checks the granted scope, -// authorization details and identity claims against the request the -// server stored itself, so a modified copy can make a consent screen -// show less than was asked, and the grant built from it smaller, but -// never larger. Store it where only the application can write it all the +// authorization details, resources and identity claims against the +// request the server stored itself, so a modified copy can make a +// consent screen show less than was asked, and the grant built from it +// smaller, but never larger. Store it where only the application can write it all the // same, so what the end user sees is what was asked. func (r InteractionRequest) MarshalText() ([]byte, error) { if r.ClientID == "" { @@ -77,8 +78,8 @@ func (r InteractionRequest) MarshalText() ([]byte, error) { UILocales: r.UILocales, ClaimsLocales: r.ClaimsLocales, Display: r.Display, ClientName: r.ClientDisplay.Name, LogoURI: urlString(r.ClientDisplay.LogoURI), PolicyURI: urlString(r.ClientDisplay.PolicyURI), TermsOfServiceURI: urlString(r.ClientDisplay.TermsOfServiceURI), - AuthorizationDetails: r.AuthorizationDetails, - IDTokenClaims: r.RequestedClaims.IDToken, UserInfoClaims: r.RequestedClaims.UserInfo, + AuthorizationDetails: r.AuthorizationDetails, Resources: r.Resources, + IDTokenClaims: r.RequestedClaims.IDToken, UserInfoClaims: r.RequestedClaims.UserInfo, Extensions: r.Extensions, }) if err != nil { @@ -149,6 +150,7 @@ func ParseInteractionRequest(text string) (InteractionRequest, error) { ClaimsLocales: languageTags(slices.Values(e.ClaimsLocales)), Display: knownDisplay(e.Display), ClientDisplay: display, AuthorizationDetails: e.AuthorizationDetails, + Resources: e.Resources, RequestedClaims: RequestedClaims{IDToken: e.IDTokenClaims, UserInfo: e.UserInfoClaims}, Extensions: e.Extensions, } diff --git a/server/interaction_encoding_test.go b/server/interaction_encoding_test.go index dabdbd26..2132ed22 100644 --- a/server/interaction_encoding_test.go +++ b/server/interaction_encoding_test.go @@ -159,6 +159,7 @@ func TestInteractionRequestFullRoundTrip(t *testing.T) { Prompt: server.Prompt{server.PromptLogin, server.PromptConsent}, ClientDisplay: storage.ClientDisplay{Name: "RP", LogoURI: logo, PolicyURI: policy, TermsOfServiceURI: tos}, RequestedClaims: server.RequestedClaims{IDToken: []string{"email"}, UserInfo: []string{"name"}}, + Resources: []string{"https://rs-a.example", "https://rs-b.example/v1"}, Extensions: extensions, } type session struct { diff --git a/server/resource_indicators.go b/server/resource_indicators.go index 87b14bb6..5414163c 100644 --- a/server/resource_indicators.go +++ b/server/resource_indicators.go @@ -190,10 +190,15 @@ func hexRune(raw []byte, i int) (rune, bool) { // named: each an absolute URI without a fragment (RFC 8707 §2), within // the client's AllowedResources, and listed in Config.Resources or // accepted by the ResourcePolicy. It returns them de-duplicated in the -// order given. Nil in, nil out. +// order given. A request naming none is first given the client's +// default (defaultResources), which is then checked the same way; nil +// when there is none. func (s *Server) checkRequestedResources(ctx context.Context, client storage.RegisteredClient, requested []string) ([]string, *Error) { if len(requested) == 0 { - return nil, nil + var err *Error + if requested, err = s.defaultResources(client); err != nil || len(requested) == 0 { + return nil, err + } } if len(requested) > resourceindicator.MaxPerRequest { return nil, newError(ErrorInvalidTarget, 400, fmt.Sprintf("at most %d resource indicators may be requested", resourceindicator.MaxPerRequest), nil) @@ -231,6 +236,26 @@ func (s *Server) checkRequestedResources(ctx context.Context, client storage.Reg } } +// defaultResources is what a request from client naming no resource is +// for. A client without AllowedResources gets none, so its tokens are +// addressed to the issuer. A client confined by AllowedResources can't +// be given an issuer-audience token, which every resource server that +// accepts the issuer as an audience would take, whatever the list says: +// with one entry, the request is for that one; with more, it must name +// which, so it's invalid_target. With resource indicators disabled, +// there is never a default: the "resource" parameter is ignored, and +// every token is addressed to the issuer. +func (s *Server) defaultResources(client storage.RegisteredClient) ([]string, *Error) { + if !s.resourceIndicatorsEnabled() { + return nil, nil + } + allowed := client.AllowedResources() + if len(allowed) > 1 { + return nil, newError(ErrorInvalidTarget, 400, "a resource is required: this client may request more than one, so must name which", nil) + } + return allowed, nil +} + // narrowResources returns the resources a token request may have its // access token restricted to: requested, when it named any, which must // all be among granted (RFC 8707 §2.2: a token request may narrow the @@ -254,6 +279,18 @@ func narrowResources(granted, requested []string) ([]string, *Error) { return narrowed, nil } +// grantedResources is the resources an approval grants of requested, a +// pending request's: all of them when granted, the application's +// GrantedAuthorization.Resources, names none, and otherwise granted, +// de-duplicated, which must all be among requested. +func grantedResources(requested, granted []string) ([]string, *Error) { + resources, err := narrowResources(requested, granted) + if err != nil { + return nil, newError(ErrorInvalidRequest, 400, "granted resources exceed what was requested", err) + } + return resources, nil +} + // allowedGrantAudience is checkGrantStillAllowed for a token request // from grant asking for scope, followed by narrowedAudience for the // resources it requested. @@ -287,12 +324,17 @@ func (s *Server) usableGrantAudience(ctx context.Context, client storage.Registe // use. When none is — resource indicators turned off, say — it must // authorize again. The refresh token keeps all of the grant's // resources, so one allowed again later can be named again. +// +// A grant without resources, made before the client was confined by +// AllowedResources, gets the client's default as a request naming none +// would (defaultResources): its single allowed resource, or +// invalid_target when it has several, so it must authorize again. func (s *Server) narrowedAudience(ctx context.Context, client storage.RegisteredClient, granted, requested []string) (string, []string, *Error) { resources, err := narrowResources(granted, requested) if err != nil { return "", nil, err } - if _, err := s.checkRequestedResources(ctx, client, resources); err != nil { + if resources, err = s.checkRequestedResources(ctx, client, resources); err != nil { return "", nil, err } audience, audiences := s.accessTokenAudience(resources) diff --git a/server/resource_indicators_consent_test.go b/server/resource_indicators_consent_test.go new file mode 100644 index 00000000..d45f6e96 --- /dev/null +++ b/server/resource_indicators_consent_test.go @@ -0,0 +1,190 @@ +package server_test + +import ( + "context" + "reflect" + "testing" + + "github.com/idfoundry/fapigo/server" + "github.com/idfoundry/fapigo/storage/memstore" +) + +// beginForResources pushes a request naming resources and begins its +// interaction. +func beginForResources(t *testing.T, h harness, resources ...string) server.InteractionRequired { + t.Helper() + pushed, err := pushWithResources(t, h, resources...) + if err != nil { + t.Fatalf("PushAuthorizationRequest: %v", err) + } + action, err := h.server.BeginAuthorization(context.Background(), server.BeginAuthorizationRequest{ + RequestURI: pushed.RequestURI.String(), ClientID: testClientID, + }) + if err != nil { + t.Fatalf("BeginAuthorization: %v", err) + } + required, ok := action.(server.InteractionRequired) + if !ok { + t.Fatalf("action = %T, want server.InteractionRequired", action) + } + return required +} + +// completeGrantingResources approves required as user-1, granting +// resources, and returns the code, or the local error. +func completeGrantingResources(t *testing.T, h harness, required server.InteractionRequired, resources []string) (string, *server.Error) { + t.Helper() + result, err := h.server.CompleteAuthorization(context.Background(), server.CompleteAuthorizationRequest{ + Handle: required.Handle, + Result: authorizedAsGranting(t, h, "user-1", server.GrantedAuthorization{ + Scope: []string{"openid", "accounts", "offline_access"}, Resources: resources, + }), + }) + if err != nil { + t.Fatalf("CompleteAuthorization: %v", err) + } + switch r := result.(type) { + case server.AuthorizationRedirect: + dest := r.Destination().URL() + return dest.Query().Get("code"), nil + case server.AuthorizationLocalError: + return "", r.Error + default: + t.Fatalf("result = %T", result) + return "", nil + } +} + +func authorizedAsGranting(t *testing.T, h harness, subject string, grant server.GrantedAuthorization) server.InteractionResult { + t.Helper() + subjectID, err := server.NewSubjectID(subject) + if err != nil { + t.Fatal(err) + } + authenticated, err := server.NewAuthenticatedSubject(subjectID) + if err != nil { + t.Fatal(err) + } + authCtx, err := server.NewAuthenticationContext(h.now, "", []string{"pwd"}) + if err != nil { + t.Fatal(err) + } + return server.Authorize(authenticated, authCtx, grant) +} + +// The consent step sees the resources a token would be for, the +// client's default among them, and can narrow them to a subset of what +// was requested, never widen them. +func TestInteractionRequestResources(t *testing.T) { + h := withResources(t, newHarness(t, server.ProfileFAPISecurity, true), []string{resourceA, resourceB, resourceC}, nil) + + if got := beginForResources(t, h).Interaction.Resources; got != nil { + t.Errorf("Resources of a request naming none = %q, want nil", got) + } + required := beginForResources(t, h, resourceA, resourceB, resourceA) + if want := []string{resourceA, resourceB}; !reflect.DeepEqual(required.Interaction.Resources, want) { + t.Errorf("Resources = %q, want %q", required.Interaction.Resources, want) + } + restrictClientResources(t, h, resourceC) + if got, want := beginForResources(t, h).Interaction.Resources, []string{resourceC}; !reflect.DeepEqual(got, want) { + t.Errorf("Resources of a confined client's request = %q, want its default %q", got, want) + } +} + +func TestCompleteAuthorizationNarrowsResources(t *testing.T) { + for name, tc := range map[string]struct { + requested, granted []string + want any + }{ + "omitted grants all": {requested: []string{resourceA, resourceB}, want: []any{resourceA, resourceB}}, + "narrowed": {requested: []string{resourceA, resourceB}, granted: []string{resourceB, resourceB}, want: resourceB}, + "none requested": {want: testIssuer}, + } { + t.Run(name, func(t *testing.T) { + h := withResources(t, newHarness(t, server.ProfileFAPISecurity, true), []string{resourceA, resourceB}, nil) + code, localErr := completeGrantingResources(t, h, beginForResources(t, h, tc.requested...), tc.granted) + if localErr != nil { + t.Fatalf("CompleteAuthorization: %v", localErr) + } + result, err := exchangeWithResources(t, h, code) + if err != nil { + t.Fatalf("ExchangeAuthorizationCode: %v", err) + } + wantAudience(t, result, tc.want) + // The grant is what was approved: a refresh can't name what + // the user left out. + if tc.granted != nil { + _, err := refreshWithResources(t, h, result.RefreshToken.Reveal(), resourceA) + wantErrorCode(t, err, server.ErrorInvalidTarget) + } + }) + } +} + +func TestCompleteAuthorizationRefusesUnrequestedResources(t *testing.T) { + for name, tc := range map[string]struct { + requested, granted []string + }{ + "another": {requested: []string{resourceA}, granted: []string{resourceB}}, + "one more": {requested: []string{resourceA}, granted: []string{resourceA, resourceB}}, + "none requested": {granted: []string{resourceA}}, + } { + t.Run(name, func(t *testing.T) { + h := withResources(t, newHarness(t, server.ProfileFAPISecurity, true), []string{resourceA, resourceB}, nil) + _, localErr := completeGrantingResources(t, h, beginForResources(t, h, tc.requested...), tc.granted) + if localErr == nil || localErr.Code() != server.ErrorInvalidRequest { + t.Fatalf("CompleteAuthorization error = %v, want invalid_request", localErr) + } + }) + } +} + +func TestBackchannelInteractionResources(t *testing.T) { + for name, tc := range map[string]struct { + granted []string + want any + wantErr server.ErrorCode + }{ + "omitted grants all": {want: []any{resourceA, resourceB}}, + "narrowed": {granted: []string{resourceA}, want: resourceA}, + "not requested": {granted: []string{resourceC}, wantErr: server.ErrorInvalidRequest}, + } { + t.Run(name, func(t *testing.T) { + h := newHarnessWithBackchannelConfig(t, memstore.NewBackchannelAuthenticationStore(), func(cfg *server.Config) { + cfg.Resources = []string{resourceA, resourceB, resourceC} + }) + required, ok := beginResourceCIBA(t, h, []string{resourceA, resourceB}).(server.BackchannelInteractionRequired) + if !ok { + t.Fatal("BeginBackchannelAuthentication didn't require an interaction") + } + want := []string{resourceA, resourceB} + if !reflect.DeepEqual(required.Interaction.Resources, want) { + t.Errorf("Interaction.Resources = %q, want %q", required.Interaction.Resources, want) + } + looked, err := h.server.LookupBackchannelInteraction(context.Background(), required.Handle) + if err != nil { + t.Fatalf("LookupBackchannelInteraction: %v", err) + } + if !reflect.DeepEqual(looked.Resources, want) { + t.Errorf("looked-up Resources = %q, want %q", looked.Resources, want) + } + + err = h.server.CompleteBackchannelAuthentication(context.Background(), server.CompleteBackchannelAuthenticationRequest{ + Handle: required.Handle, + Result: authorizedAsGranting(t, h, "user-1", server.GrantedAuthorization{Scope: []string{"openid", "accounts"}, Resources: tc.granted}), + }) + if tc.wantErr != "" { + wantErrorCode(t, err, tc.wantErr) + return + } + if err != nil { + t.Fatalf("CompleteBackchannelAuthentication: %v", err) + } + result, err := cibaTokenWithResources(t, h, required.AuthReqID.String()) + if err != nil { + t.Fatalf("ExchangeBackchannelAuthentication: %v", err) + } + wantAudience(t, result, tc.want) + }) + } +} diff --git a/server/resource_indicators_default_test.go b/server/resource_indicators_default_test.go new file mode 100644 index 00000000..841f5c6a --- /dev/null +++ b/server/resource_indicators_default_test.go @@ -0,0 +1,266 @@ +package server_test + +import ( + "context" + "reflect" + "testing" + "time" + + fapi "github.com/idfoundry/fapigo" + "github.com/idfoundry/fapigo/server" + "github.com/idfoundry/fapigo/storage" + "github.com/idfoundry/fapigo/storage/memstore" +) + +// A client confined by AllowedResources to one resource that names none +// gets tokens for that resource, never ones addressed to the issuer: a +// resource server accepting the issuer as an audience (migrating with it +// in AdditionalAudiences, or the issuer's own) would take those whatever +// AllowedResources says. +func TestResourceIndicatorsAllowedResourcesSingleIsDefault(t *testing.T) { + h := withResources(t, newHarness(t, server.ProfileFAPISecurity, true), []string{resourceA, resourceB}, nil) + restrictClientResources(t, h, resourceA) + + _, err := pushWithResources(t, h, resourceB) + wantErrorCode(t, err, server.ErrorInvalidTarget) + + dpopKey := generateKey(t) + result, err := h.server.ExchangeAuthorizationCode(context.Background(), server.AuthorizationCodeExchangeRequest{ + HTTP: server.FormRequest{Parameters: exchangeFormParams(h.clientAssertion(t), grantForResources(t, h), testRedirectURI, testCodeVerifier)}, + DPoPProofs: []string{createDPoPProof(t, dpopKey, h.now)}, + }) + if err != nil { + t.Fatalf("ExchangeAuthorizationCode: %v", err) + } + wantAudience(t, result, resourceA) + for name, verifier := range map[string]func() error{ + "resource B, also accepting the issuer": func() error { + return verifyWithDPoP(t, audienceVerifier(t, h, nil, resourceB, testIssuer), dpopKey, result.AccessToken.Reveal(), h.now) + }, + "the issuer": func() error { + return verifyWithDPoP(t, audienceVerifier(t, h, nil, testIssuer), dpopKey, result.AccessToken.Reveal(), h.now) + }, + } { + if verifier() == nil { + t.Errorf("%s accepted a token of a client confined to resource A", name) + } + } + if err := verifyWithDPoP(t, audienceVerifier(t, h, nil, resourceA), dpopKey, result.AccessToken.Reveal(), h.now); err != nil { + t.Errorf("resource A refused the token: %v", err) + } + + // The grant recorded the default, so it holds even once the client + // may name more. + restrictClientResources(t, h, resourceA, resourceB) + refreshed, err := refreshWithResources(t, h, result.RefreshToken.Reveal()) + if err != nil { + t.Fatalf("refresh: %v", err) + } + wantAudience(t, refreshed, resourceA) + _, err = refreshWithResources(t, h, result.RefreshToken.Reveal(), resourceB) + wantErrorCode(t, err, server.ErrorInvalidTarget) +} + +// A client confined to several resources must name which a request is +// for. +func TestResourceIndicatorsAllowedResourcesSeveralRequireOne(t *testing.T) { + h := withResources(t, newHarness(t, server.ProfileFAPISecurity, true), []string{resourceA, resourceB}, nil) + restrictClientResources(t, h, resourceA, resourceB) + _, err := pushWithResources(t, h) + wantErrorCode(t, err, server.ErrorInvalidTarget) + if _, err := pushWithResources(t, h, resourceB); err != nil { + t.Fatalf("PushAuthorizationRequest naming one: %v", err) + } +} + +// The default is checked as a resource the request named would be: +// against Config.Resources and the ResourcePolicy. +func TestResourceIndicatorsAllowedResourcesDefaultIsChecked(t *testing.T) { + policy := &recordingResourcePolicy{} + h := withResources(t, newHarness(t, server.ProfileFAPISecurity, true), []string{resourceA}, policy) + restrictClientResources(t, h, resourceC) + if _, err := pushWithResources(t, h); err != nil { + t.Fatalf("PushAuthorizationRequest: %v", err) + } + if want := [][]string{{resourceC}}; !reflect.DeepEqual(policy.asked, want) { + t.Fatalf("policy asked %v, want %v", policy.asked, want) + } + + h = withResources(t, h, []string{resourceA}, nil) + _, err := pushWithResources(t, h) + wantErrorCode(t, err, server.ErrorInvalidTarget) +} + +// With resource indicators disabled AllowedResources has no effect: the +// "resource" parameter is ignored, and every token is addressed to the +// issuer. +func TestResourceIndicatorsAllowedResourcesIgnoredWhenDisabled(t *testing.T) { + h := newHarness(t, server.ProfileFAPISecurity, true) + restrictClientResources(t, h, resourceA, resourceB) + result, err := exchangeWithResources(t, h, grantForResources(t, h)) + if err != nil { + t.Fatalf("ExchangeAuthorizationCode: %v", err) + } + wantAudience(t, result, testIssuer) +} + +// A grant made without resources before its client was confined gets +// the default at its token requests too: the single allowed resource, +// or invalid_target when there are several. +func TestResourceIndicatorsAllowedResourcesDefaultForEarlierGrant(t *testing.T) { + h := withResources(t, newHarness(t, server.ProfileFAPISecurity, true), []string{resourceA, resourceB}, nil) + first, second := grantForResources(t, h), grantForResources(t, h) + issued, err := exchangeWithResources(t, h, grantForResources(t, h)) + if err != nil { + t.Fatalf("ExchangeAuthorizationCode: %v", err) + } + wantAudience(t, issued, testIssuer) + + restrictClientResources(t, h, resourceA) + result, err := exchangeWithResources(t, h, first) + if err != nil { + t.Fatalf("ExchangeAuthorizationCode: %v", err) + } + wantAudience(t, result, resourceA) + refreshed, err := refreshWithResources(t, h, issued.RefreshToken.Reveal()) + if err != nil { + t.Fatalf("refresh: %v", err) + } + wantAudience(t, refreshed, resourceA) + + restrictClientResources(t, h, resourceA, resourceB) + _, err = exchangeWithResources(t, h, second) + wantErrorCode(t, err, server.ErrorInvalidTarget) + _, err = refreshWithResources(t, h, issued.RefreshToken.Reveal()) + wantErrorCode(t, err, server.ErrorInvalidTarget) +} + +func TestResourceIndicatorsAllowedResourcesDefaultCIBA(t *testing.T) { + for name, tc := range map[string]struct { + allowed []string + want any + wantBegin server.ErrorCode + }{ + "one": {allowed: []string{resourceA}, want: resourceA}, + "several": {allowed: []string{resourceA, resourceB}, wantBegin: server.ErrorInvalidTarget}, + } { + t.Run(name, func(t *testing.T) { + h := newHarnessWithBackchannelConfig(t, memstore.NewBackchannelAuthenticationStore(), func(cfg *server.Config) { + cfg.Resources = []string{resourceA, resourceB} + }) + registerClientResources(t, h, true, tc.allowed...) + action := beginResourceCIBA(t, h, nil) + if tc.wantBegin != "" { + local, ok := action.(server.BackchannelAuthenticationLocalError) + if !ok || local.Error.Code() != tc.wantBegin { + t.Fatalf("action = %#v, want a local %q error", action, tc.wantBegin) + } + return + } + required, ok := action.(server.BackchannelInteractionRequired) + if !ok { + t.Fatalf("action = %T, want server.BackchannelInteractionRequired", action) + } + if !reflect.DeepEqual(required.Interaction.Resources, []string{resourceA}) { + t.Errorf("Interaction.Resources = %q, want the default", required.Interaction.Resources) + } + result, err := approveAndPollResourceCIBA(t, h, required, nil) + if err != nil { + t.Fatalf("ExchangeBackchannelAuthentication: %v", err) + } + wantAudience(t, result, tc.want) + }) + } +} + +func TestResourceIndicatorsAllowedResourcesDefaultClientCredentials(t *testing.T) { + for name, tc := range map[string]struct { + allowed []string + want any + wantErr server.ErrorCode + }{ + "one": {allowed: []string{resourceB}, want: resourceB}, + "several": {allowed: []string{resourceA, resourceB}, wantErr: server.ErrorInvalidTarget}, + } { + t.Run(name, func(t *testing.T) { + h := withResources(t, newHarnessWithClientCredentialsGrant(t, storage.SenderConstrainDPoP, true), []string{resourceA, resourceB}, nil) + client, err := storage.NewRegisteredClient(storage.RegisteredClientConfig{ + ID: testClientID, + RedirectURIs: []fapi.RegisteredRedirectURI{testRedirectURI}, + ClientAssertionAlgorithm: fapi.ES256, + SenderConstrain: storage.SenderConstrainDPoP, + AllowedScopes: []string{"accounts"}, + AllowsClientCredentialsGrant: true, + AllowedResources: tc.allowed, + }) + if err != nil { + t.Fatalf("NewRegisteredClient: %v", err) + } + h.clients.clients[testClientID] = client + result, err := h.server.RequestClientCredentialsToken(context.Background(), server.ClientCredentialsTokenRequest{ + HTTP: server.FormRequest{Parameters: clientCredentialsFormParams(h.clientAssertion(t), "accounts")}, + DPoPProofs: []string{createDPoPProof(t, generateKey(t), h.now)}, + }) + if tc.wantErr != "" { + wantErrorCode(t, err, tc.wantErr) + return + } + if err != nil { + t.Fatalf("RequestClientCredentialsToken: %v", err) + } + wantAudience(t, result, tc.want) + }) + } +} + +// The embedder's grants get the same default, so neither a token it +// issues nor one a refresh of its refresh token issues is addressed to +// the issuer for a confined client — even when it gives the refresh +// token fewer resources than the access token. +func TestResourceIndicatorsAllowedResourcesDefaultEmbedder(t *testing.T) { + for name, tc := range map[string]struct { + allowed []string + want any + wantErr server.ErrorCode + }{ + "one": {allowed: []string{resourceA}, want: resourceA}, + "several": {allowed: []string{resourceA, resourceB}, wantErr: server.ErrorInvalidTarget}, + } { + t.Run(name, func(t *testing.T) { + attesterKey := generateKey(t) + h := newAttestationHarness(t, time.Now(), attesterKey, server.RegisteredAttesterKeys{}, func(cfg *server.Config, c *storage.RegisteredClientConfig) { + cfg.AdditionalGrantTypes = []string{preAuthorizedCodeGrant} + cfg.Resources = []string{resourceA, resourceB} + c.AllowedScopes = []string{"openid", "accounts", "offline_access"} + c.AllowedResources = tc.allowed + }) + instance := &attestedInstance{t: t, h: h, attesterKey: attesterKey, instanceKey: generateKey(t)} + req := embedderAccessTokenRequest(t, instance, generateKey(t)) + result, err := h.server.IssueAccessToken(context.Background(), req) + refreshToken, refreshErr := h.server.IssueRefreshToken(context.Background(), server.IssueRefreshTokenRequest(req)) + if tc.wantErr != "" { + wantErrorCode(t, err, tc.wantErr) + wantErrorCode(t, refreshErr, tc.wantErr) + return + } + if err != nil || refreshErr != nil { + t.Fatalf("IssueAccessToken = %v, IssueRefreshToken = %v", err, refreshErr) + } + wantAudience(t, result, tc.want) + attestations, pops := instance.headers() + refreshed, err := h.server.RefreshAccessToken(context.Background(), server.RefreshTokenRequest{ + HTTP: server.FormRequest{Parameters: []server.FormParameter{ + formParam("client_id", testClientID.String()), + formParam("grant_type", "refresh_token"), + formParam("refresh_token", refreshToken.Reveal()), + }}, + DPoPProofs: []string{createDPoPProof(t, generateKey(t), h.now)}, + ClientAttestations: attestations, ClientAttestationPoPs: pops, + }) + if err != nil { + t.Fatalf("refresh: %v", err) + } + wantAudience(t, refreshed, tc.want) + }) + } +} diff --git a/server/server.go b/server/server.go index be320927..d96f04bc 100644 --- a/server/server.go +++ b/server/server.go @@ -84,6 +84,7 @@ func New(cfg Config, deps Dependencies) (*Server, error) { automaticClients, err := federation.NewAutomaticClientRepository(deps.Clients, resolver, deps.FederationHTTP, federation.AutomaticRegistrationConfig{ AllowedScopes: cfg.AutomaticRegistration.AllowedScopes, AuthorizationDetailsTypes: cfg.AutomaticRegistration.AuthorizationDetailsTypes, + AllowedResources: cfg.AutomaticRegistration.AllowedResources, MaxCacheAge: cfg.AutomaticRegistration.MaxCacheAge, FailureCacheAge: cfg.AutomaticRegistration.FailureCacheAge, AllowsClientCredentialsGrant: cfg.AutomaticRegistration.AllowsClientCredentialsGrant, diff --git a/server/subject.go b/server/subject.go index 354ada4d..78c2336d 100644 --- a/server/subject.go +++ b/server/subject.go @@ -165,6 +165,17 @@ type GrantedAuthorization struct { // same way a scope not present in the original request is rejected. AuthorizationDetails []json.RawMessage + // Resources narrows the RFC 8707 resource indicators the grant's + // access tokens are restricted to, to those the resource owner + // approved, from the request's own (InteractionRequest.Resources, + // or BackchannelInteractionRequest's). Nil or empty grants all of + // them, so an application that doesn't show resources grants what + // was requested. CompleteAuthorization/ + // CompleteBackchannelAuthentication reject one that wasn't + // requested, the same way they reject a scope that wasn't — so a + // request that named none can't be granted any. + Resources []string + // IDTokenClaims are additional claims the application wants in the // ID token issued for this authorization — e.g. provider-specific // claims describing the authenticated subject or who it is acting diff --git a/serverresource/serverresource.go b/serverresource/serverresource.go index 6ad85e48..c74a6433 100644 --- a/serverresource/serverresource.go +++ b/serverresource/serverresource.go @@ -85,7 +85,9 @@ type Options struct { // addressed to the server's issuer identifier: those whose client // named no resource. Use it while clients move to sending // "resource", then turn it off — until then any token issued without - // a resource indicator is accepted here too. + // a resource indicator is accepted here too. (A client confined by + // storage.RegisteredClientConfig.AllowedResources is never issued + // one, so it can't reach this resource server that way.) AcceptIssuerAudience bool } diff --git a/storage/client_repository.go b/storage/client_repository.go index e042bed3..877edd9b 100644 --- a/storage/client_repository.go +++ b/storage/client_repository.go @@ -426,8 +426,28 @@ type RegisteredClientConfig struct { // (server.Config.Resources and its ResourcePolicy): set, a resource // outside it is invalid_target whatever the server would otherwise // accept. Empty, the client may name any resource the server - // accepts. Each is an absolute URI without a fragment (RFC 8707 - // §2), compared exactly. + // accepts, and a request naming none gets access tokens addressed + // to the issuer. Each is an absolute URI without a fragment (RFC + // 8707 §2), compared exactly. + // + // Set, it also confines the requests that name no resource, since a + // token addressed to the issuer is accepted by every resource server + // that accepts the issuer as an audience, whatever this list says. + // With exactly one entry, such a request is treated as naming that + // one: the grant records it, and its access tokens are addressed to + // it. With more than one, the request must name which it is for, or + // it's invalid_target. This holds wherever a resource can be named — + // at PAR, a CIBA request, client_credentials and the embedder's + // IssueAccessToken and IssueRefreshToken — and for a grant made + // without resources before the list was set, at its code exchange, + // refresh or CIBA token request. So a client confined here gets no + // issuer-audience access token at all: one that also calls the + // UserInfo endpoint, which accepts only those, must list the issuer + // identifier here too (and the server must accept it as a resource, + // in server.Config.Resources, say), and then, having more than one + // entry, name a resource in every request. The list has no effect while the + // server accepts no resource indicators (neither + // server.Config.Resources nor a ResourcePolicy). AllowedResources []string // AllowsClientCredentialsGrant permits this client to use the RFC @@ -1140,6 +1160,21 @@ func (c RegisteredClient) AllowsResource(resource string) bool { return ok } +// AllowedResources returns the RFC 8707 resource indicators this client +// is confined to, sorted, or nil when it registered none — see +// RegisteredClientConfig.AllowedResources. +func (c RegisteredClient) AllowedResources() []string { + if len(c.allowedResources) == 0 { + return nil + } + resources := make([]string, 0, len(c.allowedResources)) + for r := range c.allowedResources { + resources = append(resources, r) + } + slices.Sort(resources) + return resources +} + // AllowsAuthorizationDetailsType reports whether typ is one of the Rich // Authorization Request types this client is registered for — see // RegisteredClientConfig.AuthorizationDetailsTypes. diff --git a/storage/client_repository_test.go b/storage/client_repository_test.go index eaf1c7f3..aa5d22ad 100644 --- a/storage/client_repository_test.go +++ b/storage/client_repository_test.go @@ -683,6 +683,9 @@ func TestNewRegisteredClientAllowedResources(t *testing.T) { if !unrestricted.AllowsResource("https://rs.example") { t.Error("a client registered with no AllowedResources refuses a resource") } + if unrestricted.AllowedResources() != nil { + t.Errorf("AllowedResources() = %q, want nil", unrestricted.AllowedResources()) + } base.AllowedResources = []string{"https://rs.example", "urn:example:rs"} some, err := NewRegisteredClient(base) if err != nil { @@ -691,6 +694,9 @@ func TestNewRegisteredClientAllowedResources(t *testing.T) { if !some.AllowsResource("urn:example:rs") || some.AllowsResource("https://other.example") || some.AllowsResource("https://rs.example/") { t.Error("AllowsResource doesn't follow the registered list exactly") } + if got, want := some.AllowedResources(), []string{"https://rs.example", "urn:example:rs"}; !slices.Equal(got, want) { + t.Errorf("AllowedResources() = %q, want %q", got, want) + } for _, bad := range []string{"", "/relative", "https://rs.example#frag"} { base.AllowedResources = []string{"https://rs.example", bad} if _, err := NewRegisteredClient(base); err == nil {