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

Add optional auth to checkoutComplete operation #400

Conversation

dlobato
Copy link
Contributor

@dlobato dlobato commented Feb 15, 2019

No description provided.

@dlobato dlobato requested a review from a team as a code owner February 15, 2019 14:44
@mamazu
Copy link
Member

mamazu commented Feb 17, 2019

The checkout behaves different in those cases. If you are a logged in user you should not provide an email address in the request.

@dlobato
Copy link
Contributor Author

dlobato commented Feb 18, 2019

Yes, I noticed. This only says, you can use no auth or bearerAuth. Not sure this is 100% correct on openAPIv2 though, I only found the subject mentioned on openAPIv3: https://stackoverflow.com/questions/47659324/how-to-specify-an-endpoints-authorization-is-optional-in-openapi-v3

Also, maybe email should not be required because when you are logged in, the endpoint controller fill email with the logged user email.

@mamazu
Copy link
Member

mamazu commented Feb 18, 2019

Can you add a second definition with the same route but different parameters and different authentication?

@dlobato
Copy link
Contributor Author

dlobato commented Feb 19, 2019

Sure it's possible. So you suggest to have to different operations, one to complete checkout as guest (checkoutCompleteAsGuest) and one to complete checkout as logged in user (checkoutCompleteAsLoggedUser), right?

@mamazu
Copy link
Member

mamazu commented Mar 8, 2019

Well the current implementation has one route for checkout but in the handler it will throw an exception if the user is logged in and the form provides a different email (as discussed in this ticket: #365 (comment)) The current implementation might not make it very clear.

But your pull request is completely right. This route can be accessed with no authentication and an email => guest checkout or with a logged in user and no email => "customer" checkout.

@mamazu mamazu merged commit b9c5f8d into Sylius:master Mar 26, 2019
@mamazu
Copy link
Member

mamazu commented Mar 26, 2019

Thank you, David! 🥇

@dlobato dlobato deleted the swagger_optional_auth_checkout_complete branch April 8, 2019 15:26
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 this pull request may close these issues.

2 participants