Skip to content

fix(multi_tenancy): default and current tenant cannot be deleted (#6227)#6248

Merged
a19836 merged 10 commits into
mainfrom
issue/6227
Jun 16, 2026
Merged

fix(multi_tenancy): default and current tenant cannot be deleted (#6227)#6248
a19836 merged 10 commits into
mainfrom
issue/6227

Conversation

@a19836

@a19836 a19836 commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Description

On main, navigating to the Security settings page exposes two critical issues related to the multi-tenancy feature:

  1. Missing feature flag check: The list of tenants is visible in the Security settings even though the multi-tenancy feature has not been released yet. A feature flag check is missing on this page.
  2. Default tenant can be deleted: It is possible to delete the default tenant (the only tenant in the list), which should not be allowed. The current implementation should only permit renaming the default tenant, not deleting it.
  3. Default tenant can have new tenants added to it: It is also possible to add new tenants from this view, which should not be available at this stage.

Proposed changes

  • If featuredFlag is enabled, when a Tenant is deleted, check if is the default one or if is the current selected one. if it is, don't allow deletion showing a friendly error message.
  • If featuredFlag is disabled, then user should not access the url https://main.oaev.staging.filigran.io/<instance-id>/admin/settings/security/tenants/
  • If this url is not accessible, then user cannot add new tenants either.

Testing Instructions

If featuredFlag is enabled:

  1. Navigate to https://main.oaev.staging.filigran.io/<instance-id>/admin/settings/security/tenants/
  2. Observe the tenant list is visible
  3. Attempt to delete the default tenant
  4. Observe the deletion is not going through and an error message is displayed.

If featuredFlag is disabled:

  1. Navigate to https://main.oaev.staging.filigran.io/<instance-id>/admin/settings/security/tenants/
  2. User cannot see page - access denied.

Related issues

Checklist

  • I consider the submitted work as finished
  • I tested the code for its functionality
  • I wrote test cases for the relevant uses case
  • I added/update the relevant documentation (either on github or on notion)
  • Where necessary I refactored code to improve the overall quality
  • For bug fix -> I implemented a test that covers the bug

@a19836 a19836 self-assigned this Jun 15, 2026
@a19836 a19836 added filigran team Item from the Filigran team. multi-tenancy Functional scope: multi-tenancy labels Jun 15, 2026
@a19836 a19836 requested a review from damgouj June 15, 2026 12:05
@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 14.70588% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.82%. Comparing base (9292bb6) to head (c9b34bb).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
...front/src/components/common/DialogConfirmation.tsx 20.00% 12 Missing ⚠️
...dmin/components/platform/tenants/TenantPopover.tsx 0.00% 10 Missing ⚠️
...java/io/openaev/service/tenants/TenantService.java 40.00% 3 Missing ⚠️
...mponents/platform/tenants/routes/TenantsRoutes.tsx 0.00% 2 Missing ⚠️
...naev-front/src/admin/components/settings/Index.tsx 0.00% 2 Missing ⚠️

❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project check has failed because the head coverage (2.16%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6248      +/-   ##
============================================
- Coverage     42.90%   42.82%   -0.09%     
- Complexity     6811     6847      +36     
============================================
  Files          2247     2253       +6     
  Lines         61220    61648     +428     
  Branches       8015     8100      +85     
============================================
+ Hits          26267    26400     +133     
- Misses        33272    33564     +292     
- Partials       1681     1684       +3     
Flag Coverage Δ
backend 65.15% <40.00%> (+0.13%) ⬆️
e2e 17.96% <0.00%> (-0.78%) ⬇️
frontend 2.16% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses multi-tenancy hardening by (1) gating the Tenants management UI behind the MULTI_TENANCY feature flag + RBAC access, and (2) preventing deletion of the default tenant and the currently-selected tenant at the service layer, with added test coverage.

Changes:

  • Frontend: deny access to the Tenants admin page when MULTI_TENANCY is disabled or the user lacks TENANTS access.
  • Backend: block soft-delete of the default tenant and the current tenant; convert some tenant lifecycle errors to BadRequestException.
  • Tests/UI flows: add service tests for the new soft-delete guards; adjust dialog handling around delete/reactivate flows.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
openaev-front/src/components/common/DialogConfirmation.tsx Reworks loading/submission handling for confirmation dialogs (currently introduces async/loading correctness issues).
openaev-front/src/admin/components/platform/tenants/Tenants.tsx Adds feature-flag + RBAC gating for the tenants page (currently violates Rules of Hooks due to early return).
openaev-front/src/admin/components/platform/tenants/TenantPopover.tsx Ensures dialogs close in finally for delete/reactivate actions.
openaev-api/src/main/java/io/openaev/service/tenants/TenantService.java Adds guards to prevent deleting default/current tenant; uses BadRequestException for user-facing invalid operations.
openaev-api/src/test/java/io/openaev/service/tenants/TenantServiceTest.java Adds tests asserting deletion is blocked for default and current tenant (needs TenantContext cleanup to avoid test leakage).

Comment thread openaev-front/src/components/common/DialogConfirmation.tsx Outdated
Comment thread openaev-front/src/admin/components/platform/tenants/Tenants.tsx Outdated
Comment thread openaev-api/src/test/java/io/openaev/service/tenants/TenantServiceTest.java Outdated
@a19836 a19836 marked this pull request as ready for review June 15, 2026 13:16

@damgouj damgouj left a comment

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.

Some remarks, don't hesitate if necessary

Comment thread openaev-api/src/main/java/io/openaev/service/tenants/TenantService.java Outdated
Comment thread openaev-api/src/test/java/io/openaev/service/tenants/TenantServiceTest.java Outdated
Comment thread openaev-front/src/admin/components/platform/tenants/Tenants.tsx Outdated
Comment thread openaev-front/src/components/common/DialogConfirmation.tsx
…needed bc the TenantInterceptorTest already does this
@a19836 a19836 requested a review from damgouj June 15, 2026 16:20
@a19836 a19836 merged commit a567291 into main Jun 16, 2026
36 checks passed
@a19836 a19836 deleted the issue/6227 branch June 16, 2026 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

filigran team Item from the Filigran team. multi-tenancy Functional scope: multi-tenancy

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Multi-Tenancy] Tenant list exposed without feature flag + default tenant can be deleted

3 participants