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

Fix problematic retransmission of authentication token #436

Closed
wants to merge 1 commit into from

Conversation

JojOatXGME
Copy link
Contributor

@JojOatXGME JojOatXGME commented May 4, 2024

Should fix #343 and also fix #363. While taking a short look at the implementation, I was wondering if we are actually supposed to retransmit the authentication header. I took a short look at the API documentation, which reinforced my suspicion by not mentioning any authentication requirements. I therefore just tried to remove the headers from the request, and it worked according to my tests.

Note that the HTTP specification also suggests that you may not want to re-transmit the authentication header. Some google research suggested that you should only re-transmit the header if the target of the redirect is on the same domain.
https://www.rfc-editor.org/rfc/rfc9110.html#section-15.4-6.2.1

The retransmission of the authentication token to the server providing
the artifact caused the following errors when using Artifacts v4:

  HTTPError: Response code 400 (Authentication information is not given
  in the correct format. Check the value of Authorization header.)

It looks like the service serving the artifacts does not expect the
authentication header, and therefore complaines about inproper use of
the authentication header.
@JojOatXGME
Copy link
Contributor Author

I also created an alternative solution at #438. While the change is a bit larger, the resulting code looks better to me.

@jozefizso
Copy link
Collaborator

Hi @JojOatXGME, thanks for looking into this.

Yes, you are correct, the authentication header must be added only if the new download location has the same domain.

The #438 looks good to me.

@jozefizso jozefizso self-assigned this May 4, 2024
@jozefizso jozefizso added the bug Something isn't working label May 4, 2024
@jozefizso
Copy link
Collaborator

I will close this one in favor of #438

@jozefizso jozefizso closed this May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

actions/upload-artifact & actions/download-artifact v4 support Test Reporter fails on workflow_run
2 participants