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

JWT cookie sometimes contains a different user than the session #381

Open
robrap opened this issue Sep 8, 2023 · 3 comments
Open

JWT cookie sometimes contains a different user than the session #381

robrap opened this issue Sep 8, 2023 · 3 comments

Comments

@robrap
Copy link
Contributor

robrap commented Sep 8, 2023

Using edx-platform Safe Session monitoring, we've seen cases where the user id related to the LMS session does not match the user id in the JWT cookie. Although this issue is ultimately related to the LMS, which creates both the LMS session and the JWT cookie, this issue is being documented in this repo because this is where the JwtAuthentication class lives, which is a good place for adding observability, and potentially for taking corrective action.

  • The first step is simply to add additional observability.
  • Once we better understand the issue, we can determine what the best corrective action might be. Do we simply go with the JWT even though there there is disagreement on the user id? Do we kill the session and the cookie? Should we simply return an error from JwtAuthenticationl?

Here are some related tickets:

@robrap
Copy link
Contributor Author

robrap commented Sep 21, 2023

@feanil: Some ideas we had discussed:

Observability for user id mismatches

  • Get session_user_id from request.user at start of JWT authentication (which I think comes from temporary middleware authentication before DRF authentication).
  • Get jwt_user_id from successful JWT authentication, or
  • Get unverified_jwt_user_id from unsuccessful JWT authentication (requires decode without verifications).
  • Potential custom attributes:
    • jwt_auth_and_session_user_mismatch; True or NULL (not triggered when False).
    • jwt_auth_session_user_id; Only included when mismatch is True.
    • invalid_jwt_cookie_user_id; Only included when mismatch is True (really?) AND there is a JWT cookie auth failure.

Error handing for mismatches

For auth failure that would have resulted in jwt_auth_result of 'forgiven-failure', we will instead fail if there is a mismatch and use jwt_auth_result of ‘user-mismatch-failure’ (or something like that).

For all other cases, we discussed not doing anything new or different regarding mismatches, and simply starting with observability.

Todos for @robrap:

  • Move the safe session related bugs to github, because these might affect the mismatches.
  • Once the observability improvements land, I can update 2U's safe session runbook with this additional help.

@robrap
Copy link
Contributor Author

robrap commented Sep 21, 2023

[inform] @feanil: I moved the two known issues to github issues and added references to this ticket above. 2U does not plan to prioritize these, but they would be helpful: 1) as part of auth clean-up, and 2) to enable making safe session alerting more sensitive.

@robrap
Copy link
Contributor Author

robrap commented Oct 3, 2023

@feanil: When you return to this work, I'm wondering if this existing test_set_request_user_with_use_jwt_cookie has a good chunk of what you would need, and could be duplicated/simplified for this work?

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

1 participant