Add Kerberos authentication feature #2154

Ohasumi · 2024-12-05T13:09:38Z

Description

Add Kerberos authentication feature on opensearch-dashboard

Why these changes are required?

This add version will add feature to authentication by Kerberos via SPNEGO. So user can login without need 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 · 2024-12-06T17:01:13Z

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?

codecov · 2024-12-06T17:09:46Z

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 · 2024-12-10T16:54:19Z

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 · 2024-12-10T18:20:44Z

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 · 2024-12-11T07:13:33Z

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.

DarshitChanpura

Hi @Ohasumi, thank you for opening this feature PR. If you intend to continue working on this:

Please open an issue describing this feature.
Please update the title of this PR to be more descriptive.
Please sign all your commits to pass the DCO check.

Thank you for your contribution!

Signed-off-by: Thanakit Yuenyongphisit <[email protected]>

DarshitChanpura · 2025-06-09T19:16:49Z

server/index.ts

  }),
+  kerberos: schema.object({
+    jwt_siging_key: schema.string({
+      defaultValue: 'secret share between opensearch and dashboards',


can you change the default value to be empty?

+1
This is a placeholder, but defaults like this should never be hardcoded in production

DarshitChanpura · 2025-06-09T19:19:31Z

server/auth/types/kerberos/routes.ts

+    this.router.get(
+      {
+        path: KERBEROS_AUTH_LOGIN,
+        validate: false,


shouldn't we validate the path, specifically the nextUrl. see saml routes setup.

DarshitChanpura · 2025-06-09T19:20:53Z

server/auth/types/kerberos/routes.ts

+        let user: User;
+
+        try {
+          user = await this.securityClient.authinfo(request);


will this force login every-time this route is hit?

DarshitChanpura · 2025-06-09T19:21:52Z

server/auth/types/kerberos/routes.ts

+        // return to root
+        return response.redirected({
+          headers: {
+            location: '/',


we should redirect to the url passed in nextUrl query param.

DarshitChanpura · 2025-06-09T19:23:33Z

kerberos_notes.md

@@ -0,0 +1,120 @@
+### Required environment for Kerberos authentication ###


all the content from this file should be added to DEVELOPER_GUIDE.md. Also, please run it through grammar and spell-check.

DarshitChanpura · 2025-06-09T19:26:40Z

server/auth/types/kerberos/kerberos_auth.ts

+    config: SecurityPluginConfigType,
+    sessionStorageFactory: SessionStorageFactory<SecuritySessionCookie>,
+    router: IRouter,
+    esClient: ILegacyClusterClient,


Can we use IClusterClient as ILegacyClusterClient is marked as deprecated.

DarshitChanpura · 2025-06-09T19:29:16Z

server/auth/types/kerberos/kerberos_auth.ts

+    request: OpenSearchDashboardsRequest<unknown, unknown, unknown, any>
+  ): boolean {
+    return false;
+  }


can you update the comment to explain why we set it to return false?

DarshitChanpura · 2025-06-09T19:30:08Z

server/auth/types/kerberos/kerberos_auth.ts

+  }
+
+  getCookie(request: OpenSearchDashboardsRequest, authInfo: any): SecuritySessionCookie {
+    return {};


why do we not return cookie here?

DarshitChanpura · 2025-06-09T19:32:04Z

@Ohasumi Would you upload a screen recording of this feature in action? Will help understand the setup much better.

RyanL1997

Hi @Ohasumi , thanks for taking this on. I just left some general comments/questions. Also we may need to consider adding some unit/integration tests.

RyanL1997 · 2025-07-12T05:25:58Z

kerberos_notes.md

@@ -0,0 +1,120 @@
+### Required environment for Kerberos authentication ###


RyanL1997 · 2025-07-12T05:31:20Z

server/index.ts

  }),
+  kerberos: schema.object({
+    jwt_siging_key: schema.string({
+      defaultValue: 'secret share between opensearch and dashboards',


+1
This is a placeholder, but defaults like this should never be hardcoded in production

RyanL1997 · 2025-07-12T05:41:46Z

server/auth/types/kerberos/kerberos_auth.ts

+        },
+      });
+    } else {
+      return response.unauthorized({


In handleUnauthedRequest the WWW-Authenticate: 'Negotiate' header is only added on failure in routes. Consider adding it proactively in the handler for API requests to trigger browser negotiation.

RyanL1997 · 2025-07-12T05:43:27Z

server/auth/types/kerberos/routes.ts

+        // clear session
+        this.sessionStorageFactory.asScoped(request).clear();
+        const payload = {
+          exp: Date.now() + this.config.session.ttl,


Just being curious: do we need to a refresh mechanism for long-lived scenario?

RyanL1997 · 2025-07-12T05:46:28Z

server/auth/types/kerberos/routes.ts

+        } catch (error) {
+          context.security_plugin.logger.error(`Failed authentication: ${error}`);
+          return response.unauthorized({
+            body: `Kerberos authentication failed ${error}`,


Will this leak the stack trace when it fails? - if it has the stack trace, then we may need to intentionally blur/hide it.

RyanL1997 · 2025-07-12T05:49:52Z

server/auth/types/kerberos/routes.ts

+          default_tenant: user.default_tenant,
+        };
+
+        const encodedCredentials = sign(payload, this.config.kerberos.jwt_siging_key);


Consider add a error handling for sign() - wrap with a try/catch

RyanL1997 · 2025-07-12T05:54:07Z

server/auth/types/kerberos/routes.ts

+        }
+
+        // clear session
+        this.sessionStorageFactory.asScoped(request).clear();


this logout route clears session but doesnt invalidate the JWT. Since this JWTs are stateless, do we need a revocation for the actual token?

RyanL1997

Hi @Ohasumi , thanks for taking this on. I just left some general comments/questions. Also we may need to consider adding some unit/integration tests.

RyanL1997 · 2025-07-12T05:59:23Z

Also is this only for version 2.18.0?

Ohasumi requested review from DarshitChanpura, RyanL1997, cliu123, cwperks and derek-ho as code owners December 5, 2024 13:09

DarshitChanpura reviewed Mar 8, 2025

View reviewed changes

Ohasumi added 8 commits March 9, 2025 20:50

Add kerberos authentication

533bdce

Signed-off-by: Thanakit Yuenyongphisit <[email protected]>

Remove unused variable

7a4f01e

Signed-off-by: Thanakit Yuenyongphisit <[email protected]>

Add default value for jwt_siging_key

02b58ce

Signed-off-by: Thanakit Yuenyongphisit <[email protected]>

add defaults value for jwt_signing_key

0a5c88d

Signed-off-by: Thanakit Yuenyongphisit <[email protected]>

Add challenge header

836cebf

Signed-off-by: Thanakit Yuenyongphisit <[email protected]>

fix incorrect header

761955a

Signed-off-by: Thanakit Yuenyongphisit <[email protected]>

change default value for jwt signing key for more meanful

4e5e893

Signed-off-by: Thanakit Yuenyongphisit <[email protected]>

add notes for needed environtment

091b989

Signed-off-by: Thanakit Yuenyongphisit <[email protected]>

Ohasumi force-pushed the 2.18 branch from 141797e to 091b989 Compare March 9, 2025 13:50

DarshitChanpura changed the title ~~2.18~~ Add Kerberos authentication feature Jun 9, 2025

DarshitChanpura reviewed Jun 9, 2025

View reviewed changes

RyanL1997 reviewed Jul 12, 2025

View reviewed changes

		@@ -0,0 +1,120 @@
		### Required environment for Kerberos authentication ###

Add Kerberos authentication feature #2154

Are you sure you want to change the base?

Add Kerberos authentication feature #2154

Uh oh!

Conversation

Ohasumi commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Category

Why these changes are required?

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

Issues Resolved

Testing

Check List

Uh oh!

cwperks commented Dec 6, 2024

Uh oh!

codecov bot commented Dec 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Ohasumi commented Dec 10, 2024

Uh oh!

cwperks commented Dec 10, 2024

Uh oh!

Ohasumi commented Dec 11, 2024

Uh oh!

DarshitChanpura left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DarshitChanpura commented Jun 9, 2025

Uh oh!

RyanL1997 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RyanL1997 left a comment

Choose a reason for hiding this comment

Uh oh!

RyanL1997 commented Jul 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Ohasumi commented Dec 5, 2024 •

edited

Loading

codecov bot commented Dec 6, 2024 •

edited

Loading