spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.55k stars 5.79k forks source link

Find Roles for Authority using the RoleHierarchy? #15264

Open nielsbasjes opened 1 week ago

nielsbasjes commented 1 week ago

My current understanding (and please correct me if this incorrect) is that

At the technical level these 2 concepts are the same, the only notable difference is that the Role has a prefix (default = "ROLE_") in the string that represents it.

Now recently the RoleHierarchyImpl has a Builder that supports a much easier way of defining the roles and this way hiding these prefixes has become much cleaner.

This code looks something like this:

@Bean
static RoleHierarchy roleHierarchy() {
    return RoleHierarchyImpl.withDefaultRolePrefix()
        .role("ADMIN").implies("STAFF", "TEAMS")
        .role("TEAMS").implies("OPERATORS")
        .role("STAFF").implies("USER")
        .role("USER").implies("GUEST")
        .build();
}

Since the Authorities and Roles are technically the same I have the suggestion to extend this builder to allow for something like this snippet. Here the role() and authority() have 1 difference: The prefix is used or not.

@Bean
static RoleHierarchy roleHierarchy() {
    return RoleHierarchyImpl.withDefaultRolePrefix()
        .role("ADMIN").implies("STAFF", "TEAMS")
        .role("TEAMS").implies("OPERATORS")
        .role("STAFF").implies("USER")
        .role("USER").implies("GUEST")

        .authority("TEAM_ABC").implies("TEAMS")
        .build();
}

If you like this idea then I'll put up a pull request.

If you think this is a totally wrong approach (which is actually quite likely) then please tell me what the right way of mapping an external Authority to a Role is?

marcusdacoregio commented 1 week ago

Hi @nielsbasjes, thanks for the suggestion.

Yes, I think it makes sense since the RoleHierarchy is applied when hasAuthority/hasAnyAuthority is used. Feel free to provide a PR and we can go from there.

nielsbasjes commented 1 week ago

Working on it. In the text format you can specify

   ROLE_A > ROLE_B
   ROLE_A > ROLE_C

which gives 'A' both 'B' and 'C'

In the builder this is broken.

This example gives 'A' only 'C' (the last one)

RoleHierarchyImpl.withDefaultRolePrefix()
    .role("A").implies("B")
    .role("A").implies("C")
    .build();

Fixing this in a separate commit.