[WIP] Login/Signup with OIDC #2460
Draft
+3,280
−273
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit is rather monolithic. An attempt to split it up into smaller
changes proved too difficult (as well as frustrating) and I realized it
would absolutely increase the chance of having a broken commit (making
bisects more difficult) unless a lot of testing effort between commits
was performed, which would have very little benefit.
The changes in this commit include:
- Outcome utility used by SignupService for error handling
- SignupService, whichs implements re-usable create_user function
- Signup method in OIDCService
- flow-specific callbacks in OIDC (separates login from signup)
- **SEPARATE SESSION COOKIE AND GUI COOKIE**
- this change "rocks the boat" the most and has the highest likelihood
of causing problems
When users make sensitive changes to their account they are asked to re-enter their password. This prevents a hijacked session from causing futher damage. Users created with the new OIDC flow do not necessarily have a password set on their account, and they need to also be able to make these changes. While removal of the password entry requirement for these users would solve this problem, it would also make their accounts more vulnerable. To solve this problem while maintaining the same security standard for OIDC users, we need them to confirm via either 2FA or re-authentication via OIDC. Since users aren't required to have 2FA, the re-authentication via OIDC approach is also the minimum viable solution. This commit adds OIDC re-authentication support for all endpoints under UserProtectedEndpointsService, and makes updates to the UIWindowChangeUsername dialog for manual testing. Currently this implementation fails at the final submission to change the username because of a separate issue with the correct authentication token not being set; this is related to the separation of GUI tokens vs http-only tokens.
KernelDeimos
force-pushed
the
eric/262A0_PUT-453
branch
from
5a3cac6 to
e53a9fe
Compare
The monthly number of username changes was hardcoded as `2`. Being able to configure this value makes it easier to test the username change flow. Hosters of OSS Puter may also find this configuration beneficial.
The OIDC re-authentication flow, which replaces password confirmation for accounts that were created with OIDC and do not have a password, was previously added to "change username" for manual testing of the backend-side implementation. Add the re-authentication flow to the remaining user-protected endpoints, which are: - change password - change email - disable two-factor authentication When using "change password" on a new account created via OIDC, the account changes state to a passworded account which causes these flows to use password confirmation as before instead of re-authentication.
There is common functionality between all of the GUI code for actions on protected endpoints. Update UIWindowChangeEmail and UIWindowChangeUsername to both use a new utility function called openRevalidatePopup in util/openid.js. This file is called `openid.js` instead of `oidc.js` so that it's more easily recognized by contributors who might be more familiar with the name of the organization than the name of the standard itself. After these changes, UIWindowChangePassword and the "disable 2FA" button in UITabSecurity still need to be updated to use `util/openid.js` instead of duplicating this functionality. The justification for following DRY here instead of leaving the implementation as-is is because these flows are particularly error prone and will be difficult to maintain without this consistency. Some subtle bugs I previously wasn't aware of got fixed in the process.
Use the openRevalidatePopup function in util/openid.js within UIWindowChangePassword instead of re-implementing that functionality. Additionally, normalize some of the code so it is more similar to UIWindowChangeUsername and UIWindowChangePassword.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These changes implement login and signup for OIDC flows, as well as consequential changes because of how these changes disrupt other parts of authentication, such as: