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

Support for AJAX requests? #6

Open
tarkatronic opened this issue Oct 20, 2015 · 4 comments
Open

Support for AJAX requests? #6

tarkatronic opened this issue Oct 20, 2015 · 4 comments

Comments

@tarkatronic
Copy link

I have a case where I would like to use sudo access for a REST API, but it doesn't appear that this will work nicely with this framework currently.

What I would propose is basically an addition to the sudo_required decorator:

if request.is_ajax():
    return HttpResponseForbidden(json.dumps({'message': 'Forbidden', 'sudo_required': True}), content_type='application/json')

There would also need to be a view which could receive an AJAX payload for authentication and return the appropriate cookie.

Is this something which would be in the scope of this project, or would it be more suited to something like a django-rest-framework-sudo package?

@mattrobenolt
Copy link
Owner

So this is almost exactly what we do in Sentry.

See: https://github.com/getsentry/sentry/blob/master/src/sentry/api/decorators.py

We then wrap @sudo_required around our DRF views. You can see this being used here: https://github.com/getsentry/sentry/blob/930cbca0e99db519edca117580680c5005b17b01/src/sentry/api/endpoints/user_details.py#L26

I'm open to suggestions here. :) But this is how we use it ourselves.

@tarkatronic
Copy link
Author

Oh! Nifty! So what I'm hearing is, this could potentially be rolled in here. I like that.

So then all that would be remaining would be a view to accept the credentials, right? Or... perhaps just rig up views.sudo with its own AJAX detection? I suppose either way would work, I'm just not sure which would have less code smell.

Perhaps this even warrants a slight abstraction and providing separate @ajax_sudo_required decorator and @ajax_sudo view?

One thing I'm curious about in the Sentry implementation, it's using a 401 Unauthorized response code. From my understanding of that code, it "must" provide a "WWW-Authenticate" header in the response, whereas 403 Forbidden would indicate that current credentials are insufficient. Of course, this is purely semantics and irrelevant to it actually working. I was just curious about the decision to use 401.

@mattrobenolt
Copy link
Owner

You're probably right about the WWW-Authenticate part. :)

Not sure if I agree with implementing something in here to facilitate this though. From my perspective, bridging into JavaScript land requires tighter coordination and is very dependent on the implementation.

So not sure what your idea is, but I'd rather provide some primitives for helping out here than trying to make an end-to-end solution. I think it'd be appropriate to have a say, react-sudo module or something that implemented a pattern.

I'm not sure the best path, but this is also a reason why we don't fully have this integrated in Sentry. We don't do this for our React stuff yet.

So again, I'm open to a proposal, but I'm not sure of the best direction myself that's generic enough as primitives.

@tarkatronic
Copy link
Author

I certainly wouldn't propose including javascript in here; just a couple of endpoints that return JSON as opposed to redirects & html.

What I would see is something to facilitate this flow:

Request:
    GET /sensitive_page/

Response:
    Code: 403
    Data: {'error': 'Account verification required.',
           'sudoRequired': True,
           'username': 'someuser',
           'auth_endpoint': settings.SUDO_AJAX_ENDPOINT}

Request:
    POST /api/sudo_auth/
    password=supersecurepassword

Response:
    Code: 204
    Cookie: sudoauthcookiehere

Request:
    GET /sensitive_page/

Response:
    Code: 200
    ...

What would you think about something like that? Perhaps still out of scope?

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