Skip to content

fix(settings): hide profile 2FA and delete account when AUTH_TYPE=SSO#15

Merged
jawad-khan merged 2 commits into
foss-sandboxfrom
jawad/hide-delete-or-mfa-option
May 22, 2026
Merged

fix(settings): hide profile 2FA and delete account when AUTH_TYPE=SSO#15
jawad-khan merged 2 commits into
foss-sandboxfrom
jawad/hide-delete-or-mfa-option

Conversation

@jawad-khan
Copy link
Copy Markdown
Collaborator

Summary

  • Hide the Two Factor Authentication section on the Profile settings page when AUTH_TYPE=SSO.
  • Hide the Danger zone (delete account / leave workspace) section under the same condition.
  • Extract profile 2FA UI into ProfileTwoFactorAuthenticationSection, following the same outer/inner SSO guard pattern used by Toggle2FA and SettingsTwoFactorAuthenticationMethod.

Why

In SSO deployments, MFA and account lifecycle are owned by the identity provider. Showing Twenty's local TOTP setup and self-service account deletion on Profile is misleading — those controls don't participate in the actual login or provisioning flow.

This aligns Profile with existing SSO behavior:

  • Email is already read-only under SSO
  • Password change is already hidden under SSO
  • Workspace-level 2FA enforcement toggle is already hidden under SSO
  • Direct navigation to the 2FA setup page already redirects to Profile under SSO

Test plan

  • With AUTH_TYPE=SSO, open Settings → Profile and confirm Two Factor Authentication and Danger zone are not shown
  • With AUTH_TYPE unset or non-SSO, confirm both sections still appear
  • With AUTH_TYPE=SSO, navigate directly to /settings/two-factor-authentication/totp and confirm redirect to Profile still works

@jawad-khan
Copy link
Copy Markdown
Collaborator Author

@copilot Review.

Copy link
Copy Markdown

Copilot AI commented May 21, 2026

@copilot Review.

Code Review Summary

The implementation is clean and correct. Here's my analysis:

✅ What looks good:

  1. Outer/inner guard patternProfileTwoFactorAuthenticationSection correctly follows the same pattern used by Toggle2FA and SettingsTwoFactorAuthenticationMethod. The outer component calls useIsSsoEnabled() and bails out before inner hooks (like useCurrentUserWorkspaceTwoFactorAuthentication) run — preventing hook-order issues and unnecessary Apollo/context requirements.

  2. SSO guard on DeleteAccount in SettingsProfile.tsx via !isSsoEnabled is minimal and correct.

  3. Cleanup – Unused imports (SettingsCard, IconShield, Status, UndecoratedLink, useCurrentUserWorkspaceTwoFactorAuthentication) are properly removed from SettingsProfile.tsx.

  4. Comments explain WHY (IdP owns MFA, dead controls), consistent with existing codebase style.

  5. Test covers the key invariant: SSO enabled → null returned, inner hook never invoked.

⚠️ Minor observations (non-blocking):

  • The test only covers the SSO-on branch (the SSO-off path requires providers and is noted as out of scope — acceptable for a unit test of this pattern).
  • Import order in SettingsProfile.tsx: useIsSsoEnabled from @/auth/hooks is inserted after currentWorkspaceMemberState from @/auth/states. Alphabetically hooks comes before states, so it's slightly inconsistent, but this is cosmetic.

CI status: All functional CI checks (CI Front, CI Server, CI Shared, etc.) pass. The two failures (Preview Environment Dispatch, PR Review Dispatch) are infrastructure/deployment workflows, not test failures, and are unrelated to these changes.

Overall the implementation is solid — no functional issues found. ✅

@jawad-khan jawad-khan merged commit 3541ed4 into foss-sandbox May 22, 2026
67 of 75 checks passed
@github-actions
Copy link
Copy Markdown

Fails
🚫

node failed.

Log

Details
�[31mError: �[39m SyntaxError: Unexpected token '<', "<!DOCTYPE "... is not valid JSON
    at JSON.parse (<anonymous>)
�[90m    at parseJSONFromBytes (node:internal/deps/undici/undici:4319:19)�[39m
�[90m    at successSteps (node:internal/deps/undici/undici:6967:27)�[39m
�[90m    at readAllBytes (node:internal/deps/undici/undici:5890:13)�[39m
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:104:5)�[39m
danger-results://tmp/danger-results-fb7b2293.json

Generated by 🚫 dangerJS against a7aa518

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants