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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@WithOidcLogin using json file similarly as @WithJwt #211

Closed
ffroliva opened this issue May 10, 2024 · 8 comments
Closed

@WithOidcLogin using json file similarly as @WithJwt #211

ffroliva opened this issue May 10, 2024 · 8 comments
Assignees
Labels
enhancement New feature or request

Comments

@ffroliva
Copy link

ffroliva commented May 10, 2024

I would like to use @WithOidcLogin in the same manner as @WithJwt as I feel it is more intuitive in certain usecases.

As this approach currently doesn't exist, is there an example of how to customize @WithOidcLogin? Maybe a Javadoc with an example of usage could be added to the interface following the approach taken in WithJwt. I feel it would facilitate for newcomers.

Thank you for such a great library!

@ffroliva ffroliva added the bug Something isn't working label May 10, 2024
@ch4mpy ch4mpy added enhancement New feature or request and removed bug Something isn't working labels May 13, 2024
@ch4mpy
Copy link
Owner

ch4mpy commented May 13, 2024

What is your use case for needing to configure more than authorities on an OAuth2 client?

Configuring an OAuth2AuthenticationToken from claims would require the test Authentication factory to scan the application context for potentially customized OAuth2UserService and GrantedAuthoritiesMapper. For that, some test configuration has to be imported, which complicates usage in parameterized test or when spring-addons is not involved. So, if something like that would be done, it should probably be with another annotation.

@ffroliva
Copy link
Author

How about adding an example of usage of the annotation as javadocs, for example:

@WithOidcLogin(
        authorities = {"ROLE_ADMINISTRATOR"},
        claims = @OpenIdClaims(
                otherClaims = @Claims(
                        stringClaims = @StringClaim(name = "preferred_username", value = "admin"),
                        stringArrayClaims = @StringArrayClaim(name = "group", value = {"Administrator"}))))
@Test
void testAdminUserAuthenticated() {
...
}

@ch4mpy
Copy link
Owner

ch4mpy commented May 15, 2024

Sure, adding Javadoc is possible, but this does not answer my question...

@ffroliva
Copy link
Author

Sorry for not answering before.

The authorities are initially mapped from groups that the user belongs to. The user can be in multiple groups at the same time but only one group can be active at a time. If the user has multiple groups assigned I need to redirect him to a group section page where a given group will be activated and a new token will be generated with a special activeGroup claim provided. Any suggestion on how to handle such a use case?

@ch4mpy
Copy link
Owner

ch4mpy commented May 15, 2024

Here is the draft for the Javadoc:

Populates the test security context with an OAuth2AuthenticationToken instance with a DefaultOidcUser as principal.

Only the annotation properties are used to build the authentication Instance. Neither OAuth2UserService nor GrantedAuthoritiesMapper are called.

Usage to define just authorities:

@WithOidcLogin({"BIDULE", "CHOSE"})

Advanced usage to set any claims, including private ones:

@WithOidcLogin(
   authorities = {"NICE"},
   nameAttributeKey = StandardClaimNames.PREFERRED_USERNAME,
   claims = @OpenIdClaims(
     preferredUsername = "tonton-pirate",
     email = "[email protected]",
     otherClaims =  @Claims(
       stringClaims = { @StringClaim(name = "machin", value = "truc") })))

@ch4mpy
Copy link
Owner

ch4mpy commented May 15, 2024

only one group can be active at a time

Why that? If a user is member of several groups, requesting him to manually switch between profiles to be able to do something he is allowed to seems a pretty bad user experience...

a new token will be generated with a special activeGroup claim provided

This means logout and then login (again, pretty bad UX)

Also, issuing tokens is the job of the authorization server, when keeping the information about this activeGroup is probably the job of a resource server and redirections the job of the client. This makes the solution pretty complicated (you'll have to expose an endpoint on the resource server for the authorization server to query for the user's active group during login).

@ffroliva
Copy link
Author

ffroliva commented May 16, 2024

Here is the draft for the Javadoc:

Populates the test security context with an OAuth2AuthenticationToken instance with a DefaultOidcUser as principal.

Only the annotation properties are used to build the authentication Instance. Neither OAuth2UserService nor GrantedAuthoritiesMapper are called.

Usage to define just authorities:

@WithOidcLogin({"BIDULE", "CHOSE"})

Advanced usage to set any claims, including private ones:

@WithOidcLogin(
   authorities = {"NICE"},
   nameAttributeKey = StandardClaimNames.PREFERRED_USERNAME,
   claims = @OpenIdClaims(
     preferredUsername = "tonton-pirate",
     email = "[email protected]",
     otherClaims =  @Claims(
       stringClaims = { @StringClaim(name = "machin", value = "truc") })))

Looks good to me. I would probably add a few other @Claims like stringArrayClaims = @StringArrayClaim(name = "group", value = {"Administrator"})) just for the sake of showing people that there are multiple options of claim types.

Perhaps add the test method after the annotation to show where it should be used.

@WithOidcLogin(
   authorities = {"NICE"},
   nameAttributeKey = StandardClaimNames.PREFERRED_USERNAME,
   claims = @OpenIdClaims(
     preferredUsername = "tonton-pirate",
     email = "[email protected]",
     otherClaims =  @Claims(
       stringClaims = { @StringClaim(name = "machin", value = "truc") }),
       stringArrayClaims = @StringArrayClaim(name = "myCustomStringArrayClaim", value = {"Administrator"})))
@Test
void test() {
...
}

@ch4mpy
Copy link
Owner

ch4mpy commented May 16, 2024

There are way too many properties to demo it all. Also, the IDE auto completion is there too...

Released what was drafted with 7.7.0

@ch4mpy ch4mpy closed this as completed May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants