This is more correct and may reduce the likelihood of perpetuating a bad
mixed-auth state.
In general, we should probably be modifying session and JWT cookies in
sync at all times, never individually. This specific code probably won't
make anything worse, but a clean reset might improve user experience in
the rare cases where someone somehow gets their browser into a weird
state.
- Switch from `response.set_cookie` with past expiry to just using the
`response.delete_cookie` method.
- Docstring improvements.
ref: ARCHBOM-2030 (internal)
Change `mark_user_change_as_expected` to no longer take the response object
and instead convey the expected-change information via RequestCache.
This requires edx-django-utils 4.4.2, which fixes the bug where
RequestCache was cleared in the exception phase.
Also, no longer mark `ENFORCE_SAFE_SESSIONS` toggle as
temporary. We'll want it as an opt-out.
I was tempted to take this opportunity to move any existing
`mark_user_change_as_expected` calls to be closer to where the actual
change request.user occurs, reducing risk of both false positives and false
negatives, but it would be better to do that one at a time in case a move
breaks something. (Ideally it would be called right after any
`django.contrib.auth` `login` or `logout` call; previously, we were
constrained by having to make the call after a response object had been
created.) These changes can be made later if it becomes necessary.
Using a single query to get a user using both username and email fields
generates a massive `key_len` and causes DB overload. Separated these
lookups into two separate queries.
VAN-819
* chore: update deprecated import from collections
* chore: remove outdated imports from markdown library
as it hasn't been supported since 2.0.3 and we're on 3.x.
This was deprecated at least as early as 2012!
* docs: add docstring and remove lint-amnesty to markdown plugin
* chore: remove deprecated etree import
* style: remove unnecessary-comprehension for sets
* style: resolve a number of amnestied pylint complaints
Co-authored-by: stvn <stvn@mit.edu>
Includes:
- general documentation
- links to individual events definitions and location
- adding examples to events docs
- adding annotations at the trigger location
This is intended to silence a rare false positive that seems to happen
when someone logs in on a browser that already has an active session
for another user. We believe there should be no further positives once
this case is handled.
- login and logout views annotate the response to indicate the session
user should be changing between the request and response phases
- safe-sessions middleware skips the verify-user check when this
annotation is present
Also:
- Adds a test around existing behavior for unexpected user-changes
- Remove logging control based on `is_from_log_out`. This reverts most
of af9e26f/PR #11479 for two reasons:
- The safe-sessions `_verify_user` code has since changed to check for
`request.user.id == None`
- A commit later in the PR changes the login and logout pages to
signal that the user/session change is expected
This changes the "Sign out" link on Studio to point to Studio's own logout
view, which clears the session and then redirects to LMS's logout page. The
LMS logout page then skips loading the Studio logout because it is seen in
the Referer header.
This change also brings Studio better into line with how other IDAs perform
their logouts.
Background:
After the rollout of Studio OAuth, logouts initiated on Studio failed to
actually log out Studio (but all other IDAs were logged out). This was
because the LMS logout view loads the logout pages of other IDAs but skips
any that is a *prefix* match on the Referer header, and browsers now often
send a truncated version of the Referer for privacy. Therefore, Studio was
always skipped when coming from Studio.
The fix is to make sure that Studio has already performed its logout by the
time the LMS logout page is loaded.
One wrinkle here is that the LMS logout view is activated by `/logout`, but
the correct logout view (provided by auth_backends) is activated by
`/logout/` -- with a trailing slash. This is fragile and unfortunate, but
can be cleaned up when we later remove other leftovers of Studio's previous
ability to handle logistration.
ref: ARCHBOM-1897
