You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/07/27 16:42:57 UTC
[GitHub] [druid] lokesh-lingarajan opened a new pull request #11502: Adding custom callback url for pac4j config to support ssl termination at ELB
lokesh-lingarajan opened a new pull request #11502:
URL: https://github.com/apache/druid/pull/11502
Fixes #11437
### Description
In order to support setups where SSL termination happens at the ELB and the communication from ELB to druid is on plain HTTP protocol, we would need some way to represent proper call back url which is based on HTTPS protocol.
Currently NoParameterCallbackUrlResolver picks up the current webserver configuration, which in this case happens to be http, but the actual communication goes out from an https connection and okta ends up barfing out with the following error.,
Error code - 400 bad request (Invalid request)
Error message - The 'redirect_uri' parameter must be a Login redirect URI in the client app settings: https://something-admin.okta.com/admin/app/oidc_client/instance/xxxxxxxxxxxxxxx#tab-general
If there was configuration that allows users to manually specify a custom callback url, then we can easily override the current jetty webserver config on druid and proxy the ELB's url.
This is similar to what knox provides in some sense using the following parameter
"knoxJwtRealm.redirectParam = originalUrl"
Tested the changes locally using dev.okta account.
This PR has:
- [x] been self-reviewed.
- [x] been tested in a test Druid cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] lokesh-lingarajan commented on pull request #11502: Adding custom callback url for pac4j config to support ssl termination at ELB
Posted by GitBox <gi...@apache.org>.
lokesh-lingarajan commented on pull request #11502:
URL: https://github.com/apache/druid/pull/11502#issuecomment-888815181
Currently we are using HTTPS load balancer. Is there a special way to capture http headers using jetty servers ?? Enabling debug messages doesnt seem to cut it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug commented on pull request #11502: Adding custom callback url for pac4j config to support ssl termination at ELB
Posted by GitBox <gi...@apache.org>.
himanshug commented on pull request #11502:
URL: https://github.com/apache/druid/pull/11502#issuecomment-888808462
@lokesh-lingarajan Were you actually able to dump request headers inside Druid process and verify that the 3 headers were set correctly, my guess is that `X-Forwarded-Proto` is not set to `https`.
> One question - you had mentioned this setup worked for you in the past, was your druid setup on https as well ?
No, Druid process was not TLS aware.
From https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/using-elb-listenerconfig-quickref.html , it seems that X-Forwarded-* headers wouldn't be set if you were using the `TCP/SSL` and not the `HTTP/HTTPS` load balancer. If you have to use the TCP/SSL load balancer and consequently unable to set `X-Forwarded-*` headers then the code in this PR would be necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] xvrl commented on pull request #11502: Adding custom callback url for pac4j config to support ssl termination at ELB
Posted by GitBox <gi...@apache.org>.
xvrl commented on pull request #11502:
URL: https://github.com/apache/druid/pull/11502#issuecomment-887810631
@himanshug it would be great to give your feedback here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug edited a comment on pull request #11502: Adding custom callback url for pac4j config to support ssl termination at ELB
Posted by GitBox <gi...@apache.org>.
himanshug edited a comment on pull request #11502:
URL: https://github.com/apache/druid/pull/11502#issuecomment-888808462
@lokesh-lingarajan Were you actually able to dump request headers inside Druid process and verify that the 3 headers were set correctly, my guess is that `X-Forwarded-Proto` is not set to `https`.
> One question - you had mentioned this setup worked for you in the past, was your druid setup on https as well ?
No, Druid process was not TLS aware.
From https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/using-elb-listenerconfig-quickref.html , it seems that X-Forwarded-* headers wouldn't be set if you were using the `TCP/SSL` and not the `HTTP/HTTPS` load balancer. If you have to use the TCP/SSL load balancer and consequently unable to set `X-Forwarded-*` headers then the code in this PR would be necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] lokesh-lingarajan commented on pull request #11502: Adding custom callback url for pac4j config to support ssl termination at ELB
Posted by GitBox <gi...@apache.org>.
lokesh-lingarajan commented on pull request #11502:
URL: https://github.com/apache/druid/pull/11502#issuecomment-887943151
> hmmm, I had exactly same setup and things used to work fine.
>
> My guess is that ELB, in your case, has one or more of following issues...
>
> * Not preserving the original "Host" header of the original/upstream request
> * Not adding the correct "X-Forwarded-Proto" and "X-Forwarded-Port" headers to the downstream request (i.e. protocol/https and port on ELB which receives original request)
>
> can you check the headers on request when you receive them in the Druid process and see if above are present and correct?
>
> If above are set, then my memory is probably not serving me correctly :)
Thanks Himanshu, that might be the reason too. Will get back after playing a bit more at the ELB side in our setup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug commented on pull request #11502: Adding custom callback url for pac4j config to support ssl termination at ELB
Posted by GitBox <gi...@apache.org>.
himanshug commented on pull request #11502:
URL: https://github.com/apache/druid/pull/11502#issuecomment-887928177
hmmm, I had exactly same setup and things used to work fine.
My guess is that ELB, in your case, has one or more of following issues...
- Not preserving the original "Host" header of the original/upstream request
- Not adding the "X-Forwarded-Proto" and "X-Forwarded-Port" headers to the downstream request
can you check the headers on request when you receive them in the Druid process and see if above are present and correct?
If above are set, then my memory is probably not serving me correctly :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] lokesh-lingarajan commented on pull request #11502: Adding custom callback url for pac4j config to support ssl termination at ELB
Posted by GitBox <gi...@apache.org>.
lokesh-lingarajan commented on pull request #11502:
URL: https://github.com/apache/druid/pull/11502#issuecomment-888828956
Looks like there is druid configuration to enable x-forwarded headers to be enabled
druid.server.http.enableForwardedRequestCustomizer
If enabled, adds Jetty ForwardedRequestCustomizer which reads X-Forwarded-* request headers to manipulate servlet request object when Druid is used behind a proxy.
Default:
false
Let me give this a try.
<br class="Apple-interchange-newline">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] lokesh-lingarajan commented on pull request #11502: Adding custom callback url for pac4j config to support ssl termination at ELB
Posted by GitBox <gi...@apache.org>.
lokesh-lingarajan commented on pull request #11502:
URL: https://github.com/apache/druid/pull/11502#issuecomment-888661264
@himanshug - We are using classic load balancer with vanilla configuration (nothing in the config is disabling x-forwarding, not sure if we can disable it too). https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/using-elb-listenerconfig-quickref.html
Tested the setup again and see the first redirect URL generated by Druid looks something like
Location: https://dev-xxxxxxx.okta.com/oauth2/default/v1/authorize?scope=openid+profile+email&response_type=code&redirect_uri=http%3A%2F%2Fdruid-lab.xxxxxx.cloud%3A8888%2Fdruid-ext%2Fdruid-pac4j%2Fcallback&state=MFvzwAZ_tCI5NLOmnV6w8Hgkj-rrAMJI2EWsLwPpzuc&nonce=xKhMtopqO6zR9dJus0tWRGNYXClvkPsaJuFUJb8Zx_M&client_id=0oa14f62kujIBNrtj5d7
If we look at the redirect url inside the response it still is http instead of https.
Below is the code for computing this url
public String compute(String url, WebContext context) {
if (this.completeRelativeUrl) {
boolean relativeUrl = url != null && !url.startsWith("http") && !url.startsWith("https");
if (context != null && relativeUrl) {
StringBuilder sb = new StringBuilder();
sb.append(context.getScheme()).append("://").append(context.getServerName());
boolean notDefaultHttpPort = ContextHelper.isHttp(context) && context.getServerPort() != 80;
boolean notDefaultHttpsPort = ContextHelper.isHttps(context) && context.getServerPort() != 443;
if (notDefaultHttpPort || notDefaultHttpsPort) {
sb.append(":").append(context.getServerPort());
}
sb.append(url.startsWith("/") ? url : "/" + url);
return sb.toString();
}
}
return url;
}
Line that builds url - sb.append(context.getScheme()).append("://").append(context.getServerName());
gets the correct DNS name from web context but it picks up the Scheme as http, this is because the communication between ELB and Druid is on http and jetty server is configured that way.
So as soon as Druid sends this redirect, okta errors out with the following message
The 'redirect_uri' parameter must be a Login redirect URI in the client app settings: https://dev-XXXXXXX-admin.okta.com/admin/app/oidc_client/instance/0oa14f62kujIBNrtj5d7#tab-general
I tried another experiment where I made the redirect URL in okta app "http" instead of "https", this worked fine from okta perspective, but it failed to resolve the dns as expected. So this is clearly okta rejecting the request as its see the inner payload stating redirect url as http but getting the actual data over https protocol.
One question - you had mentioned this setup worked for you in the past, was your druid setup on https as well ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] xvrl commented on a change in pull request #11502: Adding custom callback url for pac4j config to support ssl termination at ELB
Posted by GitBox <gi...@apache.org>.
xvrl commented on a change in pull request #11502:
URL: https://github.com/apache/druid/pull/11502#discussion_r686356638
##########
File path: docs/development/extensions-core/druid-pac4j.md
##########
@@ -44,3 +44,4 @@ druid.auth.authenticator.pac4j.type=pac4j
|`druid.auth.pac4j.oidc.clientID`|OAuth Client Application id.|none|Yes|
|`druid.auth.pac4j.oidc.clientSecret`|OAuth Client Application secret. It can be provided as plaintext string or The [Password Provider](../../operations/password-provider.md).|none|Yes|
|`druid.auth.pac4j.oidc.discoveryURI`|discovery URI for fetching OP metadata [see this](http://openid.net/specs/openid-connect-discovery-1_0.html).|none|Yes|
+|`druid.server.http.enableForwardedRequestCustomizer`|If enabled, adds Jetty ForwardedRequestCustomizer which reads X-Forwarded-* request headers to manipulate servlet request object when Druid is used behind a proxy.|false|No|
Review comment:
maybe we should add a paragraph to explain this, and link to the main place where we document this option rather than copying it in two different places.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] lokesh-lingarajan commented on pull request #11502: Adding custom callback url for pac4j config to support ssl termination at ELB
Posted by GitBox <gi...@apache.org>.
lokesh-lingarajan commented on pull request #11502:
URL: https://github.com/apache/druid/pull/11502#issuecomment-888833231
Looks like setting "druid.server.http.enableForwardedRequestCustomizer" to true seems to do the trick! Thanks @himanshug , the fix was more on druid itself :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug edited a comment on pull request #11502: Adding custom callback url for pac4j config to support ssl termination at ELB
Posted by GitBox <gi...@apache.org>.
himanshug edited a comment on pull request #11502:
URL: https://github.com/apache/druid/pull/11502#issuecomment-887928177
hmmm, I had exactly same setup and things used to work fine.
My guess is that ELB, in your case, has one or more of following issues...
- Not preserving the original "Host" header of the original/upstream request
- Not adding the correct "X-Forwarded-Proto" and "X-Forwarded-Port" headers to the downstream request (i.e. protocol/https and port on ELB which receives original request)
can you check the headers on request when you receive them in the Druid process and see if above are present and correct?
If above are set, then my memory is probably not serving me correctly :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] lokesh-lingarajan edited a comment on pull request #11502: Adding custom callback url for pac4j config to support ssl termination at ELB
Posted by GitBox <gi...@apache.org>.
lokesh-lingarajan edited a comment on pull request #11502:
URL: https://github.com/apache/druid/pull/11502#issuecomment-888661264
@himanshug - We are using classic load balancer with vanilla configuration (nothing in the config is disabling x-forwarding, not sure if we can disable it too). https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/using-elb-listenerconfig-quickref.html
Tested the setup again and see the first redirect URL generated by Druid looks something like
Location: https://dev-xxxxxxx.okta.com/oauth2/default/v1/authorize?scope=openid+profile+email&response_type=code&redirect_uri=http%3A%2F%2Fdruid-lab.xxxxxx.cloud%3A8888%2Fdruid-ext%2Fdruid-pac4j%2Fcallback&state=MFvzwAZ_tCI5NLOmnV6w8Hgkj-rrAMJI2EWsLwPpzuc&nonce=xKhMtopqO6zR9dJus0tWRGNYXClvkPsaJuFUJb8Zx_M&client_id=0oa14f62kujIBNrtj5d7
If we look at the redirect url inside the response it still is http instead of https.
Below is the code for computing this url
public String compute(String url, WebContext context) {
if (this.completeRelativeUrl) {
boolean relativeUrl = url != null && !url.startsWith("http") && !url.startsWith("https");
if (context != null && relativeUrl) {
StringBuilder sb = new StringBuilder();
sb.append(context.getScheme()).append("://").append(context.getServerName());
boolean notDefaultHttpPort = ContextHelper.isHttp(context) && context.getServerPort() != 80;
boolean notDefaultHttpsPort = ContextHelper.isHttps(context) && context.getServerPort() != 443;
if (notDefaultHttpPort || notDefaultHttpsPort) {
sb.append(":").append(context.getServerPort());
}
sb.append(url.startsWith("/") ? url : "/" + url);
return sb.toString();
}
}
return url;
}
Line that builds url - sb.append(context.getScheme()).append("://").append(context.getServerName());
gets the correct DNS name from web context but it picks up the Scheme as http, this is because the communication between ELB and Druid is on http and jetty server is configured that way.
So as soon as Druid sends this redirect, okta errors out with the following message
The 'redirect_uri' parameter must be a Login redirect URI in the client app settings: https://dev-XXXXXXX-admin.okta.com/admin/app/oidc_client/instance/0oa14f62kujIBNrtj5d7#tab-general
I tried another experiment where I made the redirect URL in okta app "http" instead of "https", this worked fine from okta perspective, but it failed to resolve the dns as expected. So this is clearly okta rejecting the request as its see the inner payload stating redirect url as http but getting the actual data over https protocol.
One question - you had mentioned this setup worked for you in the past, was your druid setup on https as well ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] lokesh-lingarajan commented on pull request #11502: Adding custom callback url for pac4j config to support ssl termination at ELB
Posted by GitBox <gi...@apache.org>.
lokesh-lingarajan commented on pull request #11502:
URL: https://github.com/apache/druid/pull/11502#issuecomment-896218027
@himanshug - added enableForwardedRequestCustomizer inside pac4j extension config. Please approve
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug commented on pull request #11502: Adding custom callback url for pac4j config to support ssl termination at ELB
Posted by GitBox <gi...@apache.org>.
himanshug commented on pull request #11502:
URL: https://github.com/apache/druid/pull/11502#issuecomment-889640171
@lokesh-lingarajan
glad that it worked out. It would be nice to document that inside pac4j extension config, so that people know if they are using a proxy then they should enable that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] lokesh-lingarajan edited a comment on pull request #11502: Adding custom callback url for pac4j config to support ssl termination at ELB
Posted by GitBox <gi...@apache.org>.
lokesh-lingarajan edited a comment on pull request #11502:
URL: https://github.com/apache/druid/pull/11502#issuecomment-888815181
Currently we are using HTTPS load balancer. Is there a special way to capture http headers using jetty servers ?? Enabling debug messages doesnt seem to cut it. @himanshug
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org