Skip to content

Reactive PermissionEvaluator? #5046

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

Closed
sp00m opened this issue Feb 27, 2018 · 20 comments
Closed

Reactive PermissionEvaluator? #5046

sp00m opened this issue Feb 27, 2018 · 20 comments
Labels
status: duplicate A duplicate of another issue

Comments

@sp00m
Copy link

sp00m commented Feb 27, 2018

Summary

I plan to implement my own PermissionEvaluator, but the methods signatures don't allow me to use reactive types.

Actual Behavior

public class MyPermissionEvaluator implements PermissionEvaluator {

  @Override
  public boolean hasPermission(Authentication authentication, Object targetDomainObject, Object permission) {
    // db calls, so reactive return types
  }

  @Override
  public boolean hasPermission(Authentication authentication, Serializable targetId, String targetType, Object permission) {
    // db calls, so reactive return types
  }

}

Expected Behavior

public class MyPermissionEvaluator implements PermissionEvaluator {

  @Override
  public Mono<Boolean> hasPermission(Authentication authentication, Object targetDomainObject, Object permission) {
    // db calls, so reactive return types
  }

  @Override
  public Mono<Boolean> hasPermission(Authentication authentication, Serializable targetId, String targetType, Object permission) {
    // db calls, so reactive return types
  }

}

Version

5.0.2.RELEASE

@sp00m sp00m closed this as completed Mar 2, 2018
@marc-christian-schulze
Copy link

@sp00m Could you please comment on why the ticket has been closed? Did you find a solution, if so - how does it look like?

@sp00m sp00m reopened this May 20, 2018
@sp00m
Copy link
Author

sp00m commented May 20, 2018

@marc-christian-schulze I gave up trying to use these magic methods to be honest, they did not really fit the complex multitenant authorisation system I'm working with anyway. Reopened the ticket though if you're facing this issue as well.

@rwinch
Copy link
Member

rwinch commented May 21, 2018

Thank you to both of you for your feedback.

We do not have plans to provide a PermissionEvaluator for reactive method security. Our recommendation is to place your logic in a Bean and then access it through @beanName.methodThatReturnsBoolean. This allows developers to avoid coupling their code to Spring Security. We do not currently support reactive methods for the bean, but that work is already being tracked by #4841.

@rwinch rwinch closed this as completed May 21, 2018
@rwinch rwinch added the status: duplicate A duplicate of another issue label May 21, 2018
@sp00m
Copy link
Author

sp00m commented May 21, 2018

Makes sense, thanks!

@edeandrea
Copy link
Contributor

edeandrea commented Aug 13, 2018

So I'm just coming across this - does that mean when using webflux you have no plans to support @PreAuthorize/@PostAuthorize annotations on methods using the hasPermission expression?

Something like

@GetMapping("/somePath")
@PreAuthorize("hasPermission('someResource', 'someAction')")
public Mono<SomeObject> getSomething() {
  return Mono.fromSupplier(() -> new SomeObject());
}

Where the hasPermission expression is implemented using a PermissionEvaluator.

It also looks like even if an application wanted to do it for themselves, spring security has completely closed it off so that it isn't extensible. Spring Security's ReactiveMethodSecurityConfiguration is package-protected and re-creates instances of DefaultMethodSecurityExpressionHandler inside several methods.

@rwinch
Copy link
Member

rwinch commented Aug 13, 2018

@edeandrea You can create your own Bean as described above. This decouples your code from Spring Security's APIs.

@edeandrea
Copy link
Contributor

So would

@PreAuthorize("hasPermission('someResource', 'someAction')")

become

@PreAuthorize("@myBean.someMethod('someResource', 'someAction')")

?

@rwinch
Copy link
Member

rwinch commented Aug 13, 2018

Yes

@edeandrea
Copy link
Contributor

Ok thanks - I'll give it a try.

@edeandrea
Copy link
Contributor

Thanks @rwinch this seems to work.

Does the reactive support for reactive method security allow for specifying custom expressions? Or would we have to rely on having purely bean references via the @beanName.method syntax?

@edeandrea
Copy link
Contributor

