-
-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BackChannelLogoutController injects ServerOAuth2AuthorizedClientRepository instead of SpringAddonsServerOAuth2AuthorizedClientRepository #140
Comments
The reason why All I could do here is adding an interface for this Meanwhile, if you disable back-channel logout, the problem should be gone. |
The main issue here is that in the case of a back-channel logout, the request is initiated by the authorization server and as a consequence is not part of the user session. This means that you will have a null Here is what I thought of to use database instead: Declare an interface as follow: public interface MultiTenantServerOAuth2AuthorizedClientRepository extends ServerOAuth2AuthorizedClientRepository {
/**
* Removes an authorized client and returns a list of sessions to invalidate (those for which the current user has no more authorized client after this one
* was removed)
*
* @param issuer OP issuer URI
* @param principalName current user name for this OP
* @return the list of user sessions for which this authorized client was the last one (and should be terminated)
*/
Flux<WebSession> removeAuthorizedClients(String issuer, String principalName);
} And change
Instead of the two beans you expose above, you would provide something like that (does about the same as @Component
static class MyMultiTenantServerOAuth2AuthorizedClientRepository implements MultiTenantServerOAuth2AuthorizedClientRepository {
private final ReactiveClientRegistrationRepository clientRegistrationRepository;
private final DatabaseClient databaseClient;
private final AuthenticatedPrincipalServerOAuth2AuthorizedClientRepository delegate;
public MyMultiTenantServerOAuth2AuthorizedClientRepository(
DatabaseClient databaseClient,
ReactiveClientRegistrationRepository clientRegistrationRepository) {
this.clientRegistrationRepository = clientRegistrationRepository;
this.databaseClient = databaseClient;
this.delegate = new AuthenticatedPrincipalServerOAuth2AuthorizedClientRepository(new R2dbcReactiveOAuth2AuthorizedClientService(databaseClient, clientRegistrationRepository));
}
@Override
public <
T extends OAuth2AuthorizedClient>
Mono<T>
loadAuthorizedClient(String clientRegistrationId, Authentication principal, ServerWebExchange exchange) {
return delegate.loadAuthorizedClient(clientRegistrationId, principal, exchange);
}
@Override
public Mono<Void> saveAuthorizedClient(OAuth2AuthorizedClient authorizedClient, Authentication principal, ServerWebExchange exchange) {
final var issuer = authorizedClient.getClientRegistration().getProviderDetails().getIssuerUri();
final var principalName = principal.getName();
databaseClient.sql(...); // Save in DB that this principalName is authorized by this issuer with that authorized client ID
return delegate.saveAuthorizedClient(authorizedClient, principal, exchange);
}
@Override
public Mono<Void> removeAuthorizedClient(String clientRegistrationId, Authentication principal, ServerWebExchange exchange) {
return clientRegistrationRepository.findByRegistrationId(clientRegistrationId).flatMap(clientRegistration -> {
final var issuer = clientRegistration.getProviderDetails().getIssuerUri();
final var principalName = principal.getName();
databaseClient.sql(...); // Remove from DB that this principalName is authorized by this issuer
return delegate.removeAuthorizedClient(clientRegistrationId, principal, exchange);
});
}
@Override
public Flux<WebSession> removeAuthorizedClients(String issuer, String principalName) {
// fetch from DB the authorized client ID for this principalName and that issuer
// and then flatMap delegate.removeAuthorizedClient(clientRegistrationId, principal, null)
databaseClient.sql(...).flatMap(clientRegistrationId -> {
delegate.removeAuthorizedClient(clientRegistrationId, new StubAuthentication(principalName), null);
});
}
}
static class StubAuthentication implements Authentication {
private static final long serialVersionUID = -8037754046522288880L;
private final String name;
StubAuthentication(String name) {
this.name = name;
}
@Override
public String getName() {
return name;
}
@Override
public Collection<? extends GrantedAuthority> getAuthorities() {
return List.of();
}
@Override
public Object getCredentials() {
return name;
}
@Override
public Object getDetails() {
return name;
}
@Override
public Object getPrincipal() {
return name;
}
@Override
public boolean isAuthenticated() {
return true;
}
@Override
public void setAuthenticated(boolean isAuthenticated) throws IllegalArgumentException {
}
} What do you think? |
I like what you posted above. If Thank you for your quick response. much appreciate. |
The issue is not stale. I'm still experimenting with cleaner solutions that what I exposed just before. I think I can provide with better integration with custom |
Thank you so much |
I think I'm going to remove the Back-Channel Logout support from this lib because of spring-projects/spring-security#12570 which should bring it in Spring Security (it might need some hacking because of what I report in that thread about multi-tenancy and session termination, but this is another story). However, what is reported is still worth some changes: instrumentation to bring OAuth2 clients multi-tenancy would be much more transparent for spring-addons users if this was achieved with AOP rather than with specific implementations of authorized client repositories. What I call "client multi-tenancy" is allowing a user to be authenticated with more than just on OpenID Provider at a time and being able to retrieve the right authorized client along with the right principal name: username is likely to change from an authorization server to another, specifically when the default claim (subject) is used. This is a requirement to send requests to different groups of API (expecting different issuers), each with the right access token. |
Use AOP instead of custom authorized-clients repo Remove Back-Channel Logout support Make OAuth2 clients multi-tenancy support optional Use AOP to support OAuth2 clients multi-tenancy (instead of SpringAddonsAuthorizedClientRepository)
@kwonglau you can see there the details of of what changes. I removed Back-Channel Logout support is removed from spring-addons. Getting it to work properly on reactive applications with sessions instrumented via AOP is actually quite complicated. There is a PR with a lot of activity on Spring Security to bring this feature in the framework and I don't want to provide with features already provided by the core framework. So let's wait until the feature is released, and maybe I'll add here some behavior around client multi-tenancy. |
gh-140 : ul (CONFIGURATION) Use AOP instead of custom authorized-clients repo Remove Back-Channel Logout support Make OAuth2 clients multi-tenancy support optional Use AOP to support OAuth2 clients multi-tenancy (instead of SpringAddonsAuthorizedClientRepository)
- Remove Back-Channel Logout support - Make OAuth2 clients multi-tenancy support optional (disabled by default) - Use AOP to support OAuth2 clients multi-tenancy (instead of `SpringAddonsAuthorizedClientRepository`)
@kwonglau the changes I referred to in my last comment are released with |
Is your feature request related to a problem? Please describe.
I would like to store the oauth2_authorized_client using R2dbcReactiveOAuth2AuthorizedClientService offered by Spring.
when customizing ServerOAuth2AuthorizedClientRepository bean, ReactiveSpringAddonsBackChannelLogoutBeans#BackChannelLogoutController component creation is throwing error because it depends on SpringAddonsServerOAuth2AuthorizedClientRepository instead of ServerOAuth2AuthorizedClientRepository
Describe the solution you'd like
I would like the spring-addons keeps the current backchannel logout functionality and be able to store the oauth2_authorized_client to the db as well.
Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
Additional context
Add any other context or screenshots about the feature request here.
The text was updated successfully, but these errors were encountered: