Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom claim converter in a microservice with oauth2 #12609

Merged
merged 5 commits into from
Jan 11, 2021

Conversation

Falydoor
Copy link
Contributor

@Falydoor Falydoor commented Sep 30, 2020

Add a custom claim converter to fetch the oauth2 userinfo endpoint and add custom claims. This only apply to a microservice using oauth2.

Fix #10975 @mraible (can this PR be merged after September for hacktoberfest 馃憖)


Please make sure the below checklist is followed for Pull Requests.

@mraible
Copy link
Contributor

mraible commented Sep 30, 2020

Thanks, @Falydoor! I believe you need to add a converter for WebFlux too.

Be sure to add [webflux] in your commit message when you add the implementation and test so the CI tests run.

@Falydoor
Copy link
Contributor Author

Falydoor commented Sep 30, 2020

Unfortunately I can't have ((ServletRequestAttributes) RequestContextHolder.getRequestAttributes()).getRequest() to work with WebFlux 馃槩 so might be something to see for later.

@mraible
Copy link
Contributor

mraible commented Sep 30, 2020

@Falydoor I might be able to help, but might not be for a few days. @cbornet is our resident Spring WebFlux expert.

@cbornet
Copy link
Member

cbornet commented Oct 1, 2020

RequestContextHolder is thread local so it can't be used in the reactive stack. I think the solution here is to use a ServerWebExchangeContextFilter and then get the ServerWebExchange from the subscriberContext.
See this example

@Falydoor
Copy link
Contributor Author

Falydoor commented Oct 2, 2020

@cbornet I played with the ServerWebExchangeContextFilter but for some reason the subscriberContext is always empty in the convert method of the CustomClaimConverter.

Also even if the context contained ServerWebExchange, I don't think it would be possible to have it working because the convert method must return Map<String, Object> instead of Mono<Map<String, Object>> (which mean that .block() must be called when getting ServerWebExchange).

@mraible
Copy link
Contributor

mraible commented Oct 7, 2020

@jgrandja Do you have any tips for how to implement a CustomClaimConverter for Spring WebFlux?

@jgrandja
Copy link

jgrandja commented Oct 8, 2020

@mraible A custom claim converter may be configured via OidcUserService.setClaimTypeConverterFactory() (Servlet) or OidcReactiveOAuth2UserService.setClaimTypeConverterFactory() (WebFlux). Both provide a default converter so take a look at that and customize to your needs.

@pascalgrimaud pascalgrimaud changed the base branch from master to main October 11, 2020 20:23
@mraible
Copy link
Contributor

mraible commented Oct 27, 2020

@Falydoor Does @jgrandja's advice help?

A custom claim converter may be configured via OidcUserService.setClaimTypeConverterFactory() (Servlet) or OidcReactiveOAuth2UserService.setClaimTypeConverterFactory() (WebFlux). Both provide a default converter so take a look at that and customize to your needs.

@Falydoor
Copy link
Contributor Author

@mraible I tried using OidcReactiveOAuth2UserService.setClaimTypeConverterFactory() but I wasn't able to make it work unfortunately. The method CustomClaimConverter.convert() is never called for some reason 馃槵 .

@mraible
Copy link
Contributor

mraible commented Oct 27, 2020

@Falydoor Is the code for reactive in this PR? If so, I can try and see if I can figure it out.

@Falydoor
Copy link
Contributor Author

@mraible I created a repo with the code, check the commit (I put some comments to help you) Falydoor/jhipster-reactive-oauth@c44ff7b

@mraible
Copy link
Contributor

mraible commented Oct 28, 2020

@Falydoor I forked your project and there's no src/main/docker/keycloak.yml file. Any idea why this wasn't created?

@Falydoor
Copy link
Contributor Author

@mraible I'm using Okta and the keycloak file only comes with the gateway.

@mraible
Copy link
Contributor

mraible commented Oct 28, 2020

@Falydoor As far as I can tell, the oidcUserService() is not used when the server is acting as a resource server. This comment from @jgrandja makes me think it'll only be used when you've logged in with oauth2Login().

@Falydoor
Copy link
Contributor Author

@mraible
Copy link
Contributor

mraible commented Oct 30, 2020

Yes, I did try it with that line uncommented and it does call the converter.

@Falydoor
Copy link
Contributor Author