Hi @rwinch just an additional question on this. Within my organization we have pretty much standardized on the hasPermission expression. We have frameworks which sit on top of Spring Boot/Spring Security which implement PermissionEvaluator and then call out to a centralized entitlements service for checking the authorization (in the future we're moving that service to an OAuth2-compatible AuthorizationServer - but for now that is not the case). When applications move from servlet to reactive we want to be backwards-compatible functionally, which means leaving the hasPermission expression in-tact as well as our hierarchy of PermissionEvaluators (we have a hierarchy of them depending on various available configurations).

My question is this - would we be able to bring that functionality back in simply by doing something like the following code snippet in the auto-configuration within our own custom starter? When I try it out it seems to work fine. I just want to make sure by doing this I'm not overriding something or breaking something (i.e. the OAuth2MethodSecurityExpressionHandler for example - although if the end application is using OAuth2 I'd want it to be able to use our PermissionEvaluator hierarchy as well, since we have places in the hierarchy for dealing with resolving permissions via checking scopes from tokens).

Essentially we try and abstract all the security details away from the applications so that there is 1 consistent interface when it comes to performing authorization. We want all of our authorization logic across all applications within the organization to be centralized. Applications themselves should not be making those decisions.

@Configuration
@ConditionalOnWebApplication(type = Type.REACTIVE)
@EnableReactiveMethodSecurity
@EnableWebFluxSecurity
public class ReactiveSecurityConfig {
  @ConditionalOnBean(MyCompanyCustomPermissionEvaluatorInterface.class)
  @Bean
  public DefaultMethodSecurityExpressionHandler methodSecurityExpressionHandler(MyCompanyCustomPermissionEvaluatorInterface permissionEvaluator) {
    DefaultMethodSecurityExpressionHandler handler = new DefaultMethodSecurityExpressionHandler();
    handler.setPermissionEvaluator(permissionEvaluator);

    return handler;
  }
}

@rwinch
Copy link
Member

rwinch commented Oct 4, 2018

It's going to be very challenging to make this compatible between a reactive stack and non reactive stack because you cannot have any blocking methods in a reactive stack. All of the PermissionEvaluator implementations are blocking since they don't return a Publisher. This means they cannot perform a blocking operation (i.e. cannot call a remote service).

@edeandrea
Copy link
Contributor

Thank you @rwinch. Seems like the better way to go would be to deprecate our current use of PermissionEvaluator and build something internal to our framework at a base layer that takes a generic return type, then build another layer, one that returns boolean and another that returns Mono<Boolean>. Then let our starters wire in the correct one.

Then for our current servlet stack build an adapter which can adapt a PermissionEvaluator to our new base layer.

If we do that is there a way to configure spring security so that we can wire the hasPermission expression to something other than a PermissionEvaluator? Or is that wiring static & can not be changed?

@rwinch
Copy link
Member

rwinch commented Oct 4, 2018

You would extend DefaultWebSecurityExpressionHandler and override the createSecurityExpresssionRoot to create a root object that delegates hasPermission to another object

@koenpunt
Copy link

It has been about 3 years since the last message in this thread; did anything change since then regarding permission evaluators in a reactive stack?

@rasmus-rudling
Copy link

If you don't mind sharing, how did your implementation look like @edeandrea?

@rasmus-rudling
Copy link

For anyone coming here in the future, here's how I was able to create a custom Authorization method:

// Security config

@EnableWebFluxSecurity
@EnableReactiveMethodSecurity
class SecurityConfiguration {
    // ...
// Controller

@PreAuthorize("@customGuard.someMethod(#authentication, #someId, 'someAction')")                                                                
suspend fun getController(authentication: Authentication, @PathVariable someId: UUID): ResponseEntity<CustomResponseEntity> {
    // ...
// Custom guard

@Component
class CustomGuard {
    fun someMethod(authentication: Authentication, resourceId: String, action: String): Boolean {
        // Your authorization logic goes here
        // return true if the authorization is granted; otherwise, return false
        return false
    }
}

I still don't know how my controller imports @customGuard though since I'm never defining it with lowercase, but anyways - it works🤷‍♂️

@edeandrea
Copy link
Contributor

Hi @rasmus-rudling I don't have any local changes anymore (this was a few years and several laptops ago :) ).

All the work I had done on it is in #5980

It never got merged in due to some conflicts and breaking changes, and I just got too busy to work on it anymore.

Feel free to continue with it and/or take any pieces from it. It has not been kept up to date with spring/spring security versions either.

@edeandrea
Copy link
Contributor

I still don't know how my controller imports @customGuard though since I'm never defining it with lowercase, but anyways - it works

Because in spring your bean CustomGuard gets a bean name of customGuard (by default the first letter of the bean class gets lowercased).

Then in spring expression language you reference a bean by name using @beanName.

@rasmus-rudling
Copy link

Thank you for that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

6 participants