Fixes saml login flow to work with anonymous auth #1839

DarshitChanpura · 2024-03-21T01:37:12Z

Description

This PR fixes 2 things:

Anonymous login button would not show if only 1 auth option was supplied
Broken SAML auth flow when anonymous auth is enabled.
Automatic login as anonymous user upon SAML user logout. (Discovered while fixing this bug)

Why these changes are required?

For SAML and anonymous auth options to function when both options are enabled
To prevent auto login as anonymous user upon SAML user logout

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

At present, SAML auth flow is broken since anonymous auth automatically assumes the Anonymous user when no auth credentials are found. See security plugin PR[1] for more details.

[1] - Companion PR: opensearch-project/security#4152

Issues Resolved

Testing

Automated + Manual

Manual testing:

Screen.Recording.2024-04-11.at.11.55.53.AM.mov

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: Darshit Chanpura <dchanp@amazon.com>

…orrectly Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

…ion header Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

codecov · 2024-03-21T01:47:54Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.16%. Comparing base (64995fc) to head (4317901).

❗ Current head 4317901 differs from pull request most recent head 1ad7343. Consider uploading reports for the commit 1ad7343 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1839      +/-   ##
==========================================
- Coverage   68.25%   68.16%   -0.09%     
==========================================
  Files          94       94              
  Lines        2416     2422       +6     
  Branches      330      330              
==========================================
+ Hits         1649     1651       +2     
- Misses        689      694       +5     
+ Partials       78       77       -1

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

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

server/auth/types/basic/basic_auth.ts

test/constant.ts

server/auth/types/openid/routes.ts

stephen-crawford

Overall looks good, two small comments.

server/auth/types/authentication_type.ts

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

server/backend/opensearch_security_client.ts

server/backend/opensearch_security_plugin.ts

server/readonly/readonly_service.ts

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

derek-ho

You are the 🐐

server/backend/opensearch_security_client.ts

server/auth/types/saml/routes.ts

server/auth/types/basic/basic_auth.ts

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

cwperks

LGTM! Thank you @DarshitChanpura!

FYI I don't think there is a way to setup Basic Auth + Anonymous without it automatically logging in as anonymous when first loading OSD. I think the issue is in auth_handler_factory.ts where even if multi-auth is enabled and only one auth_type is in the list it will create an instance of the single auth type instead of using MultipleAuthentication

* Fixes anonymous auth flow to work with SAML Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Adds hardcoded credentials for anonymous user Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Updates basic auth header to be a config constant Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Removes unneeded usage of anonymous auth header constant Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Updates logic to display anonymous auth login button Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Adds test to check whether anonymous auth login button is displayed correctly Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes integrationtests Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Adds integration tests for anonymous auth login with basic authorization header Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Generates random password for anonymous user Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes lint errors Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Adds saml auth header to differentiate saml requests Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes linter errors Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes basic auth tests Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Removes console loggers Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes lint error Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Addresses feedback Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Resolves #1840 Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Replace magic value with constant Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Renames query param and removes unused variables Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Uses enum instead of magic constant Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Extracts template function to a separate util file Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Renames test Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Removes unnecessary modifications required to solve this bug Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes import Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Removes unused param Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Removes unused method param Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Removes incorrect method param Signed-off-by: Darshit Chanpura <dchanp@amazon.com> --------- Signed-off-by: Darshit Chanpura <dchanp@amazon.com> (cherry picked from commit 681d1b1)

* Fixes anonymous auth flow to work with SAML Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Adds hardcoded credentials for anonymous user Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Updates basic auth header to be a config constant Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Removes unneeded usage of anonymous auth header constant Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Updates logic to display anonymous auth login button Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Adds test to check whether anonymous auth login button is displayed correctly Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes integrationtests Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Adds integration tests for anonymous auth login with basic authorization header Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Generates random password for anonymous user Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes lint errors Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Adds saml auth header to differentiate saml requests Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes linter errors Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes basic auth tests Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Removes console loggers Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes lint error Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Addresses feedback Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Resolves #1840 Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Replace magic value with constant Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Renames query param and removes unused variables Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Uses enum instead of magic constant Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Extracts template function to a separate util file Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Renames test Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Removes unnecessary modifications required to solve this bug Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes import Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Removes unused param Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Removes unused method param Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Removes incorrect method param Signed-off-by: Darshit Chanpura <dchanp@amazon.com> --------- Signed-off-by: Darshit Chanpura <dchanp@amazon.com> (cherry picked from commit 681d1b1) Co-authored-by: Darshit Chanpura <35282393+DarshitChanpura@users.noreply.github.com>

DarshitChanpura added 12 commits March 5, 2024 22:59

Fixes anonymous auth flow to work with SAML

199683c

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Adds hardcoded credentials for anonymous user

cbdf7a6

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Updates basic auth header to be a config constant

a0f2db4

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Removes unneeded usage of anonymous auth header constant

3c0c2f9

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Updates logic to display anonymous auth login button

8322a23

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Adds test to check whether anonymous auth login button is displayed c…

f331377

…orrectly Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Fixes integrationtests

63abb41

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Adds integration tests for anonymous auth login with basic authorizat…

5492d2a

…ion header Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Generates random password for anonymous user

9d47261

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Fixes lint errors

d682701

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Adds saml auth header to differentiate saml requests

ca8b4ef

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Fixes linter errors

f63bbf2

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

DarshitChanpura requested review from cliu123, cwperks, davidlago, peternied, RyanL1997 and stephen-crawford as code owners March 21, 2024 01:37

DarshitChanpura marked this pull request as draft March 21, 2024 01:37

DarshitChanpura mentioned this pull request Mar 21, 2024

Fixes anonymous auth flow to work with SAML #1817

Closed

3 tasks

Merge remote-tracking branch 'upstream/main' into fixes-saml-login-flow

43c8499

DarshitChanpura mentioned this pull request Mar 21, 2024

Adds saml auth header to differentiate saml requests and prevents auto login as anonymous user when basic authentication fails opensearch-project/security#4152

Merged

2 tasks

Fixes basic auth tests

573c715

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

DarshitChanpura changed the title ~~Fixes saml login flow~~ Fixes saml login flow to work with anonymous auth Mar 21, 2024

Removes console loggers

b9b9911

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

DarshitChanpura commented Mar 21, 2024

View reviewed changes

server/auth/types/basic/basic_auth.ts Outdated Show resolved Hide resolved

test/constant.ts Show resolved Hide resolved

derek-ho reviewed Mar 21, 2024

View reviewed changes

server/auth/types/openid/routes.ts Outdated Show resolved Hide resolved

DarshitChanpura mentioned this pull request Mar 21, 2024

[BUG] Having Anonymous and SAML as sign in options does not work. SAML Login fails when Anonymous is enabled #1731

Closed

Merge remote-tracking branch 'upstream/main' into fixes-saml-login-flow

87ac6e0

Merge remote-tracking branch 'upstream/main' into fixes-saml-login-flow

7b68f1c

DarshitChanpura marked this pull request as ready for review April 9, 2024 17:05

DarshitChanpura added the backport 2.x backport to 2.x branch label Apr 9, 2024

stephen-crawford previously approved these changes Apr 9, 2024

View reviewed changes

server/auth/types/authentication_type.ts Outdated Show resolved Hide resolved

server/auth/types/authentication_type.ts Outdated Show resolved Hide resolved

Uses enum instead of magic constant

2a1289d

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

DarshitChanpura dismissed stephen-crawford’s stale review via 2a1289d April 9, 2024 18:26

derek-ho reviewed Apr 9, 2024

View reviewed changes

server/backend/opensearch_security_client.ts Outdated Show resolved Hide resolved

derek-ho reviewed Apr 9, 2024

View reviewed changes

server/backend/opensearch_security_plugin.ts Outdated Show resolved Hide resolved

derek-ho reviewed Apr 9, 2024

View reviewed changes

server/readonly/readonly_service.ts Outdated Show resolved Hide resolved

DarshitChanpura added 2 commits April 9, 2024 16:30

Extracts template function to a separate util file

35a7f7e

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Renames test

77608a0

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

derek-ho previously approved these changes Apr 9, 2024

View reviewed changes

cwperks reviewed Apr 10, 2024

View reviewed changes

server/backend/opensearch_security_client.ts Show resolved Hide resolved

cwperks reviewed Apr 10, 2024

View reviewed changes

server/auth/types/saml/routes.ts Show resolved Hide resolved

cwperks reviewed Apr 10, 2024

View reviewed changes

server/auth/types/basic/basic_auth.ts Outdated Show resolved Hide resolved

DarshitChanpura dismissed derek-ho’s stale review via 95067e8 April 11, 2024 16:52

Removes unnecessary modifications required to solve this bug

d756b6c

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

DarshitChanpura force-pushed the fixes-saml-login-flow branch from 95067e8 to d756b6c Compare April 11, 2024 16:54

DarshitChanpura added 4 commits April 11, 2024 13:10

Fixes import

f03d3ab

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Removes unused param

d58301f

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Removes unused method param

2819b10

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

Removes incorrect method param

1ad7343

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

derek-ho approved these changes Apr 11, 2024

View reviewed changes

cwperks approved these changes Apr 12, 2024

View reviewed changes

derek-ho merged commit 681d1b1 into opensearch-project:main Apr 12, 2024
13 checks passed

opensearch-trigger-bot bot mentioned this pull request Apr 12, 2024

[Backport 2.x] Fixes saml login flow to work with anonymous auth #1887

Merged

derek-ho mentioned this pull request Apr 18, 2024

[BUG] SAML and SAML multi auth tests are flaky #1900

Closed

derek-ho mentioned this pull request May 14, 2024

[FEATURE] Automatically login as anonymous #1957

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes saml login flow to work with anonymous auth #1839

Fixes saml login flow to work with anonymous auth #1839

DarshitChanpura commented Mar 21, 2024 •

edited

Loading

codecov bot commented Mar 21, 2024 •

edited

Loading

stephen-crawford left a comment

derek-ho left a comment

cwperks left a comment

Fixes saml login flow to work with anonymous auth #1839

Fixes saml login flow to work with anonymous auth #1839

Conversation

DarshitChanpura commented Mar 21, 2024 • edited Loading

Description

Category

Why these changes are required?

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

Issues Resolved

Testing

Check List

codecov bot commented Mar 21, 2024 • edited Loading

Codecov Report

stephen-crawford left a comment

Choose a reason for hiding this comment

derek-ho left a comment

Choose a reason for hiding this comment

cwperks left a comment

Choose a reason for hiding this comment

DarshitChanpura commented Mar 21, 2024 •

edited

Loading

codecov bot commented Mar 21, 2024 •

edited

Loading