Skip to content
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

Add exclude_roles configuration parameter to LDAP authorization backend #4025

Merged
merged 13 commits into from
Feb 12, 2024

Conversation

cwperks
Copy link
Member

@cwperks cwperks commented Feb 6, 2024

Description

Based on related PR: #3809

Adds a new config value for LDAP Authorization Backend called exclude_roles. This config value lets a cluster administrator configure a list of roles (globs are accepted too) to exclude from an external LDAP system to limit the backend roles fetched for a user to the most pertinent roles for OpenSearch.

  • Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)

Enhancement

Issues Resolved

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.

MaciejMierzwa and others added 7 commits December 8, 2023 10:44
Signed-off-by: Maciej Mierzwa <dev.maciej.mierzwa@gmail.com>
Signed-off-by: Maciej Mierzwa <dev.maciej.mierzwa@gmail.com>
Signed-off-by: Maciej Mierzwa <dev.maciej.mierzwa@gmail.com>
Signed-off-by: Maciej Mierzwa <dev.maciej.mierzwa@gmail.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Copy link

codecov bot commented Feb 6, 2024

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (321604c) 65.60% compared to head (8141cd5) 65.84%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4025      +/-   ##
==========================================
+ Coverage   65.60%   65.84%   +0.23%     
==========================================
  Files         298      298              
  Lines       21247    21258      +11     
  Branches     3457     3464       +7     
==========================================
+ Hits        13940    13997      +57     
+ Misses       5585     5527      -58     
- Partials     1722     1734      +12     
Files Coverage Δ
...ava/com/amazon/dlic/auth/ldap/util/LdapHelper.java 67.50% <ø> (ø)
...g/opensearch/security/support/WildcardMatcher.java 66.87% <83.33%> (+2.13%) ⬆️
...ic/auth/ldap/backend/LDAPAuthorizationBackend.java 62.71% <55.55%> (+0.21%) ⬆️
...zon/dlic/auth/ldap2/LDAPAuthorizationBackend2.java 47.98% <50.00%> (+16.75%) ⬆️

... and 2 files with indirect coverage changes

Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

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

The changes look good to me. Could you please look at the codecov-patch percentage drop ?

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
@cwperks
Copy link
Member Author

cwperks commented Feb 8, 2024

Blocked by: #4026

@DarshitChanpura DarshitChanpura mentioned this pull request Feb 9, 2024
3 tasks
Copy link
Collaborator

@derek-ho derek-ho left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @cwperks !

@cwperks
Copy link
Member Author

cwperks commented Feb 9, 2024

@DarshitChanpura Code cov drop is because the tests aren't currently looking for the debug log statements as Peter mentioned here. I will follow-up with tests that hit those lines.

@DarshitChanpura
Copy link
Member

okay. @cwperks Are you planning to add those in follow-up PR or this one?

@cwperks
Copy link
Member Author

cwperks commented Feb 9, 2024

@DarshitChanpura I will follow-up with the additional tests in another PR. The LogsRule is the best way to write tests that expect logging statements, but its not available outside the integrationTest package and the tests in this PR are not in the integrationTest package.

@peternied peternied merged commit 0bb31ca into opensearch-project:main Feb 12, 2024
81 of 82 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 12, 2024
…kend (#4025)

Signed-off-by: Maciej Mierzwa <dev.maciej.mierzwa@gmail.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Co-authored-by: Maciej Mierzwa <dev.maciej.mierzwa@gmail.com>
(cherry picked from commit 0bb31ca)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
peternied pushed a commit that referenced this pull request Feb 12, 2024
…thorization backend (#4043)

Backport 0bb31ca from #4025.

Signed-off-by: Maciej Mierzwa <dev.maciej.mierzwa@gmail.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Maciej Mierzwa <dev.maciej.mierzwa@gmail.com>
@cwperks cwperks added the v2.13.0 Issues targeting release v2.13.0 label Feb 13, 2024
dlin2028 pushed a commit to dlin2028/security that referenced this pull request May 1, 2024
…kend (opensearch-project#4025)

Signed-off-by: Maciej Mierzwa <dev.maciej.mierzwa@gmail.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Co-authored-by: Maciej Mierzwa <dev.maciej.mierzwa@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x backport to 2.x branch v2.13.0 Issues targeting release v2.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants