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

#2616 #2730 Added additional extension settings bwcPluginMode and extensionDistinguishedNames #2862

Conversation

samuelcostae
Copy link
Contributor

@samuelcostae samuelcostae commented Jun 15, 2023

Description

Extension Projects

Added additional extension settings bwcPluginMode and extensionDistinguishedNames

Issues Resolved

#2616 #2730

Testing

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.

…nsion settings bwcPluginMode and extensionDistinguishedNames

Signed-off-by: scosta <[email protected]>
@codecov
Copy link

codecov bot commented Jun 15, 2023

Codecov Report

Merging #2862 (e65e63f) into main (ceb5ad2) will decrease coverage by 0.02%.
The diff coverage is 20.00%.

@@             Coverage Diff              @@
##               main    #2862      +/-   ##
============================================
- Coverage     61.60%   61.58%   -0.02%     
+ Complexity     3414     3413       -1     
============================================
  Files           266      266              
  Lines         18917    18922       +5     
  Branches       3303     3303              
============================================
+ Hits          11653    11654       +1     
- Misses         5669     5673       +4     
  Partials       1595     1595              
Impacted Files Coverage Δ
.../opensearch/security/OpenSearchSecurityPlugin.java 79.60% <0.00%> (-0.63%) ⬇️
...g/opensearch/security/support/ConfigConstants.java 94.73% <100.00%> (+0.29%) ⬆️

@cwperks
Copy link
Member

cwperks commented Jun 15, 2023

@samuelcostae Would you be able to add the settings in separate PRs, but also include an implementation for the setting that makes it useful? Let me know if you have any questions about either setting.

  1. For extension_dns this should emulate node_dns and define a list of trusted principals for mutual TLS between opensearch <-> extension
  2. bwcPluginMode may not be the best name for this, but we can also discuss that on a PR. What this setting is for is to send backend roles (decrypted) as a claim in an on-behalf-of token so that plugin/extensions that rely on those values today have a mechanism for getting them. i.e. Anomaly Detection relies on backend roles to implement the security model for anomaly detection.

@peternied
Copy link
Member

@owaiskazi19 is working on a change to dynamically register extensions. Could you take a look and see if this settings will still be available, or if we need to handle this in a more dynamic way?

@stephen-crawford
Copy link
Contributor

stephen-crawford commented Jun 26, 2023

Hi @samuelcostae, I am just checking in about the state of this PR. It looks like it has been in draft form for some time now so I wanted to check with you whether there was anything you could use some help with or if you felt it was ready to move forward with review?

@samuelcostae
Copy link
Contributor Author

Hi @samuelcostae, I am just checking in about the state of this PR. It looks like it has been in draft form for some time now so I wanted to check with you whether there was anything you could use some help with or if you felt it was ready to move forward with review?

Hi Stephen, with the scope extended by Craig's request, there is a good bit more of work to be done, and I didn't get the chance to work on it yet.

Also, as craig asked to commit the DNs separately, we could even close this PR and I can create a new one when ready

@stephen-crawford
Copy link
Contributor

Hi @samuelcostae, no worries. I just wanted to check in and make sure we were on the same page. I am going to close this, but if you end up opening a new version let me know and I can help you get moving on the expanded scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants