-
Notifications
You must be signed in to change notification settings - Fork 153
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
docs: ADR to enable 2FA for course team accounts #574
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportBase: 76.93% // Head: 80.57% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #574 +/- ##
==========================================
+ Coverage 76.93% 80.57% +3.63%
==========================================
Files 88 87 -1
Lines 1834 1889 +55
Branches 492 531 +39
==========================================
+ Hits 1411 1522 +111
+ Misses 405 351 -54
+ Partials 18 16 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
****************************** | ||
|
||
- We could reduce session expiration time only for course team users. Currently session expiration time is four weeks and we can reduce this time to two weeks for educator accounts. | ||
- We can also go with the first (2FA) and third (Reset password after a specific time period) option simultaneously. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that the current best practice is to not reset passwords on a cadence, since that makes people more likely to pick simple passwords. (They should be using a password manager, but we can't force them to.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We enabled HIBP compliance on password reset and registration pages hence they cannot set weak passwords. Do you still think we should not ask them to reset password?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strictly speaking, we can't keep people from setting weak passwords -- we can only prevent them from using specific known-to-be-weak passwords. If superman1
is disallowed, they might choose superm@n1
instead, which isn't much of an improvement.
HIBP will definitely help, but I do think that we should avoid doing password resets except in cases of known compromise.
Enhancements to above approach | ||
****************************** | ||
|
||
- We could reduce session expiration time only for course team users. Currently session expiration time is four weeks and we can reduce this time to two weeks for educator accounts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two weeks doesn't make a big difference compared to four weeks if the course team user's account is compromised somehow. Should it be daily or at least has a configurable period for session expiration time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- We can make it configurable.
- The only issue I see with making the session expiry time to 1 day would be that course team are normally making changes that might span over a few days and they might lose those edit if they don't save them as draft.
This change can be accommodated within our current login flow. A high level flow of this process is as follow: | ||
|
||
- A user belonging to a course team will authenticate themselves like they currently do, either by providing login credentials or by SSO. | ||
- Once they have authenticated successfully they will be sent an automatically generated authentication code to their primary email address. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you looked into other kinds of 2FA?
Email-based 2FA protects users in the situation where their Open edX password is compromised but their email account remains secure. This will be true for some users, but for others the passwords will unfortunately be the same in the two systems. Password re-use is tremendously common. For those people, email isn't truly a second factor, since they use the same password for two services (from the same device, even).
I think you can look at 2FA a few different ways, although some of these overlap:
- Whether it protects against a password compromise due to A) reuse/just being weak, B) phishing (can get one TOTP token), or C) on-device malware (can capture full TOTP secret, if stored on same device)
- Whether the password is reused for the second factor (e.g. securing the email account too)
- Whether the password is entered on the same device where the second factor is secured (malware or a thief gets both in one go)
From that perspective, here's how I see some of the options:
- Email token: Vulnerable if password is reused for email service, or if password was compromised via on-device malware, or in phishing. edX password is already equivalent in security to email account access due to reset-password functionality, so this is marginal as 2FA goes. Works OK if attack is based on password reuse but email account is secure.
- SMS token: Vulnerable to targeted attacks, either social engineering (number porting) or technical (cloning), and to phishing. Probably OK against non-targeted attacks. Requires a cell phone.
- TOTP: Vulnerable if login to edX also happens on the phone where the TOTP app is installed and malware is the attack method. (Unlikely for primarily-Studio users, much more likely for other sites.) Also vulnerable to phishing. Pretty secure otherwise, and doesn't require a phone—Yubikey can be used for TOTP, and in a pinch a desktop client can be used if the TOTP code is displayed in text under the QR code to allow manual enrollment (but this reduces the security in the malware case.)
- Various apps like Duo that involve pressing a "confirm" button on the phone rather than typing in a code; probably not any better than TOTP, and require custom integrations. Not really recommended.
- U2F: Vulnerable only to physical theft of hardware token—very secure. Even prevents phishing. But most people don't have these.
I think the email token is a good first step, but I'd like to see us support TOTP and U2F as the "preferred" methods (plus a "backup codes" feature), with SMS and email only provided as fallbacks when a user can't use the preferred methods. I suspect there's already a library that can do a good amount of this for us. Maybe I'm wrong, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@timmc-edx thank you for these suggestions. We are going to do more discovery and update ADR accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep in mind the support needs of different 2FA methods during discovery. I could imagine course staff needing help with installing 3rd party apps, with account recovery, with a change in phone access or email address, with university spam filters. Some of these are familiar support flows and some may need new tools/workflows.
|
||
This change can be accommodated within our current login flow. A high level flow of this process is as follow: | ||
|
||
- A user belonging to a course team will authenticate themselves like they currently do, either by providing login credentials or by SSO. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a definition for the roles that would be appropriate for 2FA, and would that affect the best kind of 2FA for the platform?
As a user/support person, off the top of my head:
- course staff, administrator, and course data researcher roles have high levels of access to sensitive data/tools
- course discussion roles are orthogonal to the above, and give some access to discussion data only
- beta testers do not have access to learner data, just early access to content
- a couple roles may exist that are affiliated with an organization rather than a course ("universal course researcher")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, we are doing 2FA for course teams and I don't think it would have any effect on the best Kinds of 2FA for the platform, we have just updated the document, please have a look. Thankyou
f7e2555
to
416c010
Compare
416c010
to
a1aabc0
Compare
- Can use same U2F device for multiple sites | ||
- - Needs a device | ||
- Cost | ||
- Support (only chrome supports U2F currently) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Support is broader than that -- Firefox supports U2F as well. (But agreed that it can't be the only supported method, so probably not something we want to start with.)
This ADR describes the decision and process of adding a new security step for course team accounts. One decision is highlighted within the doc that needs review. Any possible addition or suggestion is welcome.
Ticket: https://2u-internal.atlassian.net/browse/VAN-942