Optionally allow support for resource in Admin actions

[rraulinio]

Certain admin actions like admin:GetBucketQuota and admin:SetBucketQuota operate at a bucket level, but previously there was no way to restrict them to specific buckets via IAM policies, granting the action allowed it across all buckets.
This change makes resource matching conditional for admin policy statements: when a Resource field is specified, it is enforced against the target bucket; when omitted, behavior is unchanged (backward compatible).

[Copilot]

Pull request overview

This PR updates IAM policy evaluation to optionally enforce Resource / NotResource matching for admin actions, enabling per-bucket scoping for bucket-level admin APIs while keeping existing behavior when Resource is omitted.

Changes:

• Update Statement.IsAllowedPtr to ignore resource matching for admin statements only when neither Resource nor NotResource is specified.
• Add validation for admin statement Resource / NotResource when present.
• Add test coverage for admin resource scoping behavior (allow/deny cases, wildcard, invalid ARN).

Reviewed changes

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

File	Description
policy/statement.go	Makes admin resource matching conditional on explicit Resource/NotResource and adds validation for these fields when provided.
policy/policy_test.go	Adds a new test suite verifying bucket-scoped admin policy behavior and backward compatibility when no resource is specified.

---

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

[Copilot]

Pull request overview

This PR updates IAM policy evaluation to optionally enforce Resource/NotResource matching for a subset of bucket-scoped admin actions (e.g., bucket quota operations), enabling bucket-level scoping where it previously wasn’t possible.

Changes:

• Make resource matching conditional for admin actions based on membership in AdminActionsResourceSupported.
• Add AdminActionsResourceSupported to enumerate which admin actions support bucket-level resource scoping.
• Add tests covering allow/deny outcomes for scoped vs unscoped admin statements and validation behavior.

Reviewed changes

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

File	Description
policy/statement.go	Implements per-action admin resource matching and adds admin Resource-support validation logic plus a helper for Resource/NotResource conflict detection.
policy/admin-action.go	Adds AdminActionsResourceSupported allowlist for admin actions that support Resource/NotResource scoping.
policy/policy_test.go	Adds TestAdminPolicyResourceScoping covering resource-scoped admin actions and parse-time validation expectations.

---

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

[taran-p]

I still think having a condition key, (i.e. admin:QuotaBucket or quota:Bucket) rather than using resource, would help avoid any backwards compatibility issues and make it clear which actions are being affected.

That said if we go down the resource route, this looks fine to me.

[klauspost]

@taran-p We don't want that key to be different for every type.

[rraulinio]

@klauspost @taran-p If we want to, we can introduce a new StrictAdminResources bool that's passed as true from EOS when the new env var is set and then we are good against potential backward compatibility issues - this is if we really want to be on the safe side.
It is hacky, I know.

[klauspost]

@rraulinio I don't have any particular problem with that. So you mean it will transfer internally to that if it is a new admin request?

It ofc isn't optimal, but it would guarantee that no existing policies are suddenly broken. (if I understand your sugg correctly).

[rraulinio]

@klauspost Somehow correct. EOS will tell pkg that it is a request that MUST be strictly validated. So then we bring up the new condition, only under this scenario. All other scenarios - nothing changes, meaning that there will be absolutely no different behaviour for anyone not setting the new env var. Again, it is hacky, but it works. The env var also controls the condition to not have both Resource and NotResource at the same time in a given statement, so overall conceptually it is fine, in my opinion.

[taran-p]

Sure, that makes sense, though I don't think the current implementation will lead to too many issues either

[rraulinio]

@taran-p It wouldn't, most likely. If we feel like we shouldn't attempt the hacky way, then fine by me. Let's maybe also hear from @harshavardhana and/or @kannappanr. For now, I'll just leave it as is. cc @klauspost

[harshavardhana]

@rraulinio I pushed a commit to your branch that shows the idiomatic way to handle this in pkg/policy — this is what I was asking you to do over Slack.

The key idea: don't check args.BucketName == "" at eval time. That's brittle and caller-dependent. Instead, we declare an explicit Action → Resource map (AdminActionsWithResource) — the same pattern we already use for adminActionConditionKeyMap. The map is the single source of truth for which admin actions are bucket-scoped.

Changes:

1. AdminActionsWithResource map — lists the small set of admin actions that operate on a bucket resource (quota, targets, replication diff, bucket metadata, heal). AdminAction.HasResource() checks membership. This replaces the BucketName == "" check.

2. Two validation paths — ParseConfig/Validate stays permissive for loading existing policies. ParseConfigStrict/ValidateStrict is for creating/updating new policies and rejects Resource + NotResource in the same admin statement, validates ARNs on bucket-scoped actions. This replaces HasResourceConflict() which callers had to remember to call.

3. Mixed action type rejection — validateActionTypes() now rejects statements that mix namespaces (e.g. s3:GetObject + admin:ServerInfo) with a clear error instead of the confusing "unsupported admin action 's3:GetObject'" message.

[rraulinio]

@harshavardhana That's more or less what we had before. But then agreed with @klauspost and @taran-p on the Bucket == "" solution, if you follow the discussions above. Anyway, I'll check the code, thank you! 👍