-
Notifications
You must be signed in to change notification settings - Fork 3
Description
Proposal Date
2022-12-08
Target Ticket Acceptance Date
2022-12-23
Earliest Open edX Named Release Without This Functionality
Palm - 2023-04 (Q+ more likely)
Rationale
In LMS/Studio, we currently have a non-standard use of Django's user.is_active which often adds confusion, and cause incompatibilities with third party libraries that expect a the more typical use of user.is_active. Our non-standard muddies is_active by also possibly meaning whether or not the user's email has been activated. See the following history for more details.
Historical context:
- In the early days, there was a business rule that a user could not log-in (except initial registration login) without having verified email. The implementation was to override Django's is_active to include also looking at user's email verification status. This is done in edx-platform, and affects LMS/Studio. The default implementation of DRF's SessionAuthentication checks is_active, so APIs could also not be called by users without email verification. This implementation often confuses engineers, because it is an unexpected behavior for is_active.
- Mobile was added with new business requirements to be less strict about email verification. SessionAuthenticationAllowInactiveUser was introduced, which ignores is_active, and was added to a variety of endpoints to enable mobile to bypass the check for email verification. Since the is_active check was being bypassed, it also meant that we could not use is_active to disable user accounts. In LMS/Studio, there has been a switch to using has_usable_password to determine if an account has been disabled, which makes the platform more complicated to understand, and doesn't work across services.
- MFEs were introduced, making API calls on services protected by a newer JwtAuthentication. To simplify things, our JwtAuthentication implementation drops the is_active check, and we don't have a version of it that checks is_active (or email verification status). Many endpoints are now protected by JwtAuthentication and SessionAuthentication in this order. See https://github.com/openedx/edx-platform/search?q=SessionAuthentication. This means email verification will not be checked if the user authenticates with a JWT, but it will be checked if they have a session, but not a JWT. This adds to the confusion. See fix: allow inactive (email unverified users) to add and refund entitlements openedx-platform#31267 for a sample fix for this issue to a single endpoint.
- (This needs to be confirmed by the Product-WG.) The original business rule was relaxed, and now login is allowed on certain deployments, even without email verification. However, API endpoints using SessionAuthentication continue to block access.
Removal
- No longer set is_active to True based on email verification.
- See source.
- Also set for secondary email. See [source](https://github.com/openedx/edx-platform
/blob/a9525fb1d355407123c247d26b55cd243d376c49/common/djangoapps/student/models.py#L3394). - Work required to understand the official business rules, and separate account activation and email verification.
- Remove DRF auth classes that bypass the is_active check, like SessionAuthenticationAllowInactiveUser.
Replacement
- Add a method (if it doesn't already exist) to explicitly check email verification.
- Note: This may be able to use the
activation_timestamp? - See comments for other implementation ideas.
- Note: This may be able to use the
- Possibly add a DRF permission class to edx-platform, IsEmailActivated, if any APIs require this check.
- Any use of
SessionAuthenticationAllowInactiveUserwould be replaced bySessionAuthentication. (Same with BearerAuthentication classes, but BearerAuthentication should have its own DEPR ticket.) - Update features requiring email verification to be explicit, rather than implicit and buried into is_active, which has another meaning in Django.
- Remove toggles related to other email verification business requirements. Can we get a standard agreement? If not, any toggles and rules will need to be more explicit.
Deprecation
The main function affected, is_active, will not actually be deprecated, but will be changing behavior.
Some classes like SessionAuthenticationAllowInactiveUser would be deprecated, and could be marked as such.
Migration
Needs planning:
- Can we use
activation_timestampas-is, or do we need to back-fill? - Detailed plan would be needed for how and when is_active will be updated, and when and if to sync with
has_unusable_passwordand other disabling capabilities. In theory, everyone could temporarily be set to is_active = True once we have replacement explicit checks for email/account activation that is separate.
Additional Info
Once is_active is no longer tied to email/account activation, this would open up the following possibilities:
- Restore
is_activeto the original Django meaning of this field, where it can be set to false to disable (soft delete) user accounts. This might require changes wherehas_usable_passwordwas being used for disabled accounts. - Add the
is_activecheck to JwtAuthentication, which would now be limited to checking if an account were disabled.
Other Thoughts:
- It may be a stretch to call all of this DEPR, but the DEPR process still seems like a good enough fit.
- Worst case, even if we don't move forward with this, I think this history needed to be captured somewhere.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status