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

'next' parameter fails with blueprint endpoints (e.g. "User.view") #172

Closed
nk9 opened this issue Sep 1, 2019 · 1 comment
Closed

'next' parameter fails with blueprint endpoints (e.g. "User.view") #172

nk9 opened this issue Sep 1, 2019 · 1 comment
Assignees
Labels

Comments

@nk9
Copy link

nk9 commented Sep 1, 2019

NB: This is a clone of the issue I filed on the original Flask-Security project.

The login view is a NextFormMixin subclass, so it accepts a next parameter. As I understand it, the parameter can be a URL, a path or an endpoint. If you use a simple endpoint like "index", it works as expected and redirects to the provided endpoint on login.

However, endpoints in a blueprint have a period, like "User.profile". If you pass this kind of endpoint to 'next', the login view will treat it like a path and send you to e.g. http://localhost:4000/User.profile. This happens because validate_redirect_url() returns True for the blueprint endpoint. This, in turn, means that get_post_action_redirect() will use the very first URL in its list, which is just the raw endpoint name. It doesn't get to the 3rd element of the list, which is the result of get_url(request.form.get('next')) and is the correct path.

I am not sure what the right solution to this problem is. Perhaps the declared value should be compared with a list of the application's registered endpoints either in validate_redirect_url() or just before it's called. Or perhaps declared should be added later in the list so that the resolved endpoint will be evaluated first. URLs should return None when passed to get_url(), so while this will change precedence, it shouldn't change the outcome. Paths which contain a slash should work the same. Is there a similar API which we can model the behavior on?

As a workaround, I'm using the request's path in my decorator instead of passing the endpoint.

return redirect(url_for('security.login', next=request.path))
@jwag956
Copy link
Collaborator

jwag956 commented Sep 1, 2019

Thanks - this is definitely going to take some playing around - it's good that is is documented with a workaround.

@jwag956 jwag956 added the bug label Sep 1, 2019
@jwag956 jwag956 self-assigned this Dec 2, 2019
jwag956 added a commit that referenced this issue Dec 3, 2019
This was a regression when /login was changed from being decorated with @anonymous_user_required
to handling the already-authenticated case as part of the view.

That has been fixed. A subtle change in behavior is that it now uses
get_post_login_redirect() which will look for 'next' in the form or request params
and that will take precedence over POST_LOGIN_VIEW config setting. This actually makes
things more consistent, since it is EXACTLY the same now as the normal login process.

Add csrf errors to login_user.html - while this should never really occur in a real
application - it makes things much easier to debug.

Updated openapi.yaml.

closes: #221
closes: #172
jwag956 added a commit that referenced this issue Dec 3, 2019
This was a regression when /login was changed from being decorated with @anonymous_user_required
to handling the already-authenticated case as part of the view.

That has been fixed. A subtle change in behavior is that it now uses
get_post_login_redirect() which will look for 'next' in the form or request params
and that will take precedence over POST_LOGIN_VIEW config setting. This actually makes
things more consistent, since it is EXACTLY the same now as the normal login process.

Add csrf errors to login_user.html - while this should never really occur in a real
application - it makes things much easier to debug.

Updated openapi.yaml.

closes: #221
closes: #172
@jwag956 jwag956 closed this as completed in a28e2f9 Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants