From: lhazlewood (s. by Nabble.com) <li...@na...> - 2005-12-30 17:27:11
|
I'm thinking we should remove the default target and actions ('*') in the HasPermission annotation. Automatically defaulting to these will cause problems if the user wants to specify a permission class that doesn't have a 2 (or even 1) argument constructor. I'm explicitly thinking of classes like AllPermission or similar ones. Although we are assuming that Permission instances will act as JSecurity's ACL mechanism, there is no reason why the framework shouldn't support other Permission classes that don't conform 100% to this assumption. Will this pose any problems to you if the defaults become null? -- Sent from the Developer forum at Nabble.com: http://www.nabble.com/HasPermission-default-target-and-actions-t826978c13668.html#a2145624 |
From: jhaile (s. by Nabble.com) <li...@na...> - 2005-12-30 19:17:06
|
Maybe you could provide a little more info as to how the HasPermission tag would work if we made this change. Currently, all of my permissions have 2-arg constructors. I also do not use the ACL feature, but instead only ever specify the class name and the action(s) - therefore depending upon the default value of "*". The HasPermission annotation currently assumes that all permission classes have a 2-arg constructor. If we changed the annotation to not default to "*" for target and actions, what would it default to? I don't want to have to specify target for every annotation since I don't even use it. Is the HasPermission module going to guess at how many args the constructor takes in? etc etc? I could make up possibilities all day long, but how exactly are you proposing the tag would work if we changed the default from "*"? -- Sent from the Developer forum at Nabble.com: http://www.nabble.com/HasPermission-default-target-and-actions-t826978c13668.html#a2146878 |
From: lhazlewood (s. by Nabble.com) <li...@na...> - 2005-12-30 20:04:52
|
I think my concern comes from using Permission classes without 2 arg constructors. As a framework, I think we should support their use if developers want to use them. For example, this example method would not work as things stand now: @HasPermission(type=com.company.pkg.MyPermission.class, target="someName") public void foo(); if the MyPermission class only had a default no-arg or single arg constructor. JSecurity would try to look up a two-arg constructor because of the '*' argument sent implicitly as the default. Here's an interesting note though - A _lot_ of JDK Permission implementations have a 2 arg constructor, but usually ignore the 2nd argument entirely, just to maintain constructor parallelism with the BasicPermission class from which they extend. This is yet another example of the poor design decisions made in JAAS. Anyway, these dummy constructors are convenient for us, but requires custom Permission implementers who use JSecurity to make sure they do the same thing - pretty inconvenient for them and propagates the poor design. Do we want to force them to do this? -- Sent from the Developer forum at Nabble.com: http://www.nabble.com/HasPermission-default-target-and-actions-t826978c13668.html#a2147416 |
From: jhaile (s. by Nabble.com) <li...@na...> - 2005-12-30 20:10:50
|
Well, one option would be to leave the defaults as they are now, but enhance the "constructor guessing" in PermissionAnnotationAuthorizationModule. All we need to do is catch NoSuchMethodException when getting the declared constructor. I would assume the guessing would go something like this: 1) First check for the two arg constructor. (target, action) 2) Check for a single arg constructor (only action) 3) Check for a no-arg constructor. This would of course be specified in JavaDoc and referenced both by the PermissionAnnotationAuthorizationModule and the HasPermission annotation classes. -- Sent from the Developer forum at Nabble.com: http://www.nabble.com/HasPermission-default-target-and-actions-t826978c13668.html#a2147481 |
From: lhazlewood (s. by Nabble.com) <li...@na...> - 2005-12-30 22:30:50
|
I had thought of doing this, but it rubs me the wrong way. What about the case where a Permission class has a 1 and 2 argument constructor. If I specify the annotation with just a target/name and no actions, I expect the 1 argument constructor to be called. That would not happen now since JSecurity automatically assumes the second argument must be defaulted to the wildcard - thereby calling the 2 argument constructor, even if the developer didn't intend that to happen. If one desires the default behavior (as do I almost all of the time), I think it is better form to have a 1 argument constructor that merely calls the two argument constructor, automatically filling in the second argument as the wildcard. In fact, the JSecurity InstancePermission already does this. If one specifies just a name and no actions, and there does not exist a single-argument constructor, this should be a programming error - not something JSecurity 'fixes' behind the scenes. That is, I feel it should be the responsibility of the person implementing the Permission class to provide that default behavior if desired (as with InstancePermission), not JSecurity. By having JSecurity do things they way they are now, we limit flexibilty by enforcing a certain programming paradigm, which I don't favor when implementing a wide-use framework. -- Sent from the Developer forum at Nabble.com: http://www.nabble.com/HasPermission-default-target-and-actions-t826978c13668.html#a2148896 |
From: jhaile (s. by Nabble.com) <li...@na...> - 2005-12-30 22:36:51
|
So would the defaults be "" and JSecurity would know that if you specify an empty string it should use a 1 or 2 arg constructor? There definitely needs to be some default value, and annotations will not allow you to default them to null, so I imagine we will need to use an empty string. So, 1) if I specify @HasPermission(type="..") JSecurity will use a 0-arg constructor. 2) if I specify @HasPermission(type="..", actions="read"), JSecurity will use a 1-arg constructor. 3) if I specify @HasPermission(type="..", target="123"), JSecurity will again use a 1-arg constructor? 4) if I specify @HasPermission(type="..",target="123",actions="read") JSecurity will use a 2-arg constructor. Is this what you are thinking? If not, please give me a similar example and be sure to be specific about what the default values would be. -- Sent from the Developer forum at Nabble.com: http://www.nabble.com/HasPermission-default-target-and-actions-t826978c13668.html#a2148957 |
From: lhazlewood (s. by Nabble.com) <li...@na...> - 2005-12-30 22:59:18
|
Yes, this is exactly what I was thinking. But, according to the Permission class and subclasses, a single argument String constructor is always assumed to be the name/target parameter, never the actions. So, we should throw an exception if #2 ever occurred (i.e. that would be a configuration error). And yes, because of the JDK limiting null defaults, I would expect the empty string to mean unspecified (and probably convert to null in the code). -- Sent from the Developer forum at Nabble.com: http://www.nabble.com/HasPermission-default-target-and-actions-t826978c13668.html#a2149214 |
From: jhaile (s. by Nabble.com) <li...@na...> - 2005-12-31 02:08:23
|
Hmmm - but #2 is what I do everywhere. If the Permission class assumes that a name/target will always be specified, why not just leave the default target as "*"? We could make actions optional though if we want to allow a 1-arg constructor. Alternatively we could drop the use of the Permission JDK class entirely and create a JSecurity permission class. I wonder what issues that would cause in the adoption of JSecurity. On the one hand, I think users wouldn't care. On the other hand, it might make it easier to migrate from an existing JAAS implementation. -- Sent from the Developer forum at Nabble.com: http://www.nabble.com/HasPermission-default-target-and-actions-t826978c13668.html#a2150592 |
From: lhazlewood (s. by Nabble.com) <li...@na...> - 2005-12-31 03:40:25
|
I think the Permission class is too widespread and ingrained in the Java security mindset to abandon it. Aside from minor inconveniences, it seems to work well enough. The name itself is very intuitive to new users (i.e. everyone is pretty comfortable with the terms 'user' 'role' and 'permission'), so this is a good thing. I'm good with leaving the name/target attribute as '*'. This to me is certainly better than what we have now, which always forces a two 2-arg-constructor to be used. If the developer doesn't like it, they can manually override the target to be whatever they want. At least this gives them flexibility over how the permission is constructed, unlike the current code. So, I'll go ahead and make the change of the default actions string from "*" to "" and update the permission instantiation logic accordingly. This is good, I've now achieved the warm-n-fuzzy feeling I was looking for ;) -- Sent from the Developer forum at Nabble.com: http://www.nabble.com/HasPermission-default-target-and-actions-t826978c13668.html#a2150970 |