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

2.18 #2154

Open
wants to merge 8 commits into
base: 2.18
Choose a base branch
from
Open

2.18 #2154

wants to merge 8 commits into from

Conversation

Ohasumi
Copy link

@Ohasumi Ohasumi commented Dec 5, 2024

Description

Add kerberos authentication via SPNEGO for dashboards

Category

New feature

Why these changes are required?

This add version will add feature to authentication by Kerberos via SPNEGO. So user can login without need to of password in environment that Kerberos are existed.

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

This only make change to enable new authentication method

Issues Resolved

Testing

Integration testing by using google chrome setting policy to enable [AuthServerAllowlist] for dashboards server with both client and server are communicate with Kerberos server.
[Please provide details of testing done: unit testing, integration testing and manual 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.

@cwperks
Copy link
Member

cwperks commented Dec 6, 2024

Thank you for the PR @Ohasumi. Could you please sign the commits and add some unit tests? Would it be possible to write an integration test or provide steps on how to set up testing for this so another developer can verify the change?

Copy link

codecov bot commented Dec 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.46%. Comparing base (ef72c90) to head (141797e).

Additional details and impacted files
@@           Coverage Diff           @@
##             2.18    #2154   +/-   ##
=======================================
  Coverage   71.46%   71.46%           
=======================================
  Files          97       97           
  Lines        2649     2649           
  Branches      411      403    -8     
=======================================
  Hits         1893     1893           
  Misses        641      641           
  Partials      115      115           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Ohasumi
Copy link
Author

Ohasumi commented Dec 10, 2024

Thank you for the PR @Ohasumi. Could you please sign the commits and add some unit tests? Would it be possible to write an integration test or provide steps on how to set up testing for this so another developer can verify the change?

@cwperks I sign the commits and add notes for environment I'm using. But I'm not sure how to add some unit test, since it required valid kerberos token passing to opensearch core to validate then using jsonwebtoken to sign the user data to be use as cookie.

If anything I can helps please tell me, I will do my best.

@cwperks
Copy link
Member

cwperks commented Dec 10, 2024

Thank you for the PR @Ohasumi. Could you please sign the commits and add some unit tests? Would it be possible to write an integration test or provide steps on how to set up testing for this so another developer can verify the change?

@cwperks I sign the commits and add notes for environment I'm using. But I'm not sure how to add some unit test, since it required valid kerberos token passing to opensearch core to validate then using jsonwebtoken to sign the user data to be use as cookie.

If anything I can helps please tell me, I will do my best.

Can you provide a markdown document or a Github comment outlining steps used to test?

@Ohasumi
Copy link
Author

Ohasumi commented Dec 11, 2024

Thank you for the PR @Ohasumi. Could you please sign the commits and add some unit tests? Would it be possible to write an integration test or provide steps on how to set up testing for this so another developer can verify the change?

@cwperks I sign the commits and add notes for environment I'm using. But I'm not sure how to add some unit test, since it required valid kerberos token passing to opensearch core to validate then using jsonwebtoken to sign the user data to be use as cookie.

If anything I can helps please tell me, I will do my best.

Can you provide a markdown document or a Github comment outlining steps used to test?

I added setup environment note as markdown "kerberos_notes.md" in my last commit, which contained most required environment for test. For testing when access dashboard it should redirect to authentication page then browser should attached kerberos ticket with it,after passed the authentication process jsontoken with user credentials should attached as cookie.

I hope this might help clarify my test.

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.

2 participants