-
Notifications
You must be signed in to change notification settings - Fork 281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rest admin permissions #2411
Rest admin permissions #2411
Conversation
6963323
to
0bb277f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just starting to review, thanks for the contribution!
} | ||
if (adminDNs.isAdmin(userAndRemoteAddress.getLeft())) { | ||
if (logger.isDebugEnabled()) { | ||
logger.debug("Not SSL admin for {}", endpoint); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like they are admin if this block is hit, typo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh sure must be Super Admin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thorough job! Thank you for also expanding out the test coverage for non-admin cert super user cases.
I've called out some code I'd like to see changed and some areas I've got questions. Thanks!
src/main/java/org/opensearch/security/dlic/rest/api/RestApiAdminPrivilegesEvaluator.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/dlic/rest/support/Utils.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsAction.java
Show resolved
Hide resolved
src/test/java/org/opensearch/security/dlic/rest/api/AbstractRestApiUnitTest.java
Show resolved
Hide resolved
src/main/java/org/opensearch/security/securityconf/SecurityRoles.java
Outdated
Show resolved
Hide resolved
src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiTest.java
Outdated
Show resolved
Hide resolved
AdminDNs adminDNs, ConfigurationRepository cl, ClusterService cs, | ||
PrincipalExtractor principalExtractor, PrivilegesEvaluator evaluator, ThreadPool threadPool, | ||
AuditLog auditLog) { | ||
protected PatchableResourceApiAction(Settings settings, Path configPath, RestController controller, Client client, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my own understanding, is there a reason we need this to be protected vs. public?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes no reasons. Will move protected
and use public
src/main/java/org/opensearch/security/dlic/rest/api/PatchableResourceApiAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/dlic/rest/api/RestApiAdminPrivilegesEvaluator.java
Outdated
Show resolved
Hide resolved
return roles.stream() | ||
.map(r -> { | ||
final WildcardMatcher m = WildcardMatcher.from(r.clusterPerms); | ||
return m == WildcardMatcher.ANY ? WildcardMatcher.NONE : m; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this return ternary condition backwards? Isn't this saying "if wildcardmatcher.matches(any) return "NONE" else return "m"" and don't you want it to be the other way around?
I may be misinterpreting this statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This way Im trying to avoid of such configuration:
all_access:
reserved: true
hidden: false
static: true
description: "Allow full access to all indices and all cluster APIs"
cluster_permissions:
- "*"
so any user without restapi:admin/*
permission does not have access to the endpoint. TBH I do not like it maybe better solution exists?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, that makes sense given what you are trying to do. Perhaps you could leave a short comment mentioning this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure will add.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, could you add a test to verify this behavior, which explicitly tests *
, restapi:admin/*
, restapi:admin/<some-specific-action>
, and <any-other-random-action>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
admin
gets assigned to all_access
by default here: https://github.com/opensearch-project/security/blob/main/config/roles_mapping.yml#L13-L17
This will mean that admin does not get the restapi:*
permissions by default, right? If not, then the role would need to be updated or a new role created using the API, securityadmin or by creating a brand new security index in a new cluster.
If admin
currently has the ability to use the REST APIs, then after this change the admin
should still be able to modify the securityconfig via API by default. I'm not positive of the behavior, but will follow-up after testing.
@willyborankin What do you think about this? all_access
that gives all API access and a new role that has permissions cluster:*
and indices:*
.
all_access:
reserved: true
hidden: false
static: true
description: "Allow full access to all APIs"
cluster_permissions:
- "*"
all_cluster_and_indices:
reserved: true
hidden: false
static: true
description: "Allow full access to all indices and all cluster APIs"
cluster_permissions:
- "cluster:*"
- "indices:*"
src/test/java/org/opensearch/security/dlic/rest/api/SslCertsApiTest.java
Show resolved
Hide resolved
Assert.assertEquals("Unauthorized", reloadCertsResponse.getStatusReason()); | ||
} | ||
|
||
|
||
@Test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these tests be moved into the new SSL test class you made?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like with Actions tests need to be cleaned to make it more readable and better to support. I haven't touched them yet just to avoid of breaking everything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. Your contribution is greatly appreciated.
0bb277f
to
5110fb1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @willyborankin for this detailed contribution! One generic request, could you please add javadoc for classes/methods you introduced.
protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory, | ||
final Object content, | ||
final String resourceName) throws IOException { | ||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following defaults of Boolean, this should return false by default and the implementers can override to return true if needed.
src/main/java/org/opensearch/security/dlic/rest/api/PatchableResourceApiAction.java
Outdated
Show resolved
Hide resolved
return roles.stream() | ||
.map(r -> { | ||
final WildcardMatcher m = WildcardMatcher.from(r.clusterPerms); | ||
return m == WildcardMatcher.ANY ? WildcardMatcher.NONE : m; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, could you add a test to verify this behavior, which explicitly tests *
, restapi:admin/*
, restapi:admin/<some-specific-action>
, and <any-other-random-action>
ENDPOINT + "/roles", | ||
createPatchRestAdminPermissionsPayload("remove"), | ||
restApiHeader); | ||
System.out.println("RESPONSE: " + response.getBody()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can remove this sysout?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure leftovers :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@willyborankin Thank you for the PR! I took a first pass and I've got a few questions around the PR, but overall I think this is a good base to continue working from. My biggest concern here is the change to isSuperAdmin()
and that it looks like the default admin permissions may change to not allow the admin to use the API until explicitly granted. Let us know if you have any questions.
} | ||
|
||
/** | ||
* GET request to fetch transport certificate details |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Can the json be formatted in this comment similar to how it was in SecuritySSLCertsInfoAction
?
return roles.stream() | ||
.map(r -> { | ||
final WildcardMatcher m = WildcardMatcher.from(r.clusterPerms); | ||
return m == WildcardMatcher.ANY ? WildcardMatcher.NONE : m; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
admin
gets assigned to all_access
by default here: https://github.com/opensearch-project/security/blob/main/config/roles_mapping.yml#L13-L17
This will mean that admin does not get the restapi:*
permissions by default, right? If not, then the role would need to be updated or a new role created using the API, securityadmin or by creating a brand new security index in a new cluster.
If admin
currently has the ability to use the REST APIs, then after this change the admin
should still be able to modify the securityconfig via API by default. I'm not positive of the behavior, but will follow-up after testing.
@willyborankin What do you think about this? all_access
that gives all API access and a new role that has permissions cluster:*
and indices:*
.
all_access:
reserved: true
hidden: false
static: true
description: "Allow full access to all APIs"
cluster_permissions:
- "*"
all_cluster_and_indices:
reserved: true
hidden: false
static: true
description: "Allow full access to all indices and all cluster APIs"
cluster_permissions:
- "cluster:*"
- "indices:*"
@@ -558,8 +569,7 @@ public String getName() { | |||
protected abstract Endpoint getEndpoint(); | |||
|
|||
protected boolean isSuperAdmin() { | |||
User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation for this function should not change. Super admin is reserved in the security plugin to indicate that a user is using client cert authentication and the principal of the cert matches a configured admin_dn
in opensearch.yml
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and it isn't change. I just move this check here: https://github.com/willyborankin/security/blob/5110fb1fb7a673177641a6803cbc16ea56b24ec4/src/main/java/org/opensearch/security/dlic/rest/api/RestApiAdminPrivilegesEvaluator.java#L98. I think it is better to keep this code in one place instead of:
if (adminDNs.isAdmin(user.getName()) && ....)
otherwise tests wont pass for existing behave
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cwperks Regarding admin
and current roles. True I missed it!. I think split up roles is a good idea otherwise the end user can easily shoot their foot and cluster could be compromised. An alternative solution would be just forbid to create any role, roles mappings and action group for such permissions and config them ones before cluster up and running but I'm not sure that this is a good idea. Just to be on the same page you idea is to introduce:
all_cluster_and_indices:
reserved: true
hidden: false
static: true
description: "Allow full access to all indices and all cluster APIs"
cluster_permissions:
- "cluster:*"
- "indices:*"
and assign admin by default to it right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the default configuration gives admin the permission to perform:
nodesdn
allowlist
actiongroup
user / internalusers
roles
rolesmapping
tenants
but not
reloadcerts
Only the superadmin (connected via client cert) can perform reloadcerts
now.
After this change I would still expect the admin
to perform the same actions without additional config.
Thank you for addressing all of the PR comments right away, this change looks good to me. Thank you for the contribution!
if (adminDNs.isAdmin(userAndRemoteAddress.getLeft())) { | ||
if (logger.isDebugEnabled()) { | ||
logger.debug( | ||
"Security admin permissions required for endpoint {} but {} is not an admin", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This log message is the opposite of what it should be. The original block in RestApiPrivilegesEvaluator
was:
if (!adminDNs.isAdminDN(sslInfo.getPrincipal())) {
logger.warn("Security admin permissions required but {} is not an admin", sslInfo.getPrincipal());
return "Security admin permissions required but " + sslInfo.getPrincipal() + " is not an admin";
}
if the user is connecting with admin dn then they are said to be super admin and have privileges to perform all actions in the cluster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok will change np.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh sorry misread the comment. The code in the master:
User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
return adminDNs.isAdmin(user);
I only added a debug message. The purpose of isSuperAdmin()
stays the same. RestApiAdminPrivilegesEvaluator
just adds additional check for the permission nothing more since the logged in user can has such permissions to change e.g. hidden
or reserved
fields. Maybe the name for the class RestApiAdminPrivilegesEvaluator
needs to be changed for better understanding what it does. WDYT?
@@ -77,4 +79,22 @@ protected CType getConfigName() { | |||
return CType.ROLES; | |||
} | |||
|
|||
@Override | |||
protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfiguration, final Object content, final String resourceName) throws IOException { | |||
if (restApiAdminPrivilegesEvaluator.containsRestApiAdminPermissions(content)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the user is assigned restapi:admin/*
what is the purpose of the isSuperAdmin()
check? I saw that the implementation of isSuperAdmin
was also changed in this PR, but it has a specific meaning in the security plugin.
Curious, why not follow a similar pattern to cluster permissions where in ConfigModelV7
and ConfigModelV6
there is a WildcardMatcher that tests the given action to see if it matches permissions that the user is assigned.
Added granular permissions for all REST API actions in OpenSearch to be individually assigned. Permissions are: - 'restapi:admin/actiongroups' - allow full access to actiongroups - 'restapi:admin/allowlist' - allow full access to allowlist - 'restapi:admin/internalusers'- allow full access to internalusers - 'restapi:admin/nodesdn'- allow full access to nodesdn - 'restapi:admin/roles' - allow full access to roles - 'restapi:admin/rolesmapping' - allow full access to roles mappings - 'restapi:admin/ssl/certs/info' - allow full access to certs info - 'restapi:admin/ssl/certs/reload' - allow full access to certs reload - 'restapi:admin/tenants' - allow full access to tenants Signed-off-by: Andrey Pleskach <[email protected]>
Signed-off-by: Andrey Pleskach <[email protected]>
5110fb1
to
b336e27
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All my feedback has been addressed, thanks!
FYI @willyborankin, I just stumbled upon a feature of the security plugin I was unfamiliar with before today. There is a config setting called This config setting gives users of that role access to the security APIs as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this contribution @willyborankin!
metaNode.set("_meta", meta("tenants")); | ||
return SecurityDynamicConfiguration.fromNode(metaNode, CType.TENANTS, 2, 0, 0); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is awesome.. Thank you!
@cwperks Terribly sorry, was planning to add my comments today. I was on vacation previous week and this one was in the different city to meet my team :-). Only had time to push my changes. |
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-2411-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d676716e83d1ab387e9e6a0c0f3284e39ed967f5
# Push it to GitHub
git push --set-upstream origin backport/backport-2411-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x Then, create a pull request where the |
@willyborankin @reta Apologies for not adding the backport label earlier. There is a code freeze for 2.6 EOD today and I have been going through PRs to ensure the backport label was added for PRs that should be backported. If this featured should be shipped in one of the 2.x releases can someone look into creating a manual backport? |
Sure will do tomorrow. |
Permissions for REST admin user Added granular permissions for all REST API actions in OpenSearch to be individually assigned. Permissions are: - 'restapi:admin/actiongroups' - allow full access to actiongroups - 'restapi:admin/allowlist' - allow full access to allowlist - 'restapi:admin/internalusers'- allow full access to internalusers - 'restapi:admin/nodesdn'- allow full access to nodesdn - 'restapi:admin/roles' - allow full access to roles - 'restapi:admin/rolesmapping' - allow full access to roles mappings - 'restapi:admin/ssl/certs/info' - allow full access to certs info - 'restapi:admin/ssl/certs/reload' - allow full access to certs reload - 'restapi:admin/tenants' - allow full access to tenants Adds tests for these permissions. Signed-off-by: Andrey Pleskach <[email protected]>
Permissions for REST admin user Added granular permissions for all REST API actions in OpenSearch to be individually assigned. Permissions are: - 'restapi:admin/actiongroups' - allow full access to actiongroups - 'restapi:admin/allowlist' - allow full access to allowlist - 'restapi:admin/internalusers'- allow full access to internalusers - 'restapi:admin/nodesdn'- allow full access to nodesdn - 'restapi:admin/roles' - allow full access to roles - 'restapi:admin/rolesmapping' - allow full access to roles mappings - 'restapi:admin/ssl/certs/info' - allow full access to certs info - 'restapi:admin/ssl/certs/reload' - allow full access to certs reload - 'restapi:admin/tenants' - allow full access to tenants Adds tests for these permissions. Signed-off-by: Andrey Pleskach <[email protected]>
Permissions for REST admin user Added granular permissions for all REST API actions in OpenSearch to be individually assigned. Permissions are: - 'restapi:admin/actiongroups' - allow full access to actiongroups - 'restapi:admin/allowlist' - allow full access to allowlist - 'restapi:admin/internalusers'- allow full access to internalusers - 'restapi:admin/nodesdn'- allow full access to nodesdn - 'restapi:admin/roles' - allow full access to roles - 'restapi:admin/rolesmapping' - allow full access to roles mappings - 'restapi:admin/ssl/certs/info' - allow full access to certs info - 'restapi:admin/ssl/certs/reload' - allow full access to certs reload - 'restapi:admin/tenants' - allow full access to tenants Adds tests for these permissions. Signed-off-by: Andrey Pleskach <[email protected]>
Permissions for REST admin user Added granular permissions for all REST API actions in OpenSearch to be individually assigned. Permissions are: - 'restapi:admin/actiongroups' - allow full access to actiongroups - 'restapi:admin/allowlist' - allow full access to allowlist - 'restapi:admin/internalusers'- allow full access to internalusers - 'restapi:admin/nodesdn'- allow full access to nodesdn - 'restapi:admin/roles' - allow full access to roles - 'restapi:admin/rolesmapping' - allow full access to roles mappings - 'restapi:admin/ssl/certs/info' - allow full access to certs info - 'restapi:admin/ssl/certs/reload' - allow full access to certs reload - 'restapi:admin/tenants' - allow full access to tenants Adds tests for these permissions. Signed-off-by: Andrey Pleskach <[email protected]>
Permissions for REST admin user Added granular permissions for all REST API actions in OpenSearch to be individually assigned. Permissions are: - 'restapi:admin/actiongroups' - allow full access to actiongroups - 'restapi:admin/allowlist' - allow full access to allowlist - 'restapi:admin/internalusers'- allow full access to internalusers - 'restapi:admin/nodesdn'- allow full access to nodesdn - 'restapi:admin/roles' - allow full access to roles - 'restapi:admin/rolesmapping' - allow full access to roles mappings - 'restapi:admin/ssl/certs/info' - allow full access to certs info - 'restapi:admin/ssl/certs/reload' - allow full access to certs reload - 'restapi:admin/tenants' - allow full access to tenants Adds tests for these permissions. Signed-off-by: Andrey Pleskach <[email protected]>
Permissions for REST admin user Added granular permissions for all REST API actions in OpenSearch to be individually assigned. Permissions are: - 'restapi:admin/actiongroups' - allow full access to actiongroups - 'restapi:admin/allowlist' - allow full access to allowlist - 'restapi:admin/internalusers'- allow full access to internalusers - 'restapi:admin/nodesdn'- allow full access to nodesdn - 'restapi:admin/roles' - allow full access to roles - 'restapi:admin/rolesmapping' - allow full access to roles mappings - 'restapi:admin/ssl/certs/info' - allow full access to certs info - 'restapi:admin/ssl/certs/reload' - allow full access to certs reload - 'restapi:admin/tenants' - allow full access to tenants Adds tests for these permissions. Signed-off-by: Andrey Pleskach <[email protected]>
Permissions for REST admin user Added granular permissions for all REST API actions in OpenSearch to be individually assigned. Permissions are: - 'restapi:admin/actiongroups' - allow full access to actiongroups - 'restapi:admin/allowlist' - allow full access to allowlist - 'restapi:admin/internalusers'- allow full access to internalusers - 'restapi:admin/nodesdn'- allow full access to nodesdn - 'restapi:admin/roles' - allow full access to roles - 'restapi:admin/rolesmapping' - allow full access to roles mappings - 'restapi:admin/ssl/certs/info' - allow full access to certs info - 'restapi:admin/ssl/certs/reload' - allow full access to certs reload - 'restapi:admin/tenants' - allow full access to tenants Adds tests for these permissions. Signed-off-by: Andrey Pleskach <[email protected]> Co-authored-by: Ryan Liang <[email protected]> Co-authored-by: Stephen Crawford <[email protected]>
…project#2466)" This reverts commit 076715d.
…project#2466)" This reverts commit 076715d.
…project#2466)" This reverts commit 076715d.
…project#2466)" This reverts commit 076715d. Signed-off-by: Andrey Pleskach <[email protected]>
This reverts commit 076715d. Signed-off-by: Andrey Pleskach <[email protected]>
Permissions for REST admin user Added granular permissions for all REST API actions in OpenSearch to be individually assigned. Permissions are: - 'restapi:admin/actiongroups' - allow full access to actiongroups - 'restapi:admin/allowlist' - allow full access to allowlist - 'restapi:admin/internalusers'- allow full access to internalusers - 'restapi:admin/nodesdn'- allow full access to nodesdn - 'restapi:admin/roles' - allow full access to roles - 'restapi:admin/rolesmapping' - allow full access to roles mappings - 'restapi:admin/ssl/certs/info' - allow full access to certs info - 'restapi:admin/ssl/certs/reload' - allow full access to certs reload - 'restapi:admin/tenants' - allow full access to tenants Adds tests for these permissions. Signed-off-by: Andrey Pleskach <[email protected]> Signed-off-by: Maciej Mierzwa <[email protected]>
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.8 2.8
# Navigate to the new working tree
cd .worktrees/backport-2.8
# Create a new branch
git switch --create backport/backport-2411-to-2.8
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d676716e83d1ab387e9e6a0c0f3284e39ed967f5
# Push it to GitHub
git push --set-upstream origin backport/backport-2411-to-2.8
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.8 Then, create a pull request where the |
Permissions for REST admin user Added granular permissions for all REST API actions in OpenSearch to be individually assigned. Permissions are: - 'restapi:admin/actiongroups' - allow full access to actiongroups - 'restapi:admin/allowlist' - allow full access to allowlist - 'restapi:admin/internalusers'- allow full access to internalusers - 'restapi:admin/nodesdn'- allow full access to nodesdn - 'restapi:admin/roles' - allow full access to roles - 'restapi:admin/rolesmapping' - allow full access to roles mappings - 'restapi:admin/ssl/certs/info' - allow full access to certs info - 'restapi:admin/ssl/certs/reload' - allow full access to certs reload - 'restapi:admin/tenants' - allow full access to tenants Adds tests for these permissions. Signed-off-by: Andrey Pleskach <[email protected]> (cherry picked from commit d676716)
* role.yml changes for lron feature (#2789) (#2792) Signed-off-by: zhichao-aws <[email protected]> (cherry picked from commit a580dfc) Co-authored-by: zhichao-aws <[email protected]> * add ml model group system index (#2790) (#2797) Signed-off-by: Yaliang Wu <[email protected]> (cherry picked from commit 1bb2ef1) Co-authored-by: Yaliang Wu <[email protected]> * Rest admin permissions (#2411) Permissions for REST admin user Added granular permissions for all REST API actions in OpenSearch to be individually assigned. Permissions are: - 'restapi:admin/actiongroups' - allow full access to actiongroups - 'restapi:admin/allowlist' - allow full access to allowlist - 'restapi:admin/internalusers'- allow full access to internalusers - 'restapi:admin/nodesdn'- allow full access to nodesdn - 'restapi:admin/roles' - allow full access to roles - 'restapi:admin/rolesmapping' - allow full access to roles mappings - 'restapi:admin/ssl/certs/info' - allow full access to certs info - 'restapi:admin/ssl/certs/reload' - allow full access to certs reload - 'restapi:admin/tenants' - allow full access to tenants Adds tests for these permissions. Signed-off-by: Andrey Pleskach <[email protected]> (cherry picked from commit d676716) * Fixes CI errors Signed-off-by: Darshit Chanpura <[email protected]> * Fixes HTTP5 imports Signed-off-by: Darshit Chanpura <[email protected]> * Fixes password related changes in tests Signed-off-by: Darshit Chanpura <[email protected]> * Update ActionGroupsApiTest.java Remove unused import * Incorporates jar hell fix Signed-off-by: Darshit Chanpura <[email protected]> --------- Signed-off-by: Darshit Chanpura <[email protected]> Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Co-authored-by: zhichao-aws <[email protected]> Co-authored-by: Yaliang Wu <[email protected]> Co-authored-by: Andrey Pleskach <[email protected]> Co-authored-by: Stephen Crawford <[email protected]>
* Rest admin permissions (#2411) Permissions for REST admin user Added granular permissions for all REST API actions in OpenSearch to be individually assigned. Permissions are: - 'restapi:admin/actiongroups' - allow full access to actiongroups - 'restapi:admin/allowlist' - allow full access to allowlist - 'restapi:admin/internalusers'- allow full access to internalusers - 'restapi:admin/nodesdn'- allow full access to nodesdn - 'restapi:admin/roles' - allow full access to roles - 'restapi:admin/rolesmapping' - allow full access to roles mappings - 'restapi:admin/ssl/certs/info' - allow full access to certs info - 'restapi:admin/ssl/certs/reload' - allow full access to certs reload - 'restapi:admin/tenants' - allow full access to tenants Adds tests for these permissions. Signed-off-by: Andrey Pleskach <[email protected]> (cherry picked from commit d676716) * Fixes CI errors Signed-off-by: Darshit Chanpura <[email protected]> * Fixes HTTP5 imports Signed-off-by: Darshit Chanpura <[email protected]> * Fixes password related changes in tests Signed-off-by: Darshit Chanpura <[email protected]> * Update ActionGroupsApiTest.java Remove unused import * Incorporates jar hell fix Signed-off-by: Darshit Chanpura <[email protected]> --------- Signed-off-by: Darshit Chanpura <[email protected]> Co-authored-by: Andrey Pleskach <[email protected]> Co-authored-by: Stephen Crawford <[email protected]>
Description
The aim of this PR is to introduce authorization mechanism for endpoints not only for the super admin
but for users with certain permissions.
Endpoints are:
nodesdn
allowlist
actiongroup
user
/internalusers
roles
rolesmapping
tenants
Each endpoint has its own permission:
-
restapi:admin/actiongroups
- full access to action groups-
restapi:admin/allowlist
- full access to allow list-
restapi:admin/internalusers
- full access to internal users-
restapi:admin/nodesdn
- full access to nodes DN-
restapi:admin/roles
- full access to roles-
restapi:admin/rolesmapping
- full access to roles mapping-
restapi:admin/ssl/certs/info
- full access to SSL certs info-
restapi:admin/ssl/certs/reload
- full access to SSL certs reload-
restapi:admin/tenants
- full access to tenantsThe role with such permissions it is possible to create via static configuration,
all attempts to create roles, roles mapping or action group with such permissions are forbidden except super admin and the user with the certain permission.
Issues Resolved
#1878
Is this a backport? If so, please add backport PR # and/or commits #
So far not but I think it is possible.
Testing
[Please provide details of testing done: unit testing, integration testing and manual testing]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.