Support multi-tenancy using the path component for issuer

[frederikz]

Hi,

The openid-configuration endpoint is always available under /.well-known/openid-configuration . For an issuer https://example.com/issuer1 I would have expected it to be availabe under /issuer1/.well-known/openid-configuration according to https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationRequest .

Related gh-1435 gh-663

[jgrandja]

@frederikz Are you having an issue with your current set up? Are you trying to set up multi-tenancy and running into an issue?

[frederikz]

Yes, in a multitenant setup I have multiple issuers for one host. I saw in the spec that you can add a path to the issuer which is then also expected in the configuration endpoint. But at the moment OidcProviderConfigurationEndpointFilter uses a request matcher with a fixed path not according to the issuer path and you can't configure it to use your own request matcher that e.g. uses /*/.well-known/openid-configuration. I can also add it to gh-663 but I don't think it is only missing documentation - at least the way how I implement multi-tenancy.
I classified it as a bug as e.g. the OidcProviderConfigurationTests configures an issuer https://example.com/issuer1 and then expects the configuraion endpoint to be /.well-known/openid-configuration but shouldn't it even in a single tenant setup be /issuer1/.well-known/openid-configuration?

[jgrandja]

@frederikz Thanks for the details. We'll look at this shortly.

[FoxNeo]

This is the same issue I've had trying to have an OIDC Discovery endpoint on a different path /
As mentioned in #1409
The Builder could be modified to override OidcProviderConfigurationEndpointFilter like:

	public static Builder builder() {
		return new Builder()
		        .discoveryEndpoint("/metadata/.well-known/openid-configuration")
			.authorizationEndpoint("/oauth2/authorize")
			.deviceAuthorizationEndpoint("/oauth2/device_authorization")
			.deviceVerificationEndpoint("/oauth2/device_verification")
			.tokenEndpoint("/oauth2/token")
			.tokenIntrospectionEndpoint("/oauth2/introspect")
			.tokenRevocationEndpoint("/oauth2/revoke")
			.jwkSetEndpoint("/oauth2/jwks")
			.oidcLogoutEndpoint("/connect/logout")
			.oidcUserInfoEndpoint("/userinfo")
			.oidcClientRegistrationEndpoint("/connect/register");
	}

Or add a Builder with Prefix

            public static Builder builderWithPrefix(@NonNull String prefix) {
		return new Builder()
				.metadataPrefix(prefix + "/.well-known/openid-configuration")
				.authorizationEndpoint(prefix + "/oauth2/authorize")
				.tokenEndpoint(prefix +"/oauth2/token")
				.jwkSetEndpoint(prefix +"/oauth2/jwks")
				.tokenRevocationEndpoint(prefix +"/oauth2/revoke")
				.tokenIntrospectionEndpoint(prefix +"/oauth2/introspect")
				.oidcClientRegistrationEndpoint(prefix +"/connect/register")
				.oidcUserInfoEndpoint(prefix + "/userinfo");
	}

[jgrandja]

@frederikz

OidcProviderConfigurationTests configures an issuer https://example.com/issuer1 and then expects the configuraion endpoint to be /.well-known/openid-configuration but shouldn't it even in a single tenant setup be /issuer1/.well-known/openid-configuration?

Thanks for pointing this out. Indeed, the tests were incorrect and I just pushed the fixes for the tests related to OidcProviderConfigurationEndpointFilter and OAuth2AuthorizationServerMetadataEndpointFilter.

@frederikz @FoxNeo

Regarding 4.1. OpenID Provider Configuration Request, specifically,

Using path components enables supporting multiple issuers per host. This is required in some multi-tenant hosting configurations. This use of .well-known is for supporting multiple issuers per host; unlike its use in RFC 5785 [RFC5785], it does not provide general information about the host.

Can you provide more details on how you implemented or are planning on implementing multi-tenancy in Spring Authorization Server. For example, are you looking to support multiple issuers within the same JVM instance of Spring Authorization Server OR will you dedicate a JVM instance per issuer OR will you isolate issuer per web application context, etc...

[frederikz]

Our setup is a single JVM instance with a single web application (using spring-authorization-server). Inside this single web application we can have multiple tenants. Like you can have session or request scoped beans we have kind of tenant scoped beans and this is how we isolate multiple tenants inside a single web application. A tenant is identified by an path segment in the URL. In other Spring security components like Spring SAML you can in some way configure the request matcher that is used by filters so there is no issue implementing such a scenario (see also gh-663).

I saw that you fixed the tests by changing the issuer from https://example.com/issuer1 to https://example.com . But shouldn't it be the implementation that gets fixed and when presented with an issuer https://example.com/issuer1 does the correct thing? Even in a single tenant setup you allow the developer to configure an issuer https://example.com/issuer1 (for what ever reason he wants to do that) which is a valid issuer but doesn't work. Shouldn't the fix be that OidcProviderConfigurationEndpointFilter doesn't use a request matcher with a static endpoint /.well-known/openid-configuration but uses the configured issuer to create the request matcher and in case of https://example.com/issuer1 creates a request matcher /issuer1/.well-known/openid-configuration ? This should be the default but to be more flexible you should also let the developer specify its own requestMatcher for the filter in some way.
So e.g. in my multi-tenancy setup a single AuthorizationServerSettings Bean doesn't make sense. If OidcProviderConfigurationEndpointFilter would take the issuer of AuthorizationServerSettings into account for the request matcher I could then of course configure an issuer like https://example.com/* or maybe https://example.com/{tenantId} in AuthorizationServerSettings to have the request matcher match multiple path but configuring /* (where /.well-known/openid-configuration gets appended automatically) or /*/.well-known/openid-configuration directly as the path for the configuration endpoint could be cleaner.

I have a setup that works for me with the patch I shared in gh-663 and a OidcProviderConfigurationEndpointFilter, OidcProviderConfigurationEndpointConfigurer modification that uses the path from the issuer of AuthorizationServerSettings to create the requestMatcher for OidcProviderConfigurationEndpointFilter. I haven't shared that yet as I wanted to wait for the discussion of gh-663 if maybe AuthorizationServerSettings would be made optional if we have a AuthorizationServerContextResolver and then you clould alternatively configure the endpoint urls directly on the configuration objects.

[FoxNeo]

I'm planning to implement a multi-tenancy Spring Authorization Server within the same Spring Boot application on the same host. Like @frederikz comment above.

    https://example.com/authenticator/service1/.well-known/openid-configuration
    https://example.com/authenticator/service2/.well-known/openid-configuration
    https://example.com/authenticator/service3/.well-known/openid-configuration

[jgrandja]

@frederikz

I saw that you fixed the tests by changing the issuer from https://example.com/issuer1 to https://example.com/ . But shouldn't it be the implementation that gets fixed and when presented with an issuer https://example.com/issuer1 does the correct thing?

The tests were fixed to ensure it aligns with single tenant support. The current implementation does not support multi-tenancy. However, we will prioritize this for the 1.3 release. The following will be implemented as an enhancement:

Using path components enables supporting multiple issuers per host. This is required in some multi-tenant hosting configurations. This use of .well-known is for supporting multiple issuers per host; unlike its use in RFC 5785 [RFC5785], it does not provide general information about the host.

Regarding your comment:

Even in a single tenant setup you allow the developer to configure an issuer https://example.com/issuer1 (for what ever reason he wants to do that) which is a valid issuer but doesn't work.

Let me look at that. I might need to fix more tests.

I have a setup that works for me with the patch I shared in #663 and a OidcProviderConfigurationEndpointFilter, OidcProviderConfigurationEndpointConfigurer modification that uses the path from the issuer of AuthorizationServerSettings to create the requestMatcher for OidcProviderConfigurationEndpointFilter.

All of this information has been super helpful. I also want to look at your code modifications in your branch to see exactly what would work for you. If you can update that branch with all the changes you need that would be great.

I'm planning on having multi-tenancy capability available in the 1.3.0-M1 release, which is expected in Jan. However, it most likely will be merged before Jan in a snapshot so you can test it out.

This will be a bit of a tricky enhancement with the current setup so I want to make sure I get it right the first time. The more info I can get from you and you testing it out along the way will help a lot.

[frederikz]

I have pushed a few more commits to my branch.

frederikz@39eb574 showing how I think issuers with path should be handled.

frederikz@079054b frederikz@3e59aba just an unrelated thing that bothered me but doesn't cause problems: duplicate request matchers in Configurer and Filter classes (e.g. OidcProviderConfigurationEndpointConfigurer and OidcProviderConfigurationEndpointFilter) that are independently created but must be created in the same way so that requests are processed.

frederikz@fcc3a66 I wanted to make AuthorizationServerSettings optional and directly configure endpoints to be used by filters on the Configurer objects. The reason is that configuring an endpoint like /oauth2/{tenantName}/authorize on the object seemed strange and that AuthorizationServerSettings is also not only used as a configuration object to configure the filter request matchers on startup but is also used in AuthorizationServerContext as kind of a runtime object which in my multi-tenant setup will have different values. Even in the current code you can have a dynamic issuer and as a result the AuthorizationServerContext has a seperate getIssuer method which doesn't feel right as you need to know that you should call this method and not getAuthorizationServerSettings().getIssuer() .
I now accepted AuthorizationServerSettings as a configuration object but in AuthorizationServerContext I removed it to make it more clear that during request handling you no longer deal with the original AuthorizationServerSettings object and to remove the two ways getIssuer(), getAuthorizationServerSettings().getIssuer() of getting an issuer.

[jgrandja]

Thanks for updating your branch @frederikz ! I will look at this soon.

[jgrandja]

@frederikz Regarding ...

Even in a single tenant setup you allow the developer to configure an issuer https://example.com/issuer1 (for what ever reason he wants to do that) which is a valid issuer but doesn't work.

This has been fixed in gh-1435

Also, I changed the issue title to reflect that this is an enhancement request and I scheduled it for 1.3.x.

[jgrandja]

@frederikz @FoxNeo This is now merged via 168077b.

FYI, I updated the javadoc for AuthorizationServerContext.getIssuer() with examples and also to make it more clear on what the expected return value should be from this API.

spring-authorization-server/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/context/AuthorizationServerContext.java

Line 48 in 168077b

String getIssuer();