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

Include provided opaque information in the redirection #970

Merged
merged 3 commits into from
May 2, 2019

Conversation

bbockelm
Copy link
Contributor

If opaque information is given by the user as part of the URL (i.e., GET /foo?bar=1), then include this opaque information in the redirection string.

Previously, opaque information was dropped and only the resource was used. This is problematic in cases where the opaque information contains important authorization details (think: signed URLs).

Additionally, the TPC handler is updated to perform correctly in this situation.

With this PR, if http.header2cgi Authorization authz is set in the configuration, the token will be moved from the headers to the opaque information and now included in the redirect. Thus, clients which drop the Authorization: header upon redirect (this includes the latest versions of curl!) will now work by default with an Xrootd cluster.

Fixes behavior observed in the WLCG DOMA testing.

This also includes the opaque information provided in the URL; for
example, if the `Authorization` header is added to the XRootD CGI
as `authz`, then this will now be included in the redirection.
With this, if the `authz` opaque information is present in the URL,
this will be part of the redirection request.
@ffurano ffurano merged commit 5e158d3 into xrootd:master May 2, 2019
@ffurano
Copy link
Contributor

ffurano commented May 2, 2019

Here I merged it, as the usual test runs went fine. I just have some concerns over how many people may remember to add "http.header2cgi Authorization authz" to their configuration. At the same time making this the default may be excessive.

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.

None yet

2 participants