But then in the converter the subscriberContext is empty which means that it can't retrieve the token. Maybe there is another way to get the token?

I also tried using ReactiveSecurityContextHolder.getContext().map(SecurityContext::getAuthentication) but it's empty.

serverMicroserviceAndOauth: [
{
condition: generator =>
!generator.reactive && generator.authenticationType === 'oauth2' && generator.applicationType === 'microservice',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conditional (and others) should include applicationType === 'monolith' and gateway too. We configure these as resource servers by default for Ionic and React Native apps.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Falydoor I believe this comment still needs to be addressed.

@mraible
Copy link
Contributor

mraible commented Oct 31, 2020

Using this guide, I tried to register a WebClient:

@Bean
WebClient webClient(ReactiveClientRegistrationRepository clientRegistrations,
                    ServerOAuth2AuthorizedClientRepository authorizedClients) {
    ServerOAuth2AuthorizedClientExchangeFilterFunction oauth =
        new ServerOAuth2AuthorizedClientExchangeFilterFunction(
            clientRegistrations,
            authorizedClients);
    oauth.setDefaultOAuth2AuthorizedClient(true);
    oauth.setDefaultClientRegistrationId("oidc");
    return WebClient.builder()
        .filter(oauth)
        .build();
}

Then, I used the following line to create the CustomClaimConverter:

@Bean
ReactiveJwtDecoder jwtDecoder(ReactiveClientRegistrationRepository clientRegistrationRepository, WebClient webClient) {
    ...
    jwtDecoder.setClaimSetConverter(new CustomClaimConverter(clientRegistrationRepository.findByRegistrationId("oidc").block(), webClient));

    return jwtDecoder;
}

Then, I modified the CustomClaimConverter to be the following:

package com.mycompany.myapp.security;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.core.convert.converter.Converter;
import org.springframework.security.oauth2.client.OAuth2AuthorizedClient;
import org.springframework.security.oauth2.client.registration.ClientRegistration;
import org.springframework.security.oauth2.jwt.MappedJwtClaimSetConverter;
import org.springframework.web.reactive.function.client.WebClient;
import reactor.core.publisher.Mono;

import java.util.Collections;
import java.util.Map;

public class CustomClaimConverter implements Converter<Map<String, Object>, Map<String, Object>> {
    private final Logger log = LoggerFactory.getLogger(CustomClaimConverter.class);

    private final MappedJwtClaimSetConverter delegate = MappedJwtClaimSetConverter.withDefaults(Collections.emptyMap());

    private final WebClient webClient;
    private final ClientRegistration registration;

    public CustomClaimConverter(ClientRegistration registration, WebClient webClient) {
        this.webClient = webClient;
        this.registration = registration;
    }

    public Map<String, Object> convert(Map<String, Object> claims) {
        Map<String, Object> convertedClaims = this.delegate.convert(claims);
        log.debug("convertedClaims : {} ", convertedClaims);

        Mono.subscriberContext()
            .subscribe(context -> {
                log.debug("CONTEXT SIZE : {}", context.size());
            });

        ClientRegistration.ProviderDetails.UserInfoEndpoint userInfoEndpoint = registration.getProviderDetails().getUserInfoEndpoint();
        log.debug("USER INFO : {}", userInfoEndpoint);

        Mono<String> retrievedResource = webClient.get()
            .uri(userInfoEndpoint.getUri())
            .retrieve()
            .bodyToMono(String.class);

        retrievedResource.subscribe(System.out::print);

        return convertedClaims;
    }
}

I get the following error:

reactor.core.Exceptions$ErrorCallbackNotImplemented: java.lang.IllegalArgumentException: serverWebExchange cannot be null
Caused by: java.lang.IllegalArgumentException: serverWebExchange cannot be null
        at org.springframework.security.oauth2.client.web.DefaultReactiveOAuth2AuthorizedClientManager.lambda$loadAuthorizedClient$9(DefaultReactiveOAuth2AuthorizedClientManager.java:102)

If I modify the WebClient bean to remove setDefaultClientRegistrationId("oidc"), it results in:

401 Unauthorized from GET https://dev-162929.okta.com/oauth2/default/v1/userinfo

It does make the call, but maybe it doesn't have an access token associated with it? I'm not sure.

I thought it might be possible to get the access token like we get the ID token in LogoutResource, but I'm not sure how that might work.

@AuthenticationPrincipal(expression = "idToken") OidcIdToken idToken

Maybe it's possible to get the Authentication from ReactiveSecurityContextHolder (like this), and then cast it to a Jwt, which has a getTokenValue() method.

@mraible
Copy link
Contributor

mraible commented Nov 12, 2020

I spent several hours working on this yesterday but didn't succeed. Maybe documenting what I tried will help.

First of all, I found this concise blog post that makes it look easy to make a WebClient request with an access token. However, it uses client_credentials, whereas JHipster uses auth code flow. https://medium.com/@asce4s/oauth2-with-spring-webclient-761d16f89cdd

I found a Spring Security example that seems close to what we want. However, I need something that works without oauth2Login() since that鈥檚 not part of JHipster when it鈥檚 a resource server. https://github.com/spring-projects/spring-security/tree/master/samples/boot/oauth2webclient-webflux

Error I'm getting:

2020-11-11 16:08:46.649  INFO 36332 --- [ctor-http-nio-3] c.m.myapp.security.CustomClaimConverter  : userinfo endpoint https://dev-162929.okta.com/oauth2/default/v1/userinfo
2020-11-11 16:08:46.656 ERROR 36332 --- [ctor-http-nio-3] reactor.core.publisher.Operators         : Operator called default onErrorDropped
reactor.core.Exceptions$ErrorCallbackNotImplemented: java.lang.IllegalArgumentException: serverWebExchange cannot be null
Caused by: java.lang.IllegalArgumentException: serverWebExchange cannot be null
        at org.springframework.security.oauth2.client.web.DefaultReactiveOAuth2AuthorizedClientManager.lambda$loadAuthorizedClient$9(DefaultReactiveOAuth2AuthorizedClientManager.java:102)
        Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException: 
Assembly trace from producer [reactor.core.publisher.MonoErrorSupplied] :

Adding this at the top of the security config doesn鈥檛 help:

.addFilterAt(new ServerWebExchangeContextFilter(), SecurityWebFiltersOrder.FIRST)

To test everything, I'm using Ionic for JHipster since it surfaced the problem for me. I re-generated @Falydoor's project as a monolith running on 8080 so it would work with Ionic4J. You can see the difference between the projects at Falydoor/jhipster-reactive-oauth@main...mraible:userinfo.

Search for "SecurityConfiguration.java" and "CustomerClaimConverter.java" to see the changes I tried.

@mraible
Copy link
Contributor

mraible commented Dec 29, 2020

@Falydoor Maybe we should just merge this and open a new issue for reactive?

@Falydoor
Copy link
Contributor Author

@mraible yes I agree. You did more research than me for the reactive support and it is trickier than what we thought...

NimbusJwtDecoder jwtDecoder = (NimbusJwtDecoder) JwtDecoders.fromOidcIssuerLocation(issuerUri);

OAuth2TokenValidator<Jwt> audienceValidator = new AudienceValidator(jHipsterProperties.getSecurity().getOauth2().getAudience());
OAuth2TokenValidator<Jwt> withIssuer = JwtValidators.createDefaultWithIssuer(issuerUri);
OAuth2TokenValidator<Jwt> withAudience = new DelegatingOAuth2TokenValidator<>(withIssuer, audienceValidator);

jwtDecoder.setJwtValidator(withAudience);
<%_ if (authenticationType === 'oauth2' && applicationType === 'microservice') { _%>
jwtDecoder.setClaimSetConverter(new CustomClaimConverter(clientRegistrationRepository.findByRegistrationId("oidc"), new RestTemplate()));
<%_ } _%>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using new RestTemplate causes so much pains in real world (for example, when your app is behind an outbound proxy).
please prefer RestTemplate rt = restTemplateBuilder.build();

import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.when;

@SpringBootTest(classes = {<%= mainClass %>.class, TestSecurityConfiguration.class})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use new @IntegrationTest plz, otherwise, it's broken for some config like Redis etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! I opened #13462

@pascalgrimaud pascalgrimaud mentioned this pull request Jan 13, 2021
5 tasks
@pascalgrimaud pascalgrimaud added this to the 7.0.0-beta.1 milestone Jan 16, 2021
sendilkumarn added a commit to jhipster/jhipster-kotlin that referenced this pull request Feb 27, 2021
@mraible
Copy link
Contributor

mraible commented Mar 12, 2021

After thinking about this a bit more, I'm not sure we need a /userinfo lookup in a microservice. I think it's only needed in a monolith or gateway - because that's where the access token will come in from the client. Thoughts?

sendilkumarn added a commit to jhipster/jhipster-kotlin that referenced this pull request Apr 18, 2021
sendilkumarn added a commit to jhipster/jhipster-kotlin that referenced this pull request Apr 18, 2021
* finished first cut for 7.0.0 release

* feat: upgrade the entity generator to v7 beta

* feat: upgrade server generator to v7

* feat: upgrader server generator to v7

* feat: update configuration to v7

* feat: migrate web/rest to v7

* feat: upgrade security to v7.0

* migrate repository tov v7 beta

* migrate service to v7 beta

* feat: upgrade config test directory to version 7

* migrate cucumber to v7

* feat: migrate gateway to v7

* feat: migrate repository test to v7

* feat: migrate security/jwt to v7

* feat: migrate cookie token extractor to v7

* feat: migrate security to v7

* feat: migrate mapper to v7

* feat: migrate service and test service to v7

* feat: migrate rest resources to v7

* feat: migrate Cassandra config test utils to v7

* fix: type configuration fixes

* feat: migrate test and test integration scripts to v7

* migrate workflows to v7 step 1

* migrate tests and integration scripts

* feat: migrate files to v7

* fix: use proper generic import

* feat: upgrade other files to v7

* feat: update expected-files to v7

* feat: update app spec to v7

* feat: test updates and remove import static

* add missing jhi_clone path

* fix: lint fixes

* fix ejs lint error

* fix: remove duplicate sonar analysis

* fix template lint issues

* feat: fix build issues

* fix: entity server unit test

* fix: failing test case because of missing prettier transform

* remove withAudit configuration

* feat: add postWriting in server configuration

* rename primaryKeyType to primaryKey.type

* rename JHipsterBlockHoundIntegration

* compilation fixes

* fix: some more bugs

* fix: add fun modifier for functions

* fix: remove extra braces and use proper field accessor

* fix: add a line break between package and import

* fix: add secondary constructor for the UserDTO

* fix: use proper variable declaration in patch_tempalte

* fix di and type selection

* fix: add variable name

* fix: migrate entity generator to v7 and few other fixes

* fix: make converters as an object

* fix: remove trailing commas

* fix: remove return type reference

* fix: use kotlin class reference

* fix: infer type

* fix build file issue

* fix: template string

* fix: mongodb kafka app generation

* fix: enity configuration and lint issues

* remove couchbase and fix kapt

* fix: react ci build issues

* lint fix

* fix generated services and entities

* fix react builds and remove couchbase builds

* make authorities proper

* Upgrade kotlin to latest version

* fix test failures in react default application

* lint fixes and failing unit test fixes

* fix: add a proper script folder

* fix: update cloned path for angular ci

* revert change

* fix more type issues

* Update EntityResourceIT.kt.ejs

* fix react ci tests

* Update PublicUserResource.kt.ejs

* webflux fixes

* feat/webflux fixes for v7 migration

* few more fixes

* fix cucumber tests

* replace proper cassandra version variable

* fix: workflow and kotlinify

* feat: add support for --pk-type

part of jhipster/generator-jhipster#13296

* Add support to `@MapstructExpression`

Refer jhipster/generator-jhipster#13195

* fix: Replace deprecated `StringUtils.isEmpty()` by `ObjectUtils.isEmpty()` and cover its usage with tests

Refer jhipster/generator-jhipster#13369

* chore: Change from 'idx in' to 'item of' for fields and relationships at server templates.

Refer jhipster/generator-jhipster#13382

* fix: Neo4j relation rework

Refer jhipster/generator-jhipster#13106

* fix: Swagger UI for reactive applications

Refer jhipster/generator-jhipster#13409

* fix: sonar code smells

Refer jhipster/generator-jhipster#13408

* chore: typo updates

* feat: Add custom claim converter in a microservice with oauth2

Refer jhipster/generator-jhipster#12609

* fix: import fixes for custom claim converter test

* fix: Fix reactive tests by assigning @NotNull a default message

Refer: jhipster/generator-jhipster#13483

* fix: R2DBC with OAuth

Refer jhipster/generator-jhipster#13485

* fix: Remove Transactional when using no db

Refer:  jhipster/generator-jhipster#13489

* fix: Ensure /websocket/tracker/ cant bypass

Refer: jhipster/generator-jhipster#13440

* fix: Fix R2DBC with OAuth

Refer: jhipster/generator-jhipster#13493

* chore: fix minor code smells

* fix: Broken Swagger with Microservice

Refer: jhipster/generator-jhipster#13446

* fix: minor updates

Refer: jhipster/generator-jhipster#13495

* fix: Change save template to use precedence

Refer: jhipster/generator-jhipster#12802:
jhipster/generator-jhipster#12802

* fix: add where clause from data jpa criteria when reactive

Refer: jhipster/generator-jhipster#13515

* fix: update dependencies and linting fixes

* fix: Update build pipeline versions

* fix: Angular workflow

* fix: copy error

* fix: missing otherEntityRelationshipNamePlural variable

* fix: ci script

* fix: remove unsupported hr language

* fix: more angular ci fixes

* fix: more angular ci fixes

* fix: move CustomClaimConverter to security package

* fix: expected files

* fix: ci to use installed version of jhipster

* rename webpack profile to webapp

* fix: refactor ci test

* fix: scripts path

* fix use proper path

* fix: add script level JHI_HOME

* fix: use correct  kotlin-samples folder

* chore: [ci] update chmod

* fix: missing variable and correct file location

* chore: [CI] add JHI_SCRIPTS location

* chore: [CI] add JHI_CLONED_SCRIPTS location

* chore: [CI] add HOME location

* chore: [CI] add HOME location

* Use tilde for home folder

* remove 13-replace-jhipster-version script

* temp: check scripts folder

* fix: use local scripts

* chore: [CI] add default work directory

* fix: use leading comma

* chore: [CI] remove additional app test

* minor fixes

* fix: use proper primary key name

* fix: replace id with primaryKey.type

* chore: [CI] add react workflow

* chore: [CI] upgrade angular workflow

* chore: [CI] add webflux workflow

* fix: entityResource

* chore: [CI] more workflow changes

* chore: [CI] Remove prettier check

* chore: [CI] fix react and webflux workflow

* fix: add UUID import for cassandra

* chore: [CI] use proper script for webflux

* fix: use apply instead of constructor params

* fix: change the constructor params

* fix: add UUIDFilter import for entity criteria

* fix: make queryservice to use properly typed filter

* fix: tests to have proper primarykey reference

* fix: use proper type for filter in EntityQueryService

* fix: type issues

* fix: redis configuration of test containers

* fix: generator entity spec test

* fix: entity post fix

* chore: [Test] Lint fixes

* chore: remove maven cucumber tests

* fix: sql webflux issues

* fix: use property accessor instead of getter

* chore: CI remove kafka test case

* fix: remove string utils

* Update EntityRepositoryInternalImpl_reactive.kt.ejs

* fix: add test fixes and other converters

* feat: migrate templates to v7

* lint fixes

* fix: update dependencies

* update package lock with npm v7

* fix: mocha tests

* lint fixes

* fix: double ejs tags

* fix: removed extra ejs tag

* fix: refactor missing type definition

* fix: use proper class variable

* fix: import repository always

* fix: clean up tests

* fix: use proper variable

* fix: remove mock builder standalone setup

* fix: reactive templates

* fix: remove reactive block

* fix: reactive modify server open api filter

* fix: reactive gateway

* fix: more reactive fixes

* fix: reactive file generation

* fix: no return in save template

* fix more reactive test

* fix: remove deprecated size configuration

* fix row mapper type checks

* fix: tests in custom claim converter

* fix: custom claim converter test

* fix: columnConverter for webflux r2dbc

* fix: extra parentheis

* fix: null checks

* fix: use empty Set instead of null

* fix: tests and persistence constructor

* fix: add coroutines lib for webflux

* fix: add kotlin extensions library

* fix: add reactor dependencies for maven

* fix: remove mariadb converter issue

* fix: webflux build and jdk version

* fix: generator unit test

* fix: install generator after npm ci

* fix: change reactive containers for psql

* fix: remove tc in the database url

* fix: remove reactive sql tests

* chore: remove azure pipelines
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SecurityUtils .getCurrentUserLogin in a microservice returns "null" with OAuth2 and Okta
6 participants