Skip to content
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

Adding redirectParam to add custom callback url #39

Closed
wants to merge 1 commit into from

Conversation

lokesh-lingarajan
Copy link

Issue:
apache#11437

Context:
https://www.pac4j.org/blog/understanding-the-callback-endpoint.html

While using Zeppelin + Knox, knox provides a parameter to add custom callback url
https://medium.com/data-collective/apache-zeppelin-oauth-integration-using-apache-knox-dea2362e3dda
"knoxJwtRealm.redirectParam = originalUrl"

I have used the same name for this new config and overrided the compute method in NoParameterCallbackUrlResolver class.

Works fine in local setup, next step is to test in lab.

@xvrl
Copy link
Member

xvrl commented Jul 15, 2021

@lokesh-lingarajan can you explain how someone would use this parameters? My understanding based on the examples you gave is that the redirectParam is something that specifies how to extract the redirect URL from the request, and pac4j already has QueryParameterCallbackUrlResolver and PathParameterCallbackUrlResolver that implement the functionality you described above for knox+zeppelin. What you implement here seems to work a little differently, can you clarify?

@lokesh-lingarajan
Copy link
Author

@lokesh-lingarajan can you explain how someone would use this parameters? My understanding based on the examples you gave is that the redirectParam is something that specifies how to extract the redirect URL from the request, and pac4j already has QueryParameterCallbackUrlResolver and PathParameterCallbackUrlResolver that implement the functionality you described above for knox+zeppelin. What you implement here seems to work a little differently, can you clarify?

I dont see this as a way to extract url from request rather specify the whole url in cases of setups like we have in our prod, where SSL is terminating at ELB and internal communication is HTTP. So we would use this as follows in our prod

"druid.auth.pac4j.redirectParam=https://localhost:8888/druid-ext/druid-pac4j/callback"

Where although our webserver is running on http, we are overriding the url to https, it also gives users more flexibility is redirecting the url to any custom service to do additional data gathering before hitting the service.

Copy link
Member

@xvrl xvrl left a comment

Choose a reason for hiding this comment

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

LGTM, but can we add a test to Pac4jCommonConfigTest to cover this?

@xvrl
Copy link
Member

xvrl commented Aug 3, 2021

I guess we can close this PR since we no longer need this change?

@lokesh-lingarajan
Copy link
Author

Not required anymore, we need to use "druid.server.http.enableForwardedRequestCustomizer" to support druid behind a proxy.

saithal-confluent added a commit that referenced this pull request Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants