-
Notifications
You must be signed in to change notification settings - Fork 274
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
Adds saml auth header to differentiate saml requests and prevents auto login as anonymous user when basic authentication fails #4152
Conversation
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
13d5c33
to
7f3c50b
Compare
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
8af4ef6
to
61a7b35
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4152 +/- ##
=======================================
Coverage 66.19% 66.20%
=======================================
Files 301 301
Lines 21709 21712 +3
Branches 3505 3505
=======================================
+ Hits 14371 14375 +4
+ Misses 5580 5578 -2
- Partials 1758 1759 +1
|
src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java
Outdated
Show resolved
Hide resolved
61a7b35
to
e3384d2
Compare
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
src/main/java/org/opensearch/security/auth/BackendRegistry.java
Outdated
Show resolved
Hide resolved
src/integrationTest/java/org/opensearch/security/http/BasicWithAnonymousAuthTests.java
Outdated
Show resolved
Hide resolved
src/integrationTest/java/org/opensearch/security/http/BasicWithAnonymousAuthTests.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have any concerns with the implementation of this change, since I think it preserves functionality, but trying to understand more about @peternied's concern with the headers
src/main/java/org/opensearch/security/auth/BackendRegistry.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
src/main/java/org/opensearch/security/auth/BackendRegistry.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
src/integrationTest/java/org/opensearch/security/http/BasicWithAnonymousAuthTests.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
b54b619
4a6dff7
to
b54b619
Compare
Update: This issue has been resolved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
d87ab3f
into
opensearch-project:main
…o login as anonymous user when basic authentication fails (opensearch-project#4152) Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Description
This PR fixes a bug where SAML and Anonymous auth cannot co-exist. At present, enabling SAML along with anonymous auth causes SAML Login to fail before it hits the IdP because both SAML and anonymous auth are wired to not have credentials on the first login attempt, causing the authn check against the auth domains to be skipped (see here). This, eventually, results in setting the default user as anonymous (see here) since anonymous auth is enabled.
This PR also resolves another bug involving anonymous auth where a user would be automatically logged in as anonymous user upon failed basic authentication if anonymous auth is enabled.
Security-dashboards companion PR: opensearch-project/security-dashboards-plugin#1839
Issues Resolved
Testing
Check List
- [ ] New functionality has been documentedBy 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.