Skip to content

Conversation

@AdrianMaj
Copy link
Contributor

What?

Simplified getTenantOptions utility to respect access control when populating tenant selector options.

Why?

Previously, the tenant selector would display all tenants that a user had in their tenants array, regardless of whether they actually had read access to those tenants through the collection's access control configuration. This created a security/UX issue where users could see tenant options they couldn't actually access.

The old implementation manually iterated through user[tenantsArrayFieldName] and populated tenant IDs, bypassing Payload's access control system. This meant that even if a tenant collection had restrictive read access rules, those rules weren't applied when generating selector options.

How?

Removed the branching logic that differentiated between users with "access to all tenants" vs. limited access. Now, all users go through the same code path:

  1. Single payload.find() call with overrideAccess: false and the user context
  2. Payload's built-in access control automatically filters results based on the tenant collection's read access configuration
  3. Returns only tenants the user has actual read access to

Before:

  • If userHasAccessToAllTenants(user) → fetch all tenants from DB
  • Otherwise → manually extract tenant IDs from user[tenantsArrayFieldName], then query those specific IDs
  • Access control was only partially enforced

After:

  • Always use payload.find() with overrideAccess: false
  • Access control is fully enforced by Payload's existing access control system
  • Simpler code, fewer edge cases

Question: Should I remove the now-unused parameters (tenantsArrayFieldName, tenantsArrayTenantFieldName, userHasAccessToAllTenants) from getTenantOptions, or keep them?

Fixes #14051

@JarrodMFlesch
Copy link
Contributor

JarrodMFlesch commented Nov 21, 2025

@AdrianMaj Im not opposed to this fix, I like it. Can we see why the test is failing? Specifically this one "should only show users assigned tenants".

The other test is flakey and will pass on re-run, but I would like to figure out if the one that failed is real.

@AdrianMaj
Copy link
Contributor Author

Hey @JarrodMFlesch I'll check this test today and update this PR with fix, thanks!

@JarrodMFlesch JarrodMFlesch self-assigned this Nov 21, 2025
@JarrodMFlesch JarrodMFlesch added the plugin: multi-tenant @payloadcms/plugin-multi-tenant label Nov 21, 2025
@AdrianMaj
Copy link
Contributor Author

@JarrodMFlesch The multi-tenant tests are now passing, but some unrelated flaky tests are failing. While the fix should work correctly now, I think we should add an e2e test that verifies if the selector respects access control when populating options. The simplification also made some props unused - should I remove them or keep them for backward compatibility?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants