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

Directly authenticate when building with password #258

Merged
merged 1 commit into from
Feb 27, 2023

Conversation

damien-carcel
Copy link
Contributor

When building a client with AkeneoPimClientBuilder::buildAuthenticatedByPassword, the returned client has no access nor refresh tokens. Both corresponding getters will simply return null.
For the token to be set, one have to perform at least one API call, so the lower level AuthenticatedHttpClient::sendRequest() method tries to get the access token and authenticate if there is none.

From an integrator POV, this is confusing, as one would expect to get the tokens right away to store them for future usage.

This PR proposes to call the authentication API when building a client with username and password. This way, the returned client already contains an access token and a refresh token.

@@ -1,4 +1,4 @@
DOCKER_RUN = DOCKER_BUILDKIT=1 docker-compose run php_client
DOCKER_RUN ?= DOCKER_BUILDKIT=1 docker-compose run php_client
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This allows to override locally the environment variable with an export. In my case, I don't use docker-compose but docker compose.

Copy link
Contributor

Choose a reason for hiding this comment

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

@samokiss your solution to be able to use Compose V2 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another solution would be to replicate what JM did on the PIM (I think the PR is not merged yet) and what we did for now quite some time on Shared Catalogs: abandon Compose v1. Compose v2 is out for more than 2 years now, and Circle CI latests "machine" images work like a charm with it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@damien-carcel damien-carcel force-pushed the authenticate-build-password branch from 2bb117e to 13f6cc3 Compare February 24, 2023 13:11
@damien-carcel damien-carcel merged commit ab4a2cd into master Feb 27, 2023
@damien-carcel damien-carcel deleted the authenticate-build-password branch February 27, 2023 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants