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: add authentification through JSON body for API v3 WRITE requests #7813

Merged
merged 17 commits into from
Dec 12, 2022

Conversation

stephanegigandet
Copy link
Contributor

This PR adds authentification for API v3 WRITE requests, by providing user_id and password fields in the JSON body of the request (it was documented in OpenAPI, but not implemented).

Also added tests for authentification for API READ + WRITE for v2 + v3

@stephanegigandet stephanegigandet requested a review from a team as a code owner December 8, 2022 09:54
@github-actions github-actions bot added API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…) API READ All READ APIs include Product, Search… API v3 API WRITE WRITE API to allow sending product info and image Display 📚 Documentation Documentation issues improve the project for everyone. 🧪 tests Translations We use a non-standard version of GetText, lack language variants support translate.openfoodfacts.org 👥 Users labels Dec 8, 2022
@hangy
Copy link
Member

hangy commented Dec 8, 2022

I kinda missed the discussion: Why do we want the password in the JSON body instead of HTTP Basic Auth or a token (ORY Hydra+Kratos)? HTTP bodies might be logged somewhere, and using HTTP Headers for auth is a more standard behaviour.

OpenAPI has built-in support for basic auth, which is often also supported by design tools (ie. Swagger): https://swagger.io/docs/specification/authentication/basic-authentication/

@github-actions github-actions bot added the 💥 Merge Conflicts 💥 Merge Conflicts label Dec 8, 2022
@stephanegigandet
Copy link
Contributor Author

I kinda missed the discussion: Why do we want the password in the JSON body instead of HTTP Basic Auth or a token (ORY Hydra+Kratos)? HTTP bodies might be logged somewhere, and using HTTP Headers for auth is a more standard behaviour.

OpenAPI has built-in support for basic auth, which is often also supported by design tools (ie. Swagger): https://swagger.io/docs/specification/authentication/basic-authentication/

Hi @hangy. Thanks for the feedback. We will use HTTP basic auth soon, this is a temporary solution because we need something working as soon as possible as we need the mobile app to support packagings before an event that happens on January 12th.

There is a discussion about it here: #7564 (comment)

@sonarcloud
Copy link

sonarcloud bot commented Dec 12, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@github-actions github-actions bot removed the 💥 Merge Conflicts 💥 Merge Conflicts label Dec 12, 2022
Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good !

=head3 Return value

Reference to request object.

=cut

sub init_request() {
sub init_request ($request_ref = {}) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In python if you use a dict as a default parameter, it will be shared between every call.
(so you prefer to initialize with "undefined" and then if request_ref is undefined, you create a new dict).

But this seems to work in perl (I've done a test !)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API READ All READ APIs include Product, Search… API v3 API WRITE WRITE API to allow sending product info and image API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…) Display 📚 Documentation Documentation issues improve the project for everyone. 🧪 tests Translations We use a non-standard version of GetText, lack language variants support translate.openfoodfacts.org 👥 Users
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants