Skip to content

Remove action allowlist on the server side#96936

Merged
elasticsearchmachine merged 3 commits intoelastic:mainfrom
ywangd:rcs2-remote-server-allowlist
Jun 21, 2023
Merged

Remove action allowlist on the server side#96936
elasticsearchmachine merged 3 commits intoelastic:mainfrom
ywangd:rcs2-remote-server-allowlist

Conversation

@ywangd
Copy link
Member

@ywangd ywangd commented Jun 20, 2023

With the new cross-cluster API keys (#95714), RCS 2.0 is more tightly controlled. This removes the need for the action allowlist on the server side because cross-cluster API keys (1) require manage_security to create and (2) are specifized to give out only cross-cluster operation related privileges. Hence this PR removes the allowlist.

With the new cross-cluster API keys (elastic#95714), RCS 2.0 is more tightly
controlled. This removes the need for the action allowlist on the server
side because cross-cluster API keys (1) require manage_security to
create and (2) are specifized to give out only cross-cluster operation
related privileges. Hence this PR removes the allowlist.
@ywangd ywangd added >non-issue :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC v8.9.0 labels Jun 20, 2023
@ywangd ywangd requested a review from n1v0lg June 20, 2023 05:39
@elasticsearchmachine elasticsearchmachine added the Team:Security Meta label for security team label Jun 20, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

Copy link
Contributor

@n1v0lg n1v0lg left a comment

Choose a reason for hiding this comment

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

LGTM. I'm on board with this as it makes the code easier to maintain. I agree with your analysis in the Jira of the security impact (i.e., lack therefore) of this change.

@@ -48,7 +48,6 @@
import static org.elasticsearch.xpack.core.security.support.Exceptions.authenticationError;
Copy link
Contributor

Choose a reason for hiding this comment

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

We gotta fix up testInboundDestructiveOperations as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch 👍 I totally missed it

@ywangd ywangd added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jun 21, 2023
@elasticsearchmachine elasticsearchmachine merged commit 3f216d1 into elastic:main Jun 21, 2023
@ywangd ywangd deleted the rcs2-remote-server-allowlist branch June 21, 2023 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >non-issue :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Security Meta label for security team v8.9.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants