ogr icon indicating copy to clipboard operation
ogr copied to clipboard

Authentication lost on 301 redirect

Open rh-hemartin opened this issue 1 year ago • 3 comments

What happened? What is the problem?

The problem is that if we try to get information about a repository that has been moved, the first time we try GitHub returns a 301 which is correctly handled. However when we reach the GitHub API after the 301, the token is not correct and the API call fails.

I think this may be related with this CVE-2018-20060, however that talks about cross-origin, but here the redirection is to the same origin. It is possible that urllib3 decided to drop all auth headers on any redirect to avoid complexity and we are seeing that.

What did you expect to happen?

I expect that the token is reused for the call after the redirect.

Example URL(s)

import logging

from ogr import GithubService

logging.basicConfig(level=logging.DEBUG)

service = GithubService(token="CORRECT_TOKEN")
project_failure = service.get_project_from_url("https://github.com/konveyor/volume-snapshot-mover.git")
project_failure.github_repo

Steps to reproduce

1. Get a valid token, replace it in the code snippet in the previous section
2. Run it. It should fail.
3. You will see how urllib3 reacts to the 301 properly, but the call after the 301 will fail. If you look at the `x-ratelimit-limit` value the first one should be 5k (or more) and the second call has 60 as value. This indicates that the token for the second call was not correct.

Workaround

  • [X] There is an existing workaround that can be used until this issue is fixed.

Participation

  • [ ] I am willing to submit a pull request for this issue. (Packit team is happy to help!)

rh-hemartin avatar Apr 28 '23 08:04 rh-hemartin

@mfocko could you please have a look?

I wonder if we could be impacted by this in Packit.

TomasTomecek avatar May 02 '23 07:05 TomasTomecek

@TomasTomecek sure, I'll have a look and decide if it's on us or PyGithub to fix this

mfocko avatar May 02 '23 08:05 mfocko

Packit service should not be affected by this, still we should fix on our side but it is low prio.

majamassarini avatar Sep 12 '24 07:09 majamassarini