SEC-1234: RolesAllowed or Secured Annotations on Interface don't apply to child Interfaces #1473

Closed
spring-issuemaster opened this Issue Sep 3, 2009 · 11 comments

1 participant

@spring-issuemaster

Karl Palsson (Migrated from SEC-1234) said:

I have an interface IGenericDAO, which has save, saveOrUpdate and delete methods, marked with @RolesAllowed("ROLE_ADMIN") [1]
I then have an interface ICustomerDAO which extends IGenericDAO.

There is a concrete implementation GenericDAO, which implements save, saveOrUpdate and delete, with no annotations.
There is a concrete implementation CustomerDAO which extends GenericDAO and adds some extra search helpers.

CustomerDAO does not override save, saveOrUpdate or delete, nor does ICustomerDAO.

I then get the following unusual behaviour....

IGenericDAO dao; // injected by spring (concrete GenericDAO)
ICustomerDAO csDAO; // injected by spring (concrete CustomerDAO)

dao.save(new BlahDomainObject("genericblah)); // rejected for ROLE_USER, as expected
csDAO.save(new BlahDomainObject("csBlah")); // Succeeds!!! for ROLE_USER, definitely not expected!

This is NOT expected. save() is annotated in the only place it can be, but has no affect when I use the extension interface :(

[1] I tried this with @Secured from spring as well, with the same results

@spring-issuemaster

Luke Taylor said:

Please submit a test case which demonstrates the issue.

@spring-issuemaster

Karl Palsson said:

I've tried adding tests to SecuredMethodDefinitionSourceTests and Jsr250MethodDefinitionSourceTests but I can't get the working case to pass the test, at least, not following the other tests there... Until I can get the known working case to pass a test, I can't get a failing test :(

Any help?

//// Test to add to SecuredMethodDefinitionSourceTests

public void testInheritedExtendedSecured() throws Exception {
    Parent parent = new Parent();
    Method methodParent = parent.getClass().getMethod("adminMethod");

// both of these return null....
// ConfigAttributeDefinition attrsParent = this.mds.findAttributes(methodParent, parent.getClass());
ConfigAttributeDefinition attrsParent = this.mds.findAttributes(methodParent, IParent.class);
assertNotNull(attrsParent);
assertTrue(attrsParent.getConfigAttributes().size() == 1);
assertEquals("ADMIN", attrsParent.getConfigAttributes().iterator().next().toString());
}

public interface IParent {
    @Secured("ADMIN")
    public void adminMethod();
}

public interface IChild extends IParent {
    public void extraMethod();
}

public class Parent implements IParent {
    public void adminMethod() {
        System.out.println("admin Method happily invoked");
    }
}

public class Child extends Parent implements IChild {
    public void extraMethod() {
        System.out.println("extra method happily invoked");
    }
}
@spring-issuemaster

Luke Taylor said:

The issue here is probably that the method you are using is the one on the class, rather than the proxied interface (which will be the invoked method in practice). So

public void testInheritedExtendedSecured() throws Exception {
    Parent parent = new Parent();
    Method methodParent = IParent.class.getMethod("adminMethod");
    List<ConfigAttribute> attrsParent = this.mds.findAttributes(methodParent, parent.getClass());
    assertNotNull(attrsParent);
    attrsParent = this.mds.findAttributes(methodParent, IParent.class);
    assertNotNull(attrsParent);
}

will work. But if you can put together something that represents the actual scenario as a test case (using an application context) then that will probably be more representative of the issue you're encountering.

@spring-issuemaster

Karl Palsson said:

Hrmmmm, this is working as expected in a tiny sample webapp. I'll try and find out more about what's different between my full config and this sample config.

@spring-issuemaster

Karl Palsson said:

Yuck. I've built up what started as a stripped down version up almost back up to my full config, and it works properly in the test project, and fails miserably in the full config. I'm about out of time for this right now, and it's going to get shelved for a week or two. No idea what the difference is between the two setups yet :(

@spring-issuemaster

Luke Taylor said:

Thanks for putting in the time to try anyway. I will leave the issue open for the time being.

@spring-issuemaster

Luke Taylor said:

Not ACLs - changed to core.

@spring-issuemaster

Luke Taylor said:

Any further updates here? If not, I'll close as "cannot reproduce" for the time being.

@spring-issuemaster

Karl Palsson said:

Much as I'd love to, I'm afraid no, I've not gotten back to this. It's still on our radar internally, so if/when we try this again, we'll update this bug. You can "cannot reproduce" it for now though :) Thanks

@spring-issuemaster

Luke Taylor said:

OK, thanks :)

@spring-issuemaster

Karl Palsson said:

I tried the original code again with spring security 3, and it works now. So while something may have been wrong before, it's not anymore :)

@spring-issuemaster spring-issuemaster added this to the 3.0.0 RC1 milestone Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment