Skip to content

Let cluster admins admin#604

Open
sprucely wants to merge 2 commits intomainfrom
swe/ClusterAdminPerms2
Open

Let cluster admins admin#604
sprucely wants to merge 2 commits intomainfrom
swe/ClusterAdminPerms2

Conversation

@sprucely
Copy link
Copy Markdown
Contributor

@sprucely sprucely commented Apr 6, 2026

Closes #601

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Enables Cluster Admins (even without an account in the cluster) to access administrative approval flows for group-related requests, addressing issue #601 where Cluster Admins could approve account requests but not group/create-group requests.

Changes:

  • Update usePermissions to allow Cluster Admins to manage/view groups within the current cluster route.
  • Update RequireAupAgreement to avoid blocking Cluster Admins (and to scope FinancialAdmin checks to the current cluster).
  • Add test fixtures and a new UI test covering Cluster Admin access without an account.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
Hippo.Web/ClientApp/src/test/mockData.ts Adds a ClusterAdmin app context fixture with no accounts for tests.
Hippo.Web/ClientApp/src/Shared/usePermissions.ts Grants Cluster Admins manage/view permissions for groups in the current cluster.
Hippo.Web/ClientApp/src/Shared/RequireAupAgreement.tsx Bypasses AUP gate for Cluster Admins; scopes FinancialAdmin permission checks to the current cluster.
Hippo.Web/ClientApp/src/components/Account/Requests.test.tsx Adds a test ensuring Cluster Admins without an account can access the approvals UI.
Comments suppressed due to low confidence (1)

Hippo.Web/ClientApp/src/Shared/RequireAupAgreement.tsx:88

  • The comment above this return says "cluster doesn't have an AUP, so no verification necessary", but the condition now also bypasses AUP verification for System/Cluster admins even when the cluster does have an AUP. Please update the comment to reflect the actual reason(s) for bypassing the AUP gate here (or split the condition so the comment remains accurate).
  if (
    hasSystemPermission ||
    hasClusterAdminPermission ||
    !cluster.acceptableUsePolicyUrl ||
    !cluster.acceptableUsePolicyUpdatedOn
  ) {
    // cluster doesn't have an AUP, so no verification necessary
    return <>{children}</>;

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const canManageGroup = (groupName: string) => {
if (isSystemAdmin) return true;
if (!clusterName) return false;
if (isSystemAdmin || isClusterAdmin) return true;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there anything the system admin allows that we don't want a cluster admin to do?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so.

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.

Cluster Admin can approve Create Account requests, but not Create Group requests

3 participants