Allow empty groups for reverse proxy sso auth

[heisbrot]

Summary by CodeRabbit

Release Notes

• New Features & Improvements
  • SSO authentication modal now displays current authentication state for better visibility
  • Enhanced group selector UI with visual "All Users" badge and improved placeholder display
  • Ability to save SSO configuration without requiring group selection
  • Updated button label from "Add Groups" to "Add SSO" for clarity

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR extends PeerGroupSelector's placeholder capability to accept React nodes alongside strings, refines the reverse proxy authentication modal by exposing bearer authentication state, and enhances AuthSSOModal with integrated user data loading and improved group selection handling.

Changes

Cohort / File(s)	Summary
PeerGroupSelector Enhancement src/components/PeerGroupSelector.tsx	Expanded placeholder prop type to support React.ReactNode alongside string values; adjusted conditional padding to apply only for string placeholders, enabling rich React components as placeholders.
Reverse Proxy Authentication Flow src/modules/reverse-proxy/ReverseProxyModal.tsx, src/modules/reverse-proxy/auth/AuthSSOModal.tsx	Added isEnabled prop to expose bearer authentication state to AuthSSOModal; integrated useUsers hook for user data loading; refactored isEditing logic to rely on isEnabled state; updated Save button to always close modal and invoke onSave; enhanced placeholder with Badge and CircleUser UI elements to display "All Users" indicator; renamed button text to "Add SSO".

Sequence Diagram

sequenceDiagram
    participant ReverseProxyModal
    participant AuthSSOModal
    participant useUsers Hook
    participant PeerGroupSelector
    participant OnSave Callback

    ReverseProxyModal->>AuthSSOModal: Pass isEnabled={bearerEnabled}
    AuthSSOModal->>useUsers Hook: Load users data
    useUsers Hook-->>AuthSSOModal: Return users list
    AuthSSOModal->>PeerGroupSelector: Render with users prop & <br/>Badge + "All Users" placeholder
    PeerGroupSelector-->>AuthSSOModal: User selects groups
    AuthSSOModal->>AuthSSOModal: Update groups state
    AuthSSOModal->>OnSave Callback: Call onSave with selected groups
    OnSave Callback-->>AuthSSOModal: Process save
    AuthSSOModal->>AuthSSOModal: Close modal

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Add fine-grained ssh policy #522: Modifies PeerGroupSelector to restructure group/item rendering and counters, directly related to the placeholder expansion changes in this PR.

Suggested reviewers

• mlsmaycon

Poem

🐰 A Badge shines bright, and users bloom,
No more just strings in the selection room,
With nodes and hooks and SSO's grace,
Our auth flow finds its perfect place! 🌟

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Allow empty groups for reverse proxy sso auth' accurately reflects the main change of enabling users to save SSO authentication without selecting any groups, making it clear and specific.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/reverse-proxy-sso

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

src/modules/reverse-proxy/auth/AuthSSOModal.tsx (1)

29-31: isEditing is now a bare alias for isEnabled.

Consider inlining isEnabled directly at line 71 ({isEditing ? ...}) or keeping the alias only if you expect the editing condition to diverge from isEnabled in the future. Either way, no functional issue.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/modules/reverse-proxy/auth/AuthSSOModal.tsx` around lines 29 - 31, The
isEditing variable is just a direct alias of isEnabled (const isEditing =
isEnabled); remove the unnecessary alias or inline isEnabled where isEditing is
used (e.g., replace {isEditing ? ...} with {isEnabled ? ...}) — if you prefer to
keep an alias for future divergence, add a comment explaining that intent;
update references to isEditing in AuthSSOModal to use isEnabled (or
keep/annotate isEditing) and remove the redundant declaration to avoid
confusion.

src/components/PeerGroupSelector.tsx (1)

79-79: string is already a subtype of React.ReactNode.

The union React.ReactNode | string is redundant since React.ReactNode already includes string. That said, it does document the intent that the component handles string vs. non-string differently (line 400), so it's fine to keep if you prefer the explicitness.

♻️ Optional: simplify the type

-  placeholder?: React.ReactNode | string;
+  placeholder?: React.ReactNode;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/PeerGroupSelector.tsx` at line 79, The placeholder prop type
is redundant: change the declaration "placeholder?: React.ReactNode | string" in
the PeerGroupSelector component to "placeholder?: React.ReactNode" to simplify
the type; ensure any usage or conditional logic that treats strings specially
(the render/path that checks for string vs non-string in PeerGroupSelector)
still works with React.ReactNode (no runtime change needed), and update any
related JSDoc/comments to reflect the simplified type.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/components/PeerGroupSelector.tsx`:
- Line 79: The placeholder prop type is redundant: change the declaration
"placeholder?: React.ReactNode | string" in the PeerGroupSelector component to
"placeholder?: React.ReactNode" to simplify the type; ensure any usage or
conditional logic that treats strings specially (the render/path that checks for
string vs non-string in PeerGroupSelector) still works with React.ReactNode (no
runtime change needed), and update any related JSDoc/comments to reflect the
simplified type.

In `@src/modules/reverse-proxy/auth/AuthSSOModal.tsx`:
- Around line 29-31: The isEditing variable is just a direct alias of isEnabled
(const isEditing = isEnabled); remove the unnecessary alias or inline isEnabled
where isEditing is used (e.g., replace {isEditing ? ...} with {isEnabled ? ...})
— if you prefer to keep an alias for future divergence, add a comment explaining
that intent; update references to isEditing in AuthSSOModal to use isEnabled (or
keep/annotate isEditing) and remove the redundant declaration to avoid
confusion.