Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

SEC-1041: Simplify custom Permission implementations based on AbstractPermission #1292

spring-issuemaster opened this Issue Nov 26, 2008 · 2 comments


None yet
1 participant

Grzegorz Borkowski(Migrated from SEC-1041) said:

Implementation of custom Permissions should be an easy task. However, if just look at JavaDocs available at Spring site, and you want to extend either AbstractPermission or BasePermission, it is not so easy. You have to call the only constructor taking two parameters: mask and code; but there is no hint in JavaDoc what is mask and what is code (personally I was able to guess what it mask, but I had no idea what is a code). In fact you have to download the source code for Spring Security and only then you can find the the code of AbstractPermission what is the meaning. I believe, you should be able to write you own Permission without looking into the source.
Also, Javadoc says you cannot use RESERVED_ON or RESERVED_OFF – but looking at JavaDoc you don’t know their values!
So the JavaDoc should be improved.

Also, I don’t understand why I have to pass the code. It is used only for printing the string-representation of Permission in logs or on console, so personally I don’t care much (it can simplify debugging, but is not crucial definitively). Only mask is crucial. So why not provide second simplified constructor taking only mask, and using some default “ON” character for active bits (AclFormattingUtils already uses default character ‘*’ for such case in one-arg printBinary() method, we can use it too – BTW. there is also bug in javadoc for this method, the actual character that is being printed is missing)

Grzegorz Borkowski said:

I’m attaching the patch with the propositions described earlier:
1. Fixed javadoc for Permission and AclFormattingUtils classes
2. Created new field DEFAULT_ON=‘*’ in Permission interface
3. Added simplified constructor for AbstractPermission and BasePermission, using DEFAULT_ON character for code.

(I hope I’m not missing some important reason for which you couldn’t use such simplified constructor with DEFAULT_ON code character)

Luke Taylor said:

Thanks for the patch. I've applied it with the exception that I kept the Permission class as is - i.e. I didn't introduce the extra constant but just added the constructor to AbstractPermission which sets the code to '*'.

@spring-issuemaster spring-issuemaster added this to the 3.0.0 M1 milestone Feb 5, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment