Spring Security
  1. Spring Security
  2. SEC-1558

Change signatures of PrePostInvocationAttributeFactory to use String arguments rather than annotations

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Complete
    • Affects Version/s: 3.0.3
    • Fix Version/s: 3.1.0.RC1
    • Component/s: Core
    • Labels:
      None

      Description

      Hi,

      For the next release of Cibet control framework (www.logitags.com/cibet) I have integrated Spring Security authorization. It was possible but I had to do some 'dirty' hacks which could be avoided if Spring Security provides some very small changes:

      Again the MetadataSource: My ConfigAttributes come from another source. In my implementation of MethodSecurityMetadataSource I have to instantiate PreInvocationExpressionAttribute and PostInvocationExpressionAttribute.

      Problem: PreInvocationExpressionAttribute and PostInvocationExpressionAttribute classes are not public and have no public constructors.
      Workaround: Create own Attribute classes in package org.springframework.security.access.expression.method which inherit from the two Pre and Post classes.
      Solution: Make classes and constructor of PreInvocationExpressionAttribute and PostInvocationExpressionAttribute public

        Issue Links

          Activity

          Hide
          Luke Taylor added a comment -

          An alternative here might be to alter the signature of PrePostInvocationAttributeFactory, making it non-annotation specific, thus allowing the metadata to be sourced from elsewhere. Alternatively, extra methods could be added to ExpressionBasedAnnotationAttributeFactory. I don't want to expose all the internal implementations though.

          Show
          Luke Taylor added a comment - An alternative here might be to alter the signature of PrePostInvocationAttributeFactory, making it non-annotation specific, thus allowing the metadata to be sourced from elsewhere. Alternatively, extra methods could be added to ExpressionBasedAnnotationAttributeFactory. I don't want to expose all the internal implementations though.
          Hide
          Wolfgang Winter added a comment -

          That surely would mean bigger code changes for you especially changing method signatures is a big thing. On the other hand, the other implementations of ConfigAttribute are public, only the ExpressionBased classes are not. But I can understand when you want prevent a too tight coupling to your code.

          Show
          Wolfgang Winter added a comment - That surely would mean bigger code changes for you especially changing method signatures is a big thing. On the other hand, the other implementations of ConfigAttribute are public, only the ExpressionBased classes are not. But I can understand when you want prevent a too tight coupling to your code.
          Hide
          Luke Taylor added a comment -

          I've decided to go with modifying the signature. There's no real reason why it needs to take annotations as the arguments. It should be a simple change for implementers, since the attributes only have a String value anyway. It will be more flexible to just use Strings directly.

          Show
          Luke Taylor added a comment - I've decided to go with modifying the signature. There's no real reason why it needs to take annotations as the arguments. It should be a simple change for implementers, since the attributes only have a String value anyway. It will be more flexible to just use Strings directly.
          Hide
          Luke Taylor added a comment -

          Done. This will have a minor impact on anyone with a custom PrePostInvocationAttributeFactory implementation, but I suspect there are very few of those out there.

          Show
          Luke Taylor added a comment - Done. This will have a minor impact on anyone with a custom PrePostInvocationAttributeFactory implementation, but I suspect there are very few of those out there.

            People

            • Assignee:
              Luke Taylor
              Reporter:
              Wolfgang Winter
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: