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

feat: always allow twofactor apps #1278

Merged
merged 1 commit into from
Jan 7, 2025
Merged

Conversation

st3iny
Copy link
Member

@st3iny st3iny commented Jan 6, 2025

Ref #149 (comment)
Ref #1249

Problem

A guest user will not be able to set up their second factor authentication method by default, e.g. TOTP and Webauthn. Admins have to specifically add the corresponding twofactor_* apps to the white list first.

Instead, attempting to enable, disable a second factor method or generating backup codes as a user will yield a cryptic internal server error response with no context. However, if a method is set up and the app is removed from the white list, the login will still work.

How to reproduce?

  1. Create a guest user.
  2. Install at least one twofactor app, e.g. twofactor_totp.
  3. Enable the app white list for guest users and remove twofactor_totp from it.
  4. Log in as the guest user.
  5. Try to set up TOTP.

Observe 500 Internal Server Error responses.

Solution

Allow supported twofactor apps by default:

  • twofactor_webauthn
  • twofactor_totp
  • twofactor_backupcodes
  • twofactor_nextcloud_notifications

Signed-off-by: Richard Steinmetz <[email protected]>
@st3iny st3iny added enhancement New feature or request 3. to review Waiting for reviews labels Jan 6, 2025
@st3iny st3iny self-assigned this Jan 6, 2025
@st3iny st3iny requested review from icewind1991 and skjnldsv January 6, 2025 08:21
@skjnldsv skjnldsv merged commit 22d2871 into master Jan 7, 2025
37 of 41 checks passed
@skjnldsv skjnldsv deleted the feat/always-allow-twofactor-apps branch January 7, 2025 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants