feat: admin-set 'require 2fa' override for key staff#286
Open
leowilkin wants to merge 1 commit into
Open
Conversation
Adds a two_factor_required boolean override on identities, editable from the backend overrides section. Once set: - if the user has a 2FA method, every login demands a second factor (via the existing LoginAttempt required-factors machinery) - if they don't, they can log in with an email code but every request (including /oauth/authorize and SAML) redirects to /security until they enroll a TOTP or passkey - the user can't disable the 2FA requirement or remove their last method (toggle_2fa and step-up completion paths refuse) Known limitation, flagged for follow-up: previously issued OAuth access tokens keep working (API layer isn't gated); consider revoking tokens when the override is enabled.
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
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.
what
adds a require 2fa override to the backend identity overrides section (next to
hq_override/disallow slack). it's for key staff who use HCA to sign into sensitive apps but aren't HCA admins — admins are already forced onto TOTP/passkey byBackend::ApplicationController#require_2fa!, but nothing covered this group.how it works
new
two_factor_requiredboolean on identities (normal-admin editable; goes through the existing reason +admin_updateaudit flow).user already has a 2FA method →
Identity#requires_two_factor?now also honors the override, so the existingLoginAttemptmachinery demands a second factor on every login. the user can no longer disable the requirement or remove their last method —toggle_2faand the step-updisable_2fa/remove_totp/remove_passkeycompletions all refuse (removing one method while another remains is still allowed). the security page shows "required by Hack Club HQ" instead of the disable button.user has no 2FA method yet (the chicken-and-egg case — you can't enroll without a session) → the email-code login completes, but a new
ApplicationControllergate (require_two_factor_enrollment!) redirects every request to/securityuntil they enroll. crucially this includes/oauth/authorize(doorkeeper'sbase_controllerisApplicationController) and SAML, so they can't reach sensitive apps un-enrolled. exemptions are limited to the security page and its htmx panels (totp/passkey/backup-code/sessions/authorized-apps), login, step-up, and logout. htmx requests get anHX-Redirectheader instead of a 302 so the panels don't loop.flipping the override takes effect immediately for existing sessions (the gate runs per-request).
testing
/and/oauth/authorize, exempt paths don't loop, gate lifts once a TOTP is verified, unaffected users pass through; all disable/removal refusal paths; last-method vs not-last-method removalrequires_two_factor?/two_factor_enrollment_required?truth tables/securitywith the enrollment banner → enroll TOTP → next login demands TOTP → disable refusedscreenshots