Skip to content

Conversation

venkatesh-sivaraman
Copy link
Collaborator

Per a user's feedback email, instead of setting the student's name only once when the user first logs in, update it each time we handle an OAuth login. (I'm not 100% sure this will work as I can't remember if the student name is always sent by Touchstone, but should be good to test out.)

@gabrc52
Copy link

gabrc52 commented Jul 3, 2023

I think Shibboleth does send it every time: https://fireroad.mit.edu/Shibboleth.sso/Session

@gabrc52 gabrc52 mentioned this pull request Jul 3, 2023
@psvenk
Copy link
Member

psvenk commented Jul 11, 2023

I'm not 100% sure this will work as I can't remember if the student name is always sent by Touchstone, but should be good to test out.

It looks like we already assume that the student name is always sent by Touchstone:
https://github.com/venkatesh-sivaraman/fireroad-server/blob/3930bd2bc75b38458efd0912b33ae5b3578e8713/common/views.py#L85-L94

This is assuming that the return value of login_touchstone is checked somewhere and this does indeed log an error somewhere. (I'm not sure if the return should have been raise; I see both return HttpResponse(...) and raise HttpResponse(...) used further down in the function, both for 403 responses.)

Also, @venkatesh-sivaraman could you point me to where exactly the request.META is coming from? I assume that it is being picked up from Shibboleth somehow, but I couldn't find this in the code. (And I assume that login_touchstone has effectively supplanted login_oauth in production, given the redirect in urls.py?)

@gabrc52
Copy link

gabrc52 commented Jul 11, 2023

I think request.META is a sort of equivalent to the $_SERVER variable in PHP (for variables set by the web server plugin, in this case Shibboleth). I encountered it when implementing cert authentication for Django (email = request.META['SSL_CLIENT_S_DN_Email'] on scripts), but I didn't find it documented anywhere at that time.

At least, the PHP sample in IS&T's website uses $_SERVER: https://wikis.mit.edu/confluence/display/TOUCHSTONE/Sample+Source+Code+-+The+SAML+Assertion

@dtemkin1
Copy link
Collaborator

dtemkin1 commented Jun 13, 2025

I'm going through old issues/pr's, can this be merged? @psvenk

Also should we be using Petrock/Okta directly now instead of the defunct OIDC...? Idk if it matters or not but something to think about ig

@gabrc52
Copy link

gabrc52 commented Jun 14, 2025

I don't think this uses the defunct OIDC?

@dtemkin1
Copy link
Collaborator

Don't we? oauh_client.py still seems to point to it

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

Successfully merging this pull request may close these issues.

4 participants