#907 kerberos authentication

[samuelcostae]

Description

Include OS Dashboards Security Plugins support for Kerberos

Category

New feature

Why these changes are required?

Security Plugin has support for Kerberos in the backend, but Dashboards Plugin doesn't support it.

What is the old behavior before changes and new behavior after changes?

Issues Resolved

#907

Testing

none yet

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.

[codecov]

Codecov Report

Merging #1575 (e09f861) into main (27c37d4) will decrease coverage by 0.19%.
The diff coverage is 12.50%.

@@            Coverage Diff             @@
##             main    #1575      +/-   ##
==========================================
- Coverage   67.99%   67.80%   -0.19%     
==========================================
  Files          93       93              
  Lines        2340     2348       +8     
  Branches      317      318       +1     
==========================================
+ Hits         1591     1592       +1     
- Misses        720      727       +7     
  Partials       29       29              

Files	Coverage Δ
public/apps/account/log-out-button.tsx	78.57% <50.00%> (-4.77%)	⬇️
public/apps/account/utils.tsx	3.57% <0.00%> (-0.43%)	⬇️
public/apps/login/login-page.tsx	80.61% <0.00%> (-2.55%)	⬇️

[peternied]

Thanks for opening this PR @samuelcostae is there anything you'd like reviewed before you flip this PR from draft to ready to merge?

Otherwise, I'll provide feedback after you've changed the status.

[samuelcostae]

Hi @peternied
I'm waiting on the feedback regarding integration testing approach (or not) as we had discussed in the meeting last week

#907

[peternied]

I've replied on that issue, I don't think we should move forward with this change without end to end validation which isn't in scope for the moment, I think we should close out this PR. What do you think?

[samuelcostae]

Hi @peternied, In my opinion we should do some basic unit tests and review/ try to merge the PR. What leads me to this conclusion are the following points:

• It is a user request based on real life use case scenario and could also be used by others
• It is currently a feature that is supported by the OS Security plugin and not by the OSD Security Plugin, which is a inconsistency/might be misleading
• The code changes have little to no impact on existing features, as mostly the code changes never get executed unless "Kerberos" is explicitly set as the Authentication mechanism in the config files.
• The code changes do not involve complex logic or library dependencies
• Setting up and configuring the kerberos ecosystem was one of the main time consuming tasks on this. If this is revisited in the future, even if the code in the PR is re-used, it would still required repeated work to be done on the setup and local testing side.
• Excluding the end to end test, I believe very little work is required to complete this.

In short, I agree that merging the changes without the end-to-end tests is a risk, but based on the points above, I would say it is a small one.

But counter-arguing my own conclusion I would say:

• Having a bug in a feature is a higher "responsibility" than not having such feature at all, which might lead to bug fix work having to be prioritised in the future.
• I'm not familiar on how popular Kerberos is as a authentication mechanism in OpenSearch, it could be the case that it would be a feature that would not benefit many users
• One shouldn't be hostage to the "Sunken costs fallacy".
• Possibly unknown consequences down the line that would only become clear in hindsight.

Let me know your thought on this,

Thanks

[samuelcostae]

Hi @peternied, In my opinion we should do some basic unit tests and review/ try to merge the PR. What leads me to this conclusion are the following points:

• It is a user request based on real life use case scenario and could also be used by others
• It is currently a feature that is supported by the OS Security plugin and not by the OSD Security Plugin, which is a inconsistency/might be misleading
• The code changes have little to no impact on existing features, as mostly the code changes never get executed unless "Kerberos" is explicitly set as the Authentication mechanism in the config files.
• The code changes do not involve complex logic or library dependencies
• Setting up and configuring the kerberos ecosystem was one of the main time consuming tasks on this. If this is revisited in the future, even if the code in the PR is re-used, it would still required repeated work to be done on the setup and local testing side.
• Excluding the end to end test, I believe very little work is required to complete this.

In short, I agree that merging the changes without the end-to-end tests is a risk, but based on the points above, I would say it is a small one.

But counter-arguing my own conclusion I would say:

• Having a bug in a feature is a higher "responsibility" than not having such feature at all, which might lead to bug fix work having to be prioritised in the future.
• I'm not familiar on how popular Kerberos is as a authentication mechanism in OpenSearch, it could be the case that it would be a feature that would not benefit many users
• One shouldn't be hostage to the "Sunken costs fallacy".
• Possibly unknown consequences down the line that would only become clear in hindsight.

Let me know your thought on this,

Thanks

Confirmed with @davidlago in the last call that we are not proceeding forward at the moment