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

Employment application requires Django login #2074

Closed
stellanl opened this issue Apr 1, 2016 · 5 comments
Closed

Employment application requires Django login #2074

stellanl opened this issue Apr 1, 2016 · 5 comments
Assignees
Milestone

Comments

@stellanl
Copy link
Contributor

stellanl commented Apr 1, 2016

Test plan

GIVEN the /rest/v1/user/../request_organisation/ endpoint
WHEN POSTing a request from outside of RSR
AND the correct authentication details are provided
THEN the request should be process accordingly, instead trying to authenticate through Django's auth system

Issue description

Just as for #2037, when posting a request for employment to the /rest/v1/user/request_organisation/ endpoint, a Django login is required, which is not the case for the rest of the API; an API key should be sufficient.

@stellanl
Copy link
Contributor Author

This does not seem to work right. Looks like access is checked against anonymous user when not logged into Django; the API key is never checked.

@stellanl
Copy link
Contributor Author

Interestingly, in project_update.py, for POST, there is a @permission_classes((IsAuthenticated, )) and that endpoint works from Up. Maybe some of the API key logic I just committed could be used there, too.

zzgvh added a commit that referenced this issue Apr 19, 2016
…ent for Up

Adding the @authentication_classes decorator sets request.user to the
owner of the auth token, thus we can compare request.user with the user
supplied by the PK.
@zzgvh
Copy link
Contributor

zzgvh commented Apr 19, 2016

I think we should review the @api_view(['POST']) endpoints, and add the @authentication_classes decorator to eliminate unauthenticated access. I think the enpoints used by Up should have
@authentication_classes([TastyTokenAuthentication, SessionAuthentication]) and the others only @authentication_classes([SessionAuthentication]).

If I understand correctly, valid authentication sets request.user to the authenticated user, as seen in the commit above, so it becomes very easy to know who's calling. But that brings me to wonder if the PK of the user is needed?

@stellanl
Copy link
Contributor Author

How are the authentication classes combined? With OR or AND?

@zzgvh
Copy link
Contributor

zzgvh commented Apr 20, 2016

I think they are tested against in order until you're either authenticated or out of methods.

zzgvh added a commit that referenced this issue Apr 21, 2016
Add SessionAuthentication to the request_organisation() endpoint
authentication_classes decorator.
KasperBrandt added a commit that referenced this issue Apr 21, 2016
@KasperBrandt KasperBrandt added this to the 3.13 Quito milestone Apr 21, 2016
@KasperBrandt KasperBrandt removed this from the 3.12 Pretoria milestone Apr 21, 2016
zzgvh added a commit that referenced this issue Apr 21, 2016
Adding a TODO that the response should be HTTP 201 Created, but I'm not
adding the actual code since it seems to break the usage of the view in
myRSR.
@MichaelAkvo MichaelAkvo moved this to Done in RSR Dec 8, 2022
@MichaelAkvo MichaelAkvo added this to RSR Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

4 participants