Skip to content

Fix OIDC looping issue - too many redriects#1014

Merged
cliu123 merged 2 commits intoopensearch-project:mainfrom
aoguan1990:main
Jun 24, 2022
Merged

Fix OIDC looping issue - too many redriects#1014
cliu123 merged 2 commits intoopensearch-project:mainfrom
aoguan1990:main

Conversation

@aoguan1990
Copy link
Copy Markdown
Contributor

@aoguan1990 aoguan1990 commented Jun 21, 2022 •
edited
Loading

Signed-off-by: Aozixuan Priscilla Guan aoguan@amazon.com

Description

Customized error handling mechanism based on the error message for OIDC routing

Category

Bug fix

Why these changes are required?

Resolve redirect login looping issues when authentication failures detected.

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

Old Behavior:

Any exceptions caught during the OIDC authentication process causes redirecting login infinitely.

New Behavior:
If error message includes "authentication error": => return 401: unauthorized
Else: redirect to login

Issues Resolved

#990

Testing

unit testing and integration 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.

@aoguan1990 aoguan1990 requested a review from a team June 21, 2022 00:35
@cliu123
Copy link
Copy Markdown
Member

cliu123 commented Jun 21, 2022

@aoguan1990 Great contribution! Please sign the commit and add tests for the fix.

aoguan1990 and others added 2 commits June 21, 2022 16:52
@aoguan1990
Fix OIDC looping issue - too many redriects
9394ed9 
Signed-off-by: Aozixuan, Priscilla, Guan <aozixuanguan1990@gmail.com>
Signed-off-by: Aozixuan Priscilla Guan <aoguan@amazon.com>
@aoguan1990
Fix unit test case failures
fa571ea 
Signed-off-by: Aozixuan Priscilla Guan <aoguan@amazon.com>
@aoguan1990
Copy link
Copy Markdown
Contributor Author

aoguan1990 commented Jun 21, 2022 •
edited
Loading

@aoguan1990 Great contribution! Please sign the commit and add tests for the fix.

@cliu123 Unit test and commit issues are fixed. Please advice when we can resolve the integration test blocker.

@cliu123
Copy link
Copy Markdown
Member

cliu123 commented Jun 21, 2022

@aoguan1990 Thanks for resoving these issues! Could you please add tests for the fix? Without the fix, the test should fail. With the fix, the test should pass.

@seraphjiang
Copy link
Copy Markdown
Member

seraphjiang commented Jun 22, 2022

I saw integration test failed on download 2.1 security artifacts. do we have 2.1 artifacts now?

https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/2.1.0/3911/linux/x64/tar/builds/opensearch/plugins/opensearch-security-2.1.0.0.zip

Run wget -O opensearch-security.zip https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/2.1.0/latest/linux/x64/tar/builds/opensearch/plugins/opensearch-security-2.1.0.0.zip
--2022-06-21 21:53:44--  https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/2.1.0/latest/linux/x64/tar/builds/opensearch/plugins/opensearch-security-2.1.0.0.zip
Resolving ci.opensearch.org (ci.opensearch.org)... 52.84.125.21, 52.84.125.109, 52.84.125.1[7](https://github.com/opensearch-project/security-dashboards-plugin/runs/6993741182?check_suite_focus=true#step:3:8), ...
Connecting to ci.opensearch.org (ci.opensearch.org)|52.84.125.21|:443... connected.
HTTP request sent, awaiting response... 302 Moved Temporarily
Location: /ci/dbc/distribution-build-opensearch/2.1.0/3911/linux/x64/tar/builds/opensearch/plugins/opensearch-security-2.1.0.0.zip [following]
--2022-06-21 21:53:44--  https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/2.1.0/3911/linux/x64/tar/builds/opensearch/plugins/opensearch-security-2.1.0.0.zip
Reusing existing connection to ci.opensearch.org:443.
HTTP request sent, awaiting response... 403 Forbidden
2022-06-21 21:53:45 ERROR 403: Forbidden.
Error: Process completed with exit code [8](https://github.com/opensearch-project/security-dashboards-plugin/runs/6993741182?check_suite_focus=true#step:3:9).

@cliu123
Copy link
Copy Markdown
Member

cliu123 commented Jun 22, 2022 •
edited
Loading

@seraphjiang This is an known issue. 2.1.0 build failed, so the artifact hasn't been available yet. This PR needs to wait for the artifact.
@zelinh @gaiksaya When is the 2.1.0 artifact expected to be available?

@DarshitChanpura
Copy link
Copy Markdown
Member

DarshitChanpura commented Jun 24, 2022 •
edited
Loading

@opensearch-project/security Can we get a second review for this?

@cliu123 cliu123 merged commit 015dc3f into opensearch-project:main Jun 24, 2022
@cliu123
Copy link
Copy Markdown
Member

cliu123 commented Jun 24, 2022

@aoguan1990 @seraphjiang Thanks for the contribution!

@peternied
Copy link
Copy Markdown
Member

peternied commented Jun 24, 2022

@aoguan1990 I know this was merged, but I do not see test modifications in the pull request, could you make another pull request to include them?

@DarshitChanpura DarshitChanpura added the backport 2.x backport to 2.x branch label Jun 27, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jun 27, 2022
@aoguan1990 @github-actions
Fix OIDC looping issue - too many redriects (#1014)
013e277 
Signed-off-by: Aozixuan Priscilla Guan <aoguan@amazon.com>
(cherry picked from commit 015dc3f)
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Jun 27, 2022
Merged
@aoguan1990
Copy link
Copy Markdown
Contributor Author

aoguan1990 commented Jun 27, 2022

@aoguan1990 I know this was merged, but I do not see test modifications in the pull request, could you make another pull request to include them?

@peternied Due to the technical challenge, our existing test framework does not include test cases for OIDC authentication. As per discussion with @seraphjiang and @zengyan-amazon, we can revisit the OIDC test case issue later. So closed the issue #990 for now.

DarshitChanpura pushed a commit that referenced this pull request Jun 27, 2022
@opensearch-trigger-bot @aoguan1990
Fix OIDC looping issue - too many redriects (#1014) (#1018)
521d8a5 
Signed-off-by: Aozixuan Priscilla Guan <aoguan@amazon.com>
(cherry picked from commit 015dc3f)

Co-authored-by: Aozixuan Priscilla Guan <92183424+aoguan1990@users.noreply.github.com>
spartan2015 pushed a commit to spartan2015/security-dashboards-plugin that referenced this pull request Aug 8, 2022
@aoguan1990
Fix OIDC looping issue - too many redriects (opensearch-project#1014)
08092a4 
Signed-off-by: Aozixuan Priscilla Guan <aoguan@amazon.com>
Signed-off-by: Vasile Negru <vasile@eosfintek.com>
@Desvelao Desvelao mentioned this pull request Oct 27, 2025
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cliu123 cliu123 cliu123 approved these changes

@DarshitChanpura DarshitChanpura DarshitChanpura approved these changes

Assignees

No one assigned

Labels

backport 2.x backport to 2.x branch

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@aoguan1990 @cliu123 @seraphjiang @DarshitChanpura @peternied