Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unauthorized redirect missing context path #16

Closed
jonmor51 opened this issue Apr 6, 2020 · 17 comments
Closed

Unauthorized redirect missing context path #16

jonmor51 opened this issue Apr 6, 2020 · 17 comments

Comments

@jonmor51
Copy link

jonmor51 commented Apr 6, 2020

In a Grails 4.0.2 app, with version 4.1 of the plugin, the unauthorized redirect, i.e., the URL the user is redirected to if not authorized, does not take account of the context path defined for the application (in server.servlet.context-path in application.groovy). So, for example, for my 'myapp' app, it comes up with http://localhost:8090/auth/login?targetUri=/myapp/mycontroller/myaction instead of http://localhost:8090/myapp/auth/login?targetUri=/myapp/mycontroller/myaction. The problem is in the resolveViewOrRedirect method of ShiroGrailsExceptionResolver.groovy. This line does not take into account the context path: `String forwardUrl = UrlMappingUtils.buildDispatchUrlForMapping(info).

Not sure whether a workaround for this is possible via settings? Bit of a showstopper as it is (as the redirect URL is not handled by my Grails app).

Small app attached which demonstrates the issue.
myapp.zip

`

@jonmor51
Copy link
Author

jonmor51 commented Apr 6, 2020

I don't know whether this is part of the same issue, but setting
security: shiro: session: handleExceptions: false
does not, in fact, result in the application using GrailsExceptionResolver instead of ShiroGrailsExceptionResolver as the docs seem to suggest - ShiroGrailsExceptionResolver still gets called,

@jonmor51
Copy link
Author

jonmor51 commented Apr 6, 2020

The good news is that using accessControl() from an Interceptor doesn't exhibit this problem, so I have a workaround. This appears to be restricted to using annotations. (EDITED: Removed extra comments here which turned out to be wrong)

@pmcneil
Copy link
Member

pmcneil commented Apr 6, 2020

You can tell the ShiroGrailsExceptionResolver where to redirect via URL mapping for 403/401 see https://nerderg.com/docs/shiro/guide.html#redirecting-unauthenticated-and-unauthorized That should "just work".

Thanks for the feedback. We have tested the handleExceptions: false so I'd like to see if there has been a problem introduced.

@pmcneil
Copy link
Member

pmcneil commented Apr 6, 2020

I think server.servlet.context-path can be an issue. I have applications where we could only get server.contextPath to work... I'll give your app a try tomorrow.
Oh and the correct setting to turn off handle exceptions is security.shiro.handleExceptions (see

boolean handleExceptions = grailsApplication.config.getProperty('security.shiro.handleExceptions', boolean, true)
)
Seems the documentation fell fowl of YAML indentation.

@jonmor51
Copy link
Author

jonmor51 commented Apr 6, 2020

Thanks, I'll give that a go. There are a couple of problems with the documentation, incidentally, in this area. In AccessControl.loginRedirect(), there is this: String redirectUri = grailsApplication.config.getProperty('security.shiro.login.uri'). But in the docs, you have url - default /auth/login (trumps controller/action) - i.e., 'url' instead of 'uri'. Happy to help out with this kind of thing in whatever way I can, once I know what I'm doing!

pmcneil added a commit that referenced this issue Apr 6, 2020
@pmcneil
Copy link
Member

pmcneil commented Apr 6, 2020

pushed fixes to guide.adoc will get them online today

pmcneil added a commit that referenced this issue Apr 6, 2020
@jonmor51
Copy link
Author

Is that a commit to fix the docs, or to address the issue with the context path?

@pmcneil
Copy link
Member

pmcneil commented Apr 14, 2020

docs

@pmcneil
Copy link
Member

pmcneil commented May 25, 2020

Sorry to take so long looking at context path issue again.

@pmcneil
Copy link
Member

pmcneil commented May 25, 2020

It looks like it's possibly a Grails 4 issue where the context-path isn't added to the DefaultUrlMapping Info - investigating.

@pmcneil
Copy link
Member

pmcneil commented May 26, 2020

latest commit upgrade to 4.3 of the plugin fixes the context-path issue and removes the use of the URL mapping as a separate configuration for Annotations. See the guide update for info.
I have re-written the ShiroGrailsExceptionResolver completely and separated it from the GrailsExceptionResolver. The result is a far more straight forward resolver that handles one job properly. It also uses the config for setting redirects for accessControl() Annotation based access.
Please give it a go and let me know of issues. I will modify the Grails 3 version now.

@jonmor51
Copy link
Author

Thanks very much! I'll give it a go as soon as I get the chance.

@pmcneil
Copy link
Member

pmcneil commented May 26, 2020

Also updated the grails 3 version (grails 3 branch).

@jonmor51
Copy link
Author

jonmor51 commented May 26, 2020

What am I missing?

compile "org.grails.plugins:grails-shiro:4.3" gives me this:

`| Error Error initializing classpath: Could not find org.grails.plugins:grails-shiro:4.3.
Searched in the following locations:

@pmcneil
Copy link
Member

pmcneil commented May 27, 2020

you need to install the plugin locally from source to test it out - I haven't published it yet :-) See the README.adoc for details.

I'm just implementing a few more tests before publishing (found a bug already!)

@pmcneil
Copy link
Member

pmcneil commented May 27, 2020

I have now published the plugins, both v3.3 and v4.3 they may take a while to show up on the grails plugin portal but you can see them at https://bintray.com/beta/#/nerderg/plugins/grails-shiro?tab=overview
this issue is now fixed :-)

@pmcneil pmcneil closed this as completed May 27, 2020
@jonmor51
Copy link
Author

I can confirm this is now working. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants