Skip to content

Tenant Permissions : added the possibility to specify tenants via parameter#1813

Merged
peternied merged 5 commits intoopensearch-project:mainfrom
peternied:TenantsParameterSubstitutions
Jun 7, 2022
Merged

Tenant Permissions : added the possibility to specify tenants via parameter#1813
peternied merged 5 commits intoopensearch-project:mainfrom
peternied:TenantsParameterSubstitutions

Conversation

@peternied
Copy link
Copy Markdown
Member

Description

This is a continuation of #819 with many thanks to @chrousto

The idea would be that this user :

new-user:
  hash: "*************"
  reserved: false
  hidden: false
  opendistro_security_roles:
  - "role-tenant1"
  attributes:
    attribute1: "tenant1"
  static: false

Would only have access to the tenant tenant1 if the role role-tenant1 was defined like this :

role-tenant1:
 reserved: false
 hidden: false
 cluster_permissions:
 - "read"
 - "cluster:monitor/nodes/stats"
 - "cluster:monitor/task/get"
 tenant_permissions:
 - tenant_patterns:
   - ${attr.internal.attribute1}
   allowed_actions:
   - "kibana_all_write"
 static: false
_meta:
 type: "roles"
 config_version: 2

Issues Resolved

Testing

Integration tests have been created for this feature

Check List

  • New functionality includes testing
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

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.

chrousto and others added 3 commits April 29, 2022 20:33
…ity to specify tenants via parameter substitution.

Signed-off-by: Christophe Chazeau <christophe.chazeau@gmail.com>
…ions

Signed-off-by: Christophe Chazeau <christophe.chazeau@gmail.com>
Following some of the practices discussed on this project I've
made several modifications to the scenario tests that were added to
ensure readiablity and maintainability.  Used this as an exercise to
be more familiar with the tenant parameter based substitution.

Signed-off-by: Peter Nied <petern@amazon.com>
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 29, 2022

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 60.90%. Comparing base (94cc878) to head (75ccda8).
⚠️ Report is 1456 commits behind head on main.

Files with missing lines Patch % Lines
...pensearch/security/securityconf/ConfigModelV7.java 90.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1813      +/-   ##
============================================
+ Coverage     60.87%   60.90%   +0.02%     
- Complexity     3215     3216       +1     
============================================
  Files           256      256              
  Lines         18012    18019       +7     
  Branches       3211     3213       +2     
============================================
+ Hits          10965    10974       +9     
+ Misses         5467     5464       -3     
- Partials       1580     1581       +1     
Files with missing lines Coverage Δ
...pensearch/security/securityconf/ConfigModelV7.java 62.76% <90.00%> (+0.28%) ⬆️

... and 2 files with indirect coverage changes

🚀 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.

DarshitChanpura
DarshitChanpura previously approved these changes May 2, 2022
@peternied
Copy link
Copy Markdown
Member Author

@cliu123 Did you have any other comments around this pull request that need to be addressed?

@cliu123 cliu123 self-requested a review May 17, 2022 23:39
Copy link
Copy Markdown
Member

@cliu123 cliu123 left a comment

Choose a reason for hiding this comment

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

Most of the changes look great to me! I missed the do...while in last review though. This can be merged when it gets fixed.

Signed-off-by: Peter Nied <petern@amazon.com>
cliu123
cliu123 previously approved these changes May 27, 2022
…itutions

Signed-off-by: Peter Nied <petern@amazon.com>
@peternied peternied force-pushed the TenantsParameterSubstitutions branch from 26e348b to 75ccda8 Compare May 27, 2022 21:15
@peternied
Copy link
Copy Markdown
Member Author

@DarshitChanpura @cliu123 could I get another review?

@peternied peternied merged commit ce59944 into opensearch-project:main Jun 7, 2022
@peternied peternied deleted the TenantsParameterSubstitutions branch June 7, 2022 17:41
stephen-crawford pushed a commit to stephen-crawford/security that referenced this pull request Nov 10, 2022
…ameter (opensearch-project#1813)

* Issue opensearch-project#817 : Tenant Permissions, added the possibility to specify tenants via parameter substitution.

Signed-off-by: Christophe Chazeau <christophe.chazeau@gmail.com>
Co-authored-by: Peter Nied <petern@amazon.com>
Signed-off-by: Stephen Crawford <steecraw@amazon.com>
wuychn pushed a commit to ochprince/security that referenced this pull request Mar 16, 2023
…ameter (opensearch-project#1813)

* Issue opensearch-project#817 : Tenant Permissions, added the possibility to specify tenants via parameter substitution.

Signed-off-by: Christophe Chazeau <christophe.chazeau@gmail.com>
Co-authored-by: Peter Nied <petern@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants