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

Allow ant:get task to disable authentication on redirect. #173

Merged
merged 1 commit into from Dec 22, 2021
Merged

Allow ant:get task to disable authentication on redirect. #173

merged 1 commit into from Dec 22, 2021

Conversation

bernolanger
Copy link
Contributor

Most clients do not send the Authorization header on redirects by default; because of security issues.

The ant:get task instead, always sends the Authorization header to the redirected location.

This PR makes this behavior configurable. The optional attribute "authenticateOnRedirect" can be set to "false".

I'm not a security expert. Therefore I didn't change the default behavior to avoid breaking existing Ant scripts. This means, "authenticateOnRedirect" defaults to "true". But maybe it would be better to change this.

Example: getting an artifact from AWS CodeArtifact which redirects to a pre signed URL and therefore mustn't contain the Authorization header:

<get src="https://codeartifact-url/..." username="aws" password="<secret>" dest="..." authenticateOnRedirect="false">
  <header name="Accept" value="*/*"/>
</get>

When the server answers with a redirect response, the request to the new
location may or may not need a second authentication. To disable the
authentication, the new attribute "authenticateOnRedirect" can be set to
"false".
@bodewig bodewig merged commit fa82fe5 into apache:master Dec 22, 2021
@bodewig
Copy link
Member

bodewig commented Dec 22, 2021

many thanks @bernolanger

we'd like to credit you in CONTRIBUTORS and contributors.xml. What is the name you'd want us to use?

asfgit pushed a commit that referenced this pull request Dec 22, 2021
@jaikiran
Copy link
Member

Therefore I didn't change the default behavior to avoid breaking existing Ant scripts. This means, "authenticateOnRedirect" defaults to "true". But maybe it would be better to change this.

I was leaning towards making this new authenticateOnRedirect to default to false to be more secure (i.e. don't set Authorization header to redirected URL unless explicitly asked to). That might break scripts but I think that's probably a good thing since it would force users to review their target URLs and decide if they really want to send the auth header on redirect for that specific URL.

The only place where this would probably be a nuisance is if the redirect is happening just for the scheme. What I mean is if the original URL http://example.com/foo was redirecting to https://example.com/foo. Or even in some cases where servers redirect a URL of the form http://example.com/foo to http://example.com/foo/ (slash at the end). So yes, I guess leaving the current backward compatible behaviour is OK.

@bodewig
Copy link
Member

bodewig commented Dec 23, 2021

you are right @jaikiran, I'll change the default.

asfgit pushed a commit that referenced this pull request Dec 23, 2021
@sfllaw
Copy link

sfllaw commented Jun 2, 2022

82f5edc changed the default value of authenticateOnRedirect from true to false, which is backwards incompatible but is actually proper behaviour.

@sfllaw
Copy link

sfllaw commented Jun 2, 2022

I encountered this bug when using <get> with HTTP Basic Auth to supply a username and password. If the server responds with a 3xx redirect to an S3 bucket, Amazon will respond with a 400 Bad Request:

Only one auth mechanism allowed; only the X-Amz-Algorithm query parameter, Signature query string parameter or the Authorization header should be specified

I’m leaving this comment so that others can find this issue. Ant 1.10.13 will probably solve this, once it is released.

See also:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants