Skip to content

Allow to override the ExpressionBasedAnnotationAttributeFactory #9470

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
ptahchiev opened this issue Feb 23, 2021 · 5 comments
Closed

Allow to override the ExpressionBasedAnnotationAttributeFactory #9470

ptahchiev opened this issue Feb 23, 2021 · 5 comments
Assignees
Labels
in: core An issue in spring-security-core status: declined A suggestion or change that we don't feel we should currently apply

Comments

@ptahchiev
Copy link

Hello,

so I am creating a bunch of services with methods inside them and on each method I add the @PreAuthorize annotation like so:

    @Override
    @PreAuthorize("hasAccess()")
    public Page<DepartmentDtoDefinition> getAllDepartments(@NonNull final Pageable pageable) {
    }

All of these services are packaged in a JAR file and shipped to the customer. What I really want is to resolve the expression in the @PreAuthorize annotation from the database or from an external file (I will cache it) so that the customer can specify their own expression and disallow access to some of the services.

I think what I need is here:

https://github.com/spring-projects/spring-security/blob/master/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java#L343

would it be possible to provide a protected method in the GlobalMethodSecurityConfiguration which will construct the ExpressionBasedAnnotationAttributeFactory? This way I would be able to override this method and provide my own implementation that reads the expressions from the db or the file.

Thank you.

@ptahchiev ptahchiev added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Feb 23, 2021
@jzheaux
Copy link
Contributor

jzheaux commented Feb 23, 2021

Hi, @ptahchiev, thanks for the suggestion. The support for this is being reworked right now in #9350, so please consider taking a look there to ensure that your use case is addressed.

In the meantime, I'd recommend resolving by bean:

@PreAuthorize("@authorizationService.hasAccess(...)")

where AuthorizationService is a custom bean in the ApplicationContext that can connect to the database.

If possible, you might also consider whether this database access can be performed during authentication time and authorities can be granted then. The nice thing about this is that you wouldn't need to access the database for each method invocation.

I'm going to close this answered, but please feel free to clarify if you think I've misunderstood.

@jzheaux jzheaux closed this as completed Feb 23, 2021
@jzheaux jzheaux self-assigned this Feb 23, 2021
@jzheaux jzheaux added in: core An issue in spring-security-core status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Feb 23, 2021
@ptahchiev
Copy link
Author

Hi @jzheaux I think you got it wrong.

I want users to change the value of the @PreAuthorize annotation. I also thought of creating a bean AuthorizationService, however in that case customers will not be able to use the expressions from spring security that they are familiar with (like permitAll(), denyAll(), or hasAnyRole([role1,role2])).

Currently what I ended up doing is a custom MethodSecurityMetadataSource where I inject the map:

public class NemesisRestrictedMetadataSource extends AbstractFallbackMethodSecurityMetadataSource {

    private Map<String, String> map;

    public NemesisRestrictedMetadataSource(Map<String, String> map) {
        this.map = map;
    }

    @Override
    protected Collection<ConfigAttribute> findAttributes(Method method, Class<?> targetClass) {
        List<ConfigAttribute> attributes = new ArrayList<>(1);

        String key = targetClass.getSimpleName() + "#" + method.getName();

        if (map.get(key) != null) {
            attributes.add(new SecurityConfig(map.get(key)));
        }

        return attributes;
    }
}

and this way I can specify denyAll() in my Map. However, now what i see is that the PreInvocationAuthorizationAdviceVoter abstains from voting and I believe it is because it expects the attribute to be of type PreInvocationAttribute and mine is of type SecurityConfig:

	private PreInvocationAttribute findPreInvocationAttribute(
			Collection<ConfigAttribute> config) {
		for (ConfigAttribute attribute : config) {
			if (attribute instanceof PreInvocationAttribute) {
				return (PreInvocationAttribute) attribute;
			}
		}

		return null;
	}

The PreInvocationExpressionAttribute however is package protected. I tried creating my own implementation of PreInvocationAttribute like this:

    class NemesisSecurityConfig extends SecurityConfig implements PreInvocationAttribute {

        public NemesisSecurityConfig(String config) {
            super(config);
        }
    }

but now i get:

java.lang.ClassCastException: class io.nemesis.platform.core.security.NemesisRestrictedMetadataSource$NemesisSecurityConfig cannot be cast to class org.springframework.security.access.expression.method.PreInvocationExpressionAttribute (io.nemesis.platform.core.security.NemesisRestrictedMetadataSource$NemesisSecurityConfig and org.springframework.security.access.expression.method.PreInvocationExpressionAttribute are in unnamed module of loader 'app')
	at org.springframework.security.access.expression.method.ExpressionBasedPreInvocationAdvice.before(ExpressionBasedPreInvocationAdvice.java:43)
	at org.springframework.security.access.prepost.PreInvocationAuthorizationAdviceVoter.vote(PreInvocationAuthorizationAdviceVoter.java:72)
	at org.springframework.security.access.prepost.PreInvocationAuthorizationAdviceVoter.vote(PreInvocationAuthorizationAdviceVoter.java:40)
	at org.springframework.security.access.vote.UnanimousBased.decide(UnanimousBased.java:75)
	at org.springframework.security.access.intercept.AbstractSecurityInterceptor.beforeInvocation(AbstractSecurityInterceptor.java:233)
	at org.springframework.security.access.intercept.aopalliance.MethodSecurityInterceptor.invoke(MethodSecurityInterceptor.java:65)

@jzheaux
Copy link
Contributor

jzheaux commented Feb 23, 2021

Have you already tried creating your own MethodSecurityExpressionHandler and MethodSecurityExpressionOperations? If you are only wanting to change the way existing expressions are handled, then this seems like the right point in the API to customize it.

@ptahchiev
Copy link
Author

Yes I did. However, the MethodSecurityExpressionHandler allows me to define new expressions like the built-in ones. Thus I can define isCustomerAuthorized() or something similar. However, I want to use the built-in ones (permitAll(), denyAll(), etc)

@ptahchiev
Copy link
Author

I ended up creating my own metadata source:

    private Properties securityProperties;

    public NemesisRestrictedMetadataSource(Properties properties) {
        this.securityProperties = properties;
    }

    @Override
    protected Collection<ConfigAttribute> findAttributes(Method method, Class<?> targetClass) {
        List<ConfigAttribute> attributes = new ArrayList<>(1);

        String key = targetClass.getSimpleName() + "." + method.getName();

        if (securityProperties.containsKey(key)) {
            attributes.add(new SecurityConfig((String) securityProperties.get(key)));
        }

        return attributes;
    }

    @Override
    protected Collection<ConfigAttribute> findAttributes(Class<?> clazz) {
        return null;
    }

    @Override
    public Collection<ConfigAttribute> getAllConfigAttributes() {
        return null;
    }

and I also register it in the security metadata source like this:

    @Bean
    @Override
    public MethodSecurityMetadataSource methodSecurityMetadataSource() {
        DelegatingMethodSecurityMetadataSource result = (DelegatingMethodSecurityMetadataSource) super.methodSecurityMetadataSource();

        Properties props = new Properties();
        try {
            ClassPathResource securityConfigurations = new ClassPathResource("security.properties");
            props.load(securityConfigurations.getInputStream());
        } catch (IOException ioex) {
            LOG.error(ioex.getMessage());
        }

        result.getMethodSecurityMetadataSources().add(new NemesisRestrictedMetadataSource(props));
        return result;
    }

As you can see I am reading a file security.properties from the classpath. The file looks like this:

DepartmentFacadeImpl.getAllDepartments=IS_AUTHENTICATED_FULLY

This allows me to use the the RoleVoter and specify configurations like IS_AUTHENTICATED and ROLE_ADMIN for example. However I cannot use the spel expression because the classes are package protected and I cannot extend it.

I still think this is a nice feature to have. Please reopen this issue. @rwinch what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core An issue in spring-security-core status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

2 participants