Fix issue when using OpenID Authentication with serverBasePath #1899

cwperks · 2024-04-18T19:44:58Z

Description

This PR fixes a bug where the security-dashboards-plugin would redirect back to / instead of /{serverBasePath} after authenticating with OpenID.

This PR also performs all SAML, OpenID and Multi-auth cypress tests both without a basePath and with a basePath

Issues Resolved

[BUG] OpenID redirects to "/" after upgrading opensearch to 2.12.0 #1897

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.

Signed-off-by: Craig Perkins <[email protected]>

codecov · 2024-04-19T21:12:18Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.94%. Comparing base (347f579) to head (bd82997).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1899   +/-   ##
=======================================
  Coverage   69.94%   69.94%           
=======================================
  Files          97       97           
  Lines        2505     2505           
  Branches      336      343    +7     
=======================================
  Hits         1752     1752           
  Misses        672      672           
  Partials       81       81

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

Signed-off-by: Craig Perkins <[email protected]>

derek-ho

Thanks for improving our testing infrastructure, base path is not something that we usually have in mind as developers, if we are using yarn start --no-base-path, but it is a real world use case. A few minor logistical comments, but otherwise looks good 👍

Signed-off-by: Craig Perkins <[email protected]>

package.json

Signed-off-by: Craig Perkins <[email protected]>

DarshitChanpura

The changes look good to me @cwperks. Could you please reply to/resolve the 2 open conversations?

cwperks · 2024-05-08T12:39:00Z

@DarshitChanpura Replied to both outstanding comments. I was not able to figure out a way to reduce it to a single step. Can those be addressed in a follow-up PR?

.github/actions/run-cypress-tests/action.yml

.github/workflows/cypress-test-multiauth-e2e.yml

package.json

* Test OIDC with basePath Signed-off-by: Craig Perkins <[email protected]> * Run OIDC tests with basePath Signed-off-by: Craig Perkins <[email protected]> * Run all tests with basePath Signed-off-by: Craig Perkins <[email protected]> * Fix kibana url Signed-off-by: Craig Perkins <[email protected]> * Remove requirement on nextUrl Signed-off-by: Craig Perkins <[email protected]> * Remove duplicate Signed-off-by: Craig Perkins <[email protected]> * Add basePath to acs endpoint Signed-off-by: Craig Perkins <[email protected]> * Update snapshot Signed-off-by: Craig Perkins <[email protected]> * Start IdP where acs contains the basePath Signed-off-by: Craig Perkins <[email protected]> * es:lint --fix Signed-off-by: Craig Perkins <[email protected]> * Generalize SAML tests to include basePath Signed-off-by: Craig Perkins <[email protected]> * Add default Signed-off-by: Craig Perkins <[email protected]> * Use -z Signed-off-by: Craig Perkins <[email protected]> * Cypress env Signed-off-by: Craig Perkins <[email protected]> * Cypress env Signed-off-by: Craig Perkins <[email protected]> * Handle undefined Signed-off-by: Craig Perkins <[email protected]> * Always pass basePath Signed-off-by: Craig Perkins <[email protected]> * Remove ampersand Signed-off-by: Craig Perkins <[email protected]> * Pass basePath in yarn_command Signed-off-by: Craig Perkins <[email protected]> * re-add osd_base_path Signed-off-by: Craig Perkins <[email protected]> * Try ternary Signed-off-by: Craig Perkins <[email protected]> * Start with basePath Signed-off-by: Craig Perkins <[email protected]> * Use separate steps Signed-off-by: Craig Perkins <[email protected]> * Update shortenUrl Signed-off-by: Craig Perkins <[email protected]> * Remove basePath in acs endpoint and fix shorten URL Signed-off-by: Craig Perkins <[email protected]> * Comma-separated env Signed-off-by: Craig Perkins <[email protected]> * update oidc_auth_test.spec.js Signed-off-by: Craig Perkins <[email protected]> * Check cookie after login Signed-off-by: Craig Perkins <[email protected]> * failOnStatusCode false Signed-off-by: Craig Perkins <[email protected]> * mimic SAML Signed-off-by: Craig Perkins <[email protected]> * mimic SAML Signed-off-by: Craig Perkins <[email protected]> * Remove cy.origin Signed-off-by: Craig Perkins <[email protected]> * Add cy.origin back Signed-off-by: Craig Perkins <[email protected]> * Try wrapping in cy.url Signed-off-by: Craig Perkins <[email protected]> * Visit directly Signed-off-by: Craig Perkins <[email protected]> * Try localhost Signed-off-by: Craig Perkins <[email protected]> * Comment out test for now Signed-off-by: Craig Perkins <[email protected]> * Add another request after logout Signed-off-by: Craig Perkins <[email protected]> * Wait for logout Signed-off-by: Craig Perkins <[email protected]> * Small update Signed-off-by: Craig Perkins <[email protected]> * Add base_redirect_url Signed-off-by: Craig Perkins <[email protected]> * Explicitly add minimist as dev dependency Signed-off-by: Craig Perkins <[email protected]> --------- Signed-off-by: Craig Perkins <[email protected]> Co-authored-by: Darshit Chanpura <[email protected]> Co-authored-by: Jialiang Liang <[email protected]> (cherry picked from commit 9662025)

#1946) * Test OIDC with basePath Signed-off-by: Craig Perkins <[email protected]> * Run OIDC tests with basePath Signed-off-by: Craig Perkins <[email protected]> * Run all tests with basePath Signed-off-by: Craig Perkins <[email protected]> * Fix kibana url Signed-off-by: Craig Perkins <[email protected]> * Remove requirement on nextUrl Signed-off-by: Craig Perkins <[email protected]> * Remove duplicate Signed-off-by: Craig Perkins <[email protected]> * Add basePath to acs endpoint Signed-off-by: Craig Perkins <[email protected]> * Update snapshot Signed-off-by: Craig Perkins <[email protected]> * Start IdP where acs contains the basePath Signed-off-by: Craig Perkins <[email protected]> * es:lint --fix Signed-off-by: Craig Perkins <[email protected]> * Generalize SAML tests to include basePath Signed-off-by: Craig Perkins <[email protected]> * Add default Signed-off-by: Craig Perkins <[email protected]> * Use -z Signed-off-by: Craig Perkins <[email protected]> * Cypress env Signed-off-by: Craig Perkins <[email protected]> * Cypress env Signed-off-by: Craig Perkins <[email protected]> * Handle undefined Signed-off-by: Craig Perkins <[email protected]> * Always pass basePath Signed-off-by: Craig Perkins <[email protected]> * Remove ampersand Signed-off-by: Craig Perkins <[email protected]> * Pass basePath in yarn_command Signed-off-by: Craig Perkins <[email protected]> * re-add osd_base_path Signed-off-by: Craig Perkins <[email protected]> * Try ternary Signed-off-by: Craig Perkins <[email protected]> * Start with basePath Signed-off-by: Craig Perkins <[email protected]> * Use separate steps Signed-off-by: Craig Perkins <[email protected]> * Update shortenUrl Signed-off-by: Craig Perkins <[email protected]> * Remove basePath in acs endpoint and fix shorten URL Signed-off-by: Craig Perkins <[email protected]> * Comma-separated env Signed-off-by: Craig Perkins <[email protected]> * update oidc_auth_test.spec.js Signed-off-by: Craig Perkins <[email protected]> * Check cookie after login Signed-off-by: Craig Perkins <[email protected]> * failOnStatusCode false Signed-off-by: Craig Perkins <[email protected]> * mimic SAML Signed-off-by: Craig Perkins <[email protected]> * mimic SAML Signed-off-by: Craig Perkins <[email protected]> * Remove cy.origin Signed-off-by: Craig Perkins <[email protected]> * Add cy.origin back Signed-off-by: Craig Perkins <[email protected]> * Try wrapping in cy.url Signed-off-by: Craig Perkins <[email protected]> * Visit directly Signed-off-by: Craig Perkins <[email protected]> * Try localhost Signed-off-by: Craig Perkins <[email protected]> * Comment out test for now Signed-off-by: Craig Perkins <[email protected]> * Add another request after logout Signed-off-by: Craig Perkins <[email protected]> * Wait for logout Signed-off-by: Craig Perkins <[email protected]> * Small update Signed-off-by: Craig Perkins <[email protected]> * Add base_redirect_url Signed-off-by: Craig Perkins <[email protected]> * Explicitly add minimist as dev dependency Signed-off-by: Craig Perkins <[email protected]> --------- Signed-off-by: Craig Perkins <[email protected]> Co-authored-by: Darshit Chanpura <[email protected]> Co-authored-by: Jialiang Liang <[email protected]> (cherry picked from commit 9662025) Co-authored-by: Craig Perkins <[email protected]>

cwperks added 16 commits April 18, 2024 09:00

Test OIDC with basePath

98b1013

Signed-off-by: Craig Perkins <[email protected]>

Run OIDC tests with basePath

6cc0d8f

Signed-off-by: Craig Perkins <[email protected]>

Run all tests with basePath

c41fc38

Signed-off-by: Craig Perkins <[email protected]>

Fix kibana url

03c3030

Signed-off-by: Craig Perkins <[email protected]>

Remove requirement on nextUrl

937fa28

Signed-off-by: Craig Perkins <[email protected]>

Remove duplicate

f4ced49

Signed-off-by: Craig Perkins <[email protected]>

Add basePath to acs endpoint

34969d2

Signed-off-by: Craig Perkins <[email protected]>

Update snapshot

7f61259

Signed-off-by: Craig Perkins <[email protected]>

Start IdP where acs contains the basePath

124ab4c

Signed-off-by: Craig Perkins <[email protected]>

es:lint --fix

d6f5089

Signed-off-by: Craig Perkins <[email protected]>

Generalize SAML tests to include basePath

c95ded8

Signed-off-by: Craig Perkins <[email protected]>

Add default

30ae872

Signed-off-by: Craig Perkins <[email protected]>

Use -z

23ee814

Signed-off-by: Craig Perkins <[email protected]>

Cypress env

f9ae8b8

Signed-off-by: Craig Perkins <[email protected]>

Cypress env

8c912e1

Signed-off-by: Craig Perkins <[email protected]>

Handle undefined

80d753c

Signed-off-by: Craig Perkins <[email protected]>

cwperks added 13 commits April 19, 2024 20:41

Always pass basePath

8557219

Signed-off-by: Craig Perkins <[email protected]>

Remove ampersand

f3c0733

Signed-off-by: Craig Perkins <[email protected]>

Pass basePath in yarn_command

940391d

Signed-off-by: Craig Perkins <[email protected]>

re-add osd_base_path

abe86e0

Signed-off-by: Craig Perkins <[email protected]>

Try ternary

1600362

Signed-off-by: Craig Perkins <[email protected]>

Start with basePath

61e07a0

Signed-off-by: Craig Perkins <[email protected]>

Use separate steps

48b8f5c

Signed-off-by: Craig Perkins <[email protected]>

Update shortenUrl

12d7855

Signed-off-by: Craig Perkins <[email protected]>

Remove basePath in acs endpoint and fix shorten URL

37b75b6

Signed-off-by: Craig Perkins <[email protected]>

Comma-separated env

beb2dd6

Signed-off-by: Craig Perkins <[email protected]>

update oidc_auth_test.spec.js

d4e2fa6

Signed-off-by: Craig Perkins <[email protected]>

Check cookie after login

e899fb7

Signed-off-by: Craig Perkins <[email protected]>

failOnStatusCode false

155497d

Signed-off-by: Craig Perkins <[email protected]>

derek-ho reviewed Apr 23, 2024

View reviewed changes

cwperks added 2 commits April 26, 2024 12:39

Merge branch 'main' into oidc-basepath-fix

410c7f6

Explicitly add minimist as dev dependency

d1a7be0

Signed-off-by: Craig Perkins <[email protected]>

cwperks commented Apr 26, 2024

View reviewed changes

package.json Show resolved Hide resolved

peternied mentioned this pull request Apr 26, 2024

[RELEASE] Release version 2.14.0 #1846

Closed

23 tasks

Merge branch 'main' into oidc-basepath-fix

1eb2209

RyanL1997 previously approved these changes Apr 30, 2024

View reviewed changes

Merge branch 'main' into oidc-basepath-fix

a071f94

cwperks mentioned this pull request May 2, 2024

[BUG] url re-direction after login does not work #926

Closed

Merge branch 'main' into oidc-basepath-fix

a1ed129

Signed-off-by: Craig Perkins <[email protected]>

cwperks dismissed RyanL1997’s stale review via a1ed129 May 3, 2024 01:17

cwperks added the backport 2.x backport to 2.x branch label May 3, 2024

DarshitChanpura reviewed May 3, 2024

View reviewed changes

derek-ho reviewed May 8, 2024

View reviewed changes

.github/actions/run-cypress-tests/action.yml Show resolved Hide resolved

derek-ho reviewed May 8, 2024

View reviewed changes

.github/workflows/cypress-test-multiauth-e2e.yml Show resolved Hide resolved

derek-ho reviewed May 8, 2024

View reviewed changes

package.json Show resolved Hide resolved

DarshitChanpura approved these changes May 8, 2024

View reviewed changes

derek-ho approved these changes May 8, 2024

View reviewed changes

RyanL1997 approved these changes May 8, 2024

View reviewed changes

DarshitChanpura and others added 2 commits May 8, 2024 17:25

Merge branch 'main' into oidc-basepath-fix

7c9718f

Merge branch 'main' into oidc-basepath-fix

bd82997

cwperks merged commit 9662025 into opensearch-project:main May 10, 2024
20 checks passed

opensearch-trigger-bot bot mentioned this pull request May 10, 2024

[Backport 2.x] Fix issue when using OpenID Authentication with serverBasePath #1946

Merged

fbegyn mentioned this pull request May 17, 2024

(fleet/opensearch/logging) Opensearch logging cluster config lsst-it/k8s-cookbook#397

Merged

derek-ho mentioned this pull request May 20, 2024

Redirect back to the originally-requested url after authentication with SSO - OIDC #1953

Open

derek-ho mentioned this pull request Jun 13, 2024

Security Plugin - OpenID Connect Authentication returns 401 Unauthorized since 2.14 #1974

Closed

stephen-crawford mentioned this pull request Aug 12, 2024

[BUG] Default Route not applying with SAML #2081

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issue when using OpenID Authentication with serverBasePath #1899

Fix issue when using OpenID Authentication with serverBasePath #1899

cwperks commented Apr 18, 2024

codecov bot commented Apr 19, 2024 •

edited

Loading

derek-ho left a comment

DarshitChanpura left a comment

cwperks commented May 8, 2024

Fix issue when using OpenID Authentication with serverBasePath #1899

Fix issue when using OpenID Authentication with serverBasePath #1899

Conversation

cwperks commented Apr 18, 2024

Description

Category

Issues Resolved

Check List

codecov bot commented Apr 19, 2024 • edited Loading

Codecov Report

derek-ho left a comment

Choose a reason for hiding this comment

DarshitChanpura left a comment

Choose a reason for hiding this comment

cwperks commented May 8, 2024

codecov bot commented Apr 19, 2024 •

edited

Loading