All software downloaded from this blog is subject to the terms below:
Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following condition is met:
- Neither the name ‘Ola Bildtsen’ nor the names of other contributors may be used to endorse or promote products derived from this software without specific prior written permission.
THIS SOFTWARE IS PROVIDED BY Ola Bildtsen ”AS IS” AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL Ola Bildtsen BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
Baha AS | 08-Jan-09 at 4:06 pm | Permalink
Hi Ola,
We’ve integrated your code in the springsecurity plugin of grails of our project but I found a tiny misbehavior when securing controller actions as posted in your code. There is a little issue in the lookupAttributes(String url) method in the PathFilterInvoicationDefinitionMap class.
The issue is about line 36 where the condition is checking for narrowestEntries among the controller’s action entries:
if (!narrowestEntry || pathMatcher.match(entry.key, narrowestEntry.key))
the problem here is narrowestEntry is preset to the default /** coming from urlMappingsPathMap class so it is never empty and thus the previous condition never becomes true.
I have added the following condition after the closure of the if condition of line 36 (after the if (…){…} ***here***)
which will check the action entries against the actual url :
if (pathMatcher.match(entry.key, url) && entry.key.length()>=narrowestEntry.key.length()) {
narrowestEntry = entry
}
the second condition after the && operation allows to restrict narrowestEntry to strictest action mapping in case of similar action name prefixes, ie say we have the following action mapping in a controller:
product:['ROLE_USER'],
productList:['ROLE_ADMIN']
where accessing productList with ROLE_USER privilege will not match an entry of product action.
I hope this workaround might be useful for your article.
Regards and thanks for the code your provided, it’s very useful.
Ola Bildtsen | 09-Jan-09 at 10:07 am | Permalink
Thanks for the comment. Please note that you’re working against old code — this plugin has since been released as a Grails plugin in the Grails repository. There are many upgrades and bug fixes with the latest release, so please start using that plugin instead. Full documentation (including install guide) is here: http://www.grails.org/Stark+Security+Plugin
As for the problem you describe, let me first say that putting a mapping of ‘/**’ in the config mappings effectively defeats the purpose of this plugin by setting an authorization mapping for the entire site. The intention of the plugin is to have access control declared on the controller/method level and avoid such site-wide mappings.
But you do point out a defect: the “narrowestEntry” logic is incorrect because the args to AntPathMatcher are switched. Where the code reads:
if (!narrowestEntry || pathMatcher.match(entry.key, narrowestEntry.key))it should read
if (!narrowestEntry || pathMatcher.match(narrowestEntry.key, entry.key))I have opened a defect for this and will fix/release soon: http://jira.codehaus.org/browse/GRAILSPLUGINS-756
Baha AS | 09-Jan-09 at 3:44 pm | Permalink
Hi Ola,
Actually I have seen the plugin under grails.org but my colleague on his end have read this blog and integrated the sample code from available here into our project. In my mind i thaught he have integrated your grails plugin but that was not the case. It’s all right this is easily fixable.
The issue is this. In summary your code allows access by default to all pages hence the default path /** is mapped to all the roles and then the filter will restrict the mappings according to specific urlMappings first then controllerMapping filters get applied. So narrowestEntry.key is ‘/**’ for the first iteration of :
“controllerPathMap.findAll{ pathMatcher.match(it.key, url) }.each { entry ->”
which will never match any controller mapping entry (ie ‘/**’ will not match ‘controller/action’ path entries). So the problem is the if condition will never be true. What should have been done is test the condition of entry.key against the ‘url’ string instead of ‘narrowestEntry.key’.
What do you think?
Thanks for your comment,
Baha
Ola Bildtsen | 09-Jan-09 at 3:54 pm | Permalink
Baha,
You definitely want to go with the 0.4 version of the plugin from the grails.org site — it’s come a long way since the original posting of this code.
Please check it out — it has the fix in it for the “narrowest mapping” problem, I think you’ll find it works as you’d expect. And please let me re-iterate: using /** mappings as a default path is a bad idea — I would strongly recommend you to remove that and instead do all controller authorization mappings on the controller/action level. You misunderstand the plugin if you think that putting up a default /** authorization for the app is the way to go. That is precisely the thing the plugin is built to avoid…
Baha AS | 09-Jan-09 at 4:07 pm | Permalink
Thank you Ola,
I’ll make sure we use the plugin.
You’ve done a great job.
Kindest regards,
Baha