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 download authentication feeds #783

Open
davidgamez opened this issue Oct 28, 2024 · 4 comments · May be fixed by #867
Open

Fix download authentication feeds #783

davidgamez opened this issue Oct 28, 2024 · 4 comments · May be fixed by #867
Assignees

Comments

@davidgamez
Copy link
Member

Description

The implementation of the authenticated feeds seems to be broken as the authentication types 1 and 2 are flipped. This might be the case for why some authenticated feeds are not downloaded, example. In an interesting case, the feed mdb-1846 is present in the UI as it supports the api_key parameter as a header and also a URL parameter.

Proposed solution

  1. As documented here make sure the implementation aligns with the proper authentication type.
    Authentication types:
    0 or (empty) - No authentication required.
    1 - The authentication requires an API key, which should be passed as value of the parameter api_key_parameter_name in the URL. Please visit URL in authentication_info_url for more information.
    2 - The authentication requires an HTTP header, which should be passed as the value of the header api_key_parameter_name in the HTTP request.

  2. Set a default user-agent similar to link. This will revert feat: Programmatic download of valid ZIP files returns HTML responses for certain producer URLs #524 changes.

  3. To address the fix introduced in feat: Programmatic download of valid ZIP files returns HTML responses for certain producer URLs #524, add the feeds to header authentication and set the proper values to the API_SOURCE_SECRETS list.

Number 2 and 3 TBD pending team review

TBD: Allow multiple headers as part of the authentication 2.

@jcpitre
Copy link
Contributor

jcpitre commented Nov 11, 2024

Tasks:

  • Flip implementation of authentication type 1 and 2
  • Wait until the dataset is fetched and verify

@qcdyx qcdyx self-assigned this Dec 3, 2024
@qcdyx
Copy link
Contributor

qcdyx commented Dec 24, 2024

I tested mdb-1974 and mdb-1313. The batch function succeeded, but it returned an invalid zip file error. I also tested the producer_url with the correct API key in a browser. Although it was flagged as an insecure download, I proceeded with the warning and successfully downloaded a valid GTFS zip file.
Image

@davidgamez
Copy link
Member Author

@qcdyx I haven't tested the feeds, but the authentication logic is still incorrect. As seen here when type set to 1 the code add a header instead of a URL parameter and the same reverse logic for the 2 value.

@qcdyx qcdyx linked a pull request Jan 7, 2025 that will close this issue
5 tasks
@qcdyx qcdyx linked a pull request Jan 7, 2025 that will close this issue
5 tasks
@qcdyx
Copy link
Contributor

qcdyx commented Jan 7, 2025

@davidgamez I just created the PR. It's been so long, I thought I did, my bad.

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 a pull request may close this issue.

3 participants