Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ISSUE-603: Minor: Registry is failing to parse the doAs user #604

Merged
merged 2 commits into from
Oct 10, 2019

Conversation

guruchai
Copy link
Contributor

@guruchai guruchai commented Oct 7, 2019

Fixes #603

@guruchai guruchai requested review from kamalcph and raju-saravanan and removed request for kamalcph October 7, 2019 13:22
@guruchai
Copy link
Contributor Author

guruchai commented Oct 7, 2019

Please merge to CSP-3.0 as well.

@@ -461,8 +461,8 @@ protected static String getDoasUser(HttpServletRequest request) {
if (doAsUser.isEmpty()) {
return null;
}
break;
Copy link
Collaborator

@raju-saravanan raju-saravanan Oct 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@guruchai : Will there be multiple "doAs" keys in the header ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raju-saravanan It's query parameter, not header.

Copy link
Collaborator

@raju-saravanan raju-saravanan Oct 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@guruchai So multiple "doAs" query parameter ? Can you tell when there would be multiple "doAs" query parameters ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, with regards to the question, it does not make sense to have multiple doAs query parameters. Since the doAs is typically attached by a proxy server, it makes sense to pick the last one.
So break statement does not make sense. Thanks for pointing!

@guruchai
Copy link
Contributor Author

@raju-saravanan Can you check?

Copy link
Collaborator

@raju-saravanan raju-saravanan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@guruchai Thanks for this PR. +1 LGTM.

@raju-saravanan raju-saravanan merged commit 754af5d into master Oct 10, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minor: Registry is failing to parse the doAs user
2 participants