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

Handle POST requests differently (ie don't "forget" them during sudo-auth step) #19

Open
hjwp opened this issue Jul 11, 2017 · 5 comments

Comments

@hjwp
Copy link

hjwp commented Jul 11, 2017

One slightly unfortunate feature of the current implementation is that POST requests just get swallowed/ignored by the interposition of the sudo authentication page.

Use case: a user can load a copy of a "secure" page, and keep it open in a browser tab for long enough that their sudo cookie expires (maybe they loaded it 2hrs59 mins after login, or maybe they just left the tab open for a long time). By the time they submit the form on that page, they are asked to re-authenticate, and on success they are redirected back to where they were, but the post request is forgotten.

Ideally there would be some magic for "saving" the post request, and then re-submitting it on successful auth. That might be a little tricky to implement mind you. A second-best / fallback would be to at least be able to display a message to the user saying they'll have to re-submit the request...

@hjwp
Copy link
Author

hjwp commented Jul 11, 2017

The ideal solution sounds like a fair bit of work, so we're thinking of going for the second-best, ie, noticing when we've intercepted a POST request, and warning the user that they need to re-submit.

We're thinking of implementing it using a flag called, eg sudo_post_intercepted on the session, and we think we can do it all by overriding the @sudo_required decorator:

# pseudocode in comments
def sudo_required(func):

    @wraps(func)
    def inner(request, *args, **kwargs):
        if not request.is_sudo():
            # we have intercepted a request that requires sudo
            # if POST, set sudo_post_intercepted flag
            # if GET, unset flag
            return redirect_to_sudo(request.get_full_path())

        # else:
        #  either the user already had sudo, and we are just passing straight through, 
        #  or the user just acquired sudo, and we are redirecting them
        #  back to where they were.
        #
        #  if we spot the was_post flag (which should only happen in redirect case),
        #  we should add a messages.warning "please resubmit your request"
        #  in either case, we un-set the sudo_post_intercepted flag.
        return func(request, *args, **kwargs)
    return inner

what does everyone think?

@hjwp
Copy link
Author

hjwp commented Jul 11, 2017

less pseudocodey:

def sudo_required(func):
    @wraps(func)
    def inner(request, *args, **kwargs):
        if not request.is_sudo():
            if request.method == 'POST':
                request.session['sudo_post_intercepted'] = True
            else:
                request.session['sudo_post_intercepted'] = False
            return redirect_to_sudo(request.get_full_path())

        if request.session.get('sudo_post_intercepted'):
            messages.warning(request, 'Authentication confirmed.  You will need to re-submit your request')
        request.session['sudo_post_intercepted'] = False
        return func(request, *args, **kwargs)
    return inner

@luzfcb
Copy link

luzfcb commented Jul 11, 2017

Ideally there would be some magic for "saving" the post request, and then re-submitting it on successful auth

SessionWizardView of django-formtools, Implements a similar functionality to persist data between steps

the data is removed in a GET request https://github.com/django/django-formtools/blob/master/formtools/wizard/views.py#L262 or after final step successful done.

@hjwp
Copy link
Author

hjwp commented Jul 11, 2017

@luzfcb that sounds like it could work for the ultimate magic solution. presumably that would mean totally rewriting sudo.views.SudoView tho? that's above my paygrade i think...

@mattrobenolt
Copy link
Owner

I would much rather this work with some sort of warning. I don’t think it’s django-sudos responsibility to intercept POST data and handle this gracefully. That’d be almost impossible to do correctly from this library’s perspective. And if you can implement some warning behavior, this should 100% be opt-in.

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

3 participants