-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
Does not patch rest api #1 #4
Conversation
Jitheesh
commented
Nov 10, 2020
- Disable new admin token generate end point functionality for 2fa
- allow merchants to use existing token generate service without enabling 2fa
Thank you @Jitheesh. Can you please list out steps that I can take to test this? |
Hi @markshust In 2.4.1, Magento changed api end point for admin token generate. `Two-Factor Authentication is implemented for Magento Web APIs with the following changes: AdminTokenServiceInterface::createAdminAccessToken() throws an exception when the Admin user doesn’t have personal 2FA configured, and also indicates that the confirmationh email has been sent. So to disable 2fa from token generate api, I've re-enabled our previous token generate end point. ie, if you disable 2fa for token generate, you can continue to use existing end point. Otherwise you should configure 2fa and integrate new end point. Test steps
|
I can confirm that this fix works, can we merge it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for this PR! I'll try to make these updates very shortly, nothing technically changed besides some code styling issues.
I'm taking the updates, will be pushed momentarily |
Thanks @markshust for your code styling suggestions, It is really helpful. |
Sorry to be "that guy who's late to the party with a problem" but these changes fail to parse with PHP7.3 Typed props only appeared in 7.4 - and I wouldn't mention it at all, except composer specifies |
Thanks @cmacdonald-au and @Rud5G -- definitely a mistake on my part. Fixed that in #6 |