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

Role Hierarchy in authorizeHttpRequests() of HttpSecurity #13188

Closed
esselesse opened this issue May 17, 2023 · 5 comments
Closed

Role Hierarchy in authorizeHttpRequests() of HttpSecurity #13188

esselesse opened this issue May 17, 2023 · 5 comments
Assignees
Labels
in: config An issue in spring-security-config status: duplicate A duplicate of another issue type: enhancement A general enhancement

Comments

@esselesse
Copy link

esselesse commented May 17, 2023

Expected Behavior

HttpSecurity's authorizeHttpRequests() uses role hierarchy defined as a bean or defined as a separate method like setRoleHierarchy(RoleHierarchy rh) {...}

so the flow will be like

@Bean
public SecurityFilterChain filterChain(@NotNull HttpSecurity http) {
...
http.authorizeHttpRequests()
  .setRoleHierarchy(getRoleHierarchy())
  .requestMatchers("my.site/admin")
  .hasRole("ADMIN")
...
}

Current Behavior

HttpSecurity's authorizeHttpRequests() uses role hierarchy by .access() method using redefined AuthorityAuthorizationManager<RequestAuthorizationContext>. it is quite inconvenient and quite a lot of code.

so i need to do

@Bean
public AuthorityAuthorizationManager<RequestAuthorizationContext> adminAuthorityAuthorizationManager() {
  AuthorityAuthorizationManager<RequestAuthorizationContext> objectAuthorityAuthorizationManager = AuthorityAuthorizationManager.hasAuthority("ROLE_ADMIN");
  objectAuthorityAuthorizationManager.setRoleHierarchy(getRoleHierarchy());
  return objectAuthorityAuthorizationManager;
}

and then use this

http.authorizeHttpRequests()
  .requestMatchers("my.site/admin")
  .access(adminAuthorityAuthorizationManager())

Context

i found that AuthorityAuthorizationManager has a non-static RoleHierarchy field.
well, i propose:

  1. make a private static RoleHierarchy field in AuthorizeHttpRequestsConfigurer.AuthorizedUrl defaulted to NullRoleHierarchy
  2. set it by a static method setRoleHierarchy()
  3. pass it to hasRole... methods under-the-hood
  4. pass it to hasRole... methods of AuthorityAuthorizationManager in 3)
  5. set it to inner field roleHierarchy (!)
  6. continue as it is now

so, we will have an easy setter for roleHierarchy without clumsy use of redefined classes for each custom role

@esselesse esselesse added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels May 17, 2023
@evgeniycheban
Copy link
Contributor

evgeniycheban commented May 17, 2023

Hi, @esselesse, I think what you are looking for is already implemented, the RoleHierarchy can be defined as a @Bean and propagated to AuthorityAuthorizationManager, here is an example of how it can be achieved:

@Bean
SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
    return http
	    .authorizeHttpRequests((requests) -> requests
		.anyRequest().hasRole("USER")
	    )
	    .build();
}

@Bean
RoleHierarchy roleHierarchy() {
    RoleHierarchyImpl roleHierarchy = new RoleHierarchyImpl();
    roleHierarchy.setHierarchy("ROLE_ADMIN > ROLE_USER");
    return roleHierarchy;
}

This was added in #12473, so you may need to consider updating Spring Security version in your project.

@jzheaux Could you please remind in which release it was included?

@esselesse
Copy link
Author

@evgeniycheban if i use these plugins in build.gradle:

id 'org.springframework.boot' version '3.0.6'
id 'io.spring.dependency-management' version '1.1.0'

(that are latest versions of plugins at this momentum)
and use
implementation 'org.springframework.boot:spring-boot-starter-security'
in the body of dependencies - well, it is not working.

i assume that maybe something wrong in versions, cuz the problem seems exactly the same as You mentioned (#12473). should i use explicit version of spring security?

------- my problem is below this line, just as example ----------

i do this:

    @Bean
    public RoleHierarchy roleHierarchy() {
        RoleHierarchyImpl hierarchy = new RoleHierarchyImpl();
        hierarchy.setHierarchy(
                "ROLE_ADMIN"
                        + " > "
                        + "ROLE_USER"
                        + " > "
                        + "ROLE_GUEST");
        return hierarchy;
    }

and in filterchain

http.authorizeHttpRequests()
                .requestMatchers(ADMIN_URL_PATTERN, API_URL_PATTERN, MAILING_URL_PATTERN)
                .hasRole("GUEST")
                .anyRequest()
                .authenticated();

when i try to access any URL matches ADMIN_URL_PATTERN using a client with ROLE_USER role, i get 403 error (access denied).
but if i change "GUEST" to "USER" - everything fine.


@jzheaux
Copy link
Contributor

jzheaux commented May 18, 2023

This feature was released in 6.1.0-M1 and subsequently 6.1.0. If you update to Spring Boot 3.1.0 once it is out, then this dependency will be managed for you. Until then, yes, you can manage Spring Security up to 6.1.0 in your pom.

Can you try using Spring Security 6.1.0 and see if this works for you?

@jzheaux jzheaux self-assigned this May 18, 2023
@jzheaux jzheaux added status: waiting-for-feedback We need additional information before we can continue in: config An issue in spring-security-config and removed status: waiting-for-triage An issue we've not yet triaged labels May 18, 2023
@esselesse
Copy link
Author

tried out boot 3.1.0.
it's amazing!
works!

thank You!

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels May 19, 2023
@jzheaux jzheaux closed this as completed May 19, 2023
@jzheaux jzheaux added status: duplicate A duplicate of another issue and removed status: feedback-provided Feedback has been provided labels May 19, 2023
@jzheaux
Copy link
Contributor

jzheaux commented May 19, 2023

Glad the latest is working for you, @esselesse! I'll close this in favor of #12505, then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: config An issue in spring-security-config status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants