https://github.com/magento/magento2/issues/24716

[ivanko-dev]

Not Able to set Rest API oAuth admin token expire in minutes or seconds. #24716

Description (*)

Checking of expiration time for oAuth tokens worked only with integers. After update it is able to use float values without rounding.

Fixed Issues (if relevant)

1. Not Able to set Rest API oAuth admin token expire in minutes or seconds. #24716: Not Able to set Rest API oAuth admin token expire in minutes or seconds.

Manual testing scenarios (*)

1. Update Customer Token Lifetime (hours) or Admin Token Lifetime (hours) in Stores-->Configuration-->Services-->OAuth to values less then 0. For examle 0.05 is 3 minutes
2. Get Access token using API POST request to /rest/V1/integration/admin/token with username and password parameters in request
3. Try to access protected API endpoints using received token
4. Token should expire after time defined on step (3 minutes in described case)

Questions or comments

app/code/Magento/Webapi/Model/Authorization/TokenUserContext.php: Line 140 - deprecated \Magento\Framework\Stdlib\DateTime:strToTime replaced to \Magento\Framework\Stdlib\DateTime\DateTime:timestamp method

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @ivan-koliadynskyy. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

[sidolov]

Hi @ivan-koliadynskyy , please, sign CLA, otherwise, we can't process your pull request

[magento-cicd2]

All committers have signed the CLA.

[ivanko-dev]

Hi @ivan-koliadynskyy , please, sign CLA, otherwise, we can't process your pull request

Hello @sidolov , thank you for pointing to required changes. Author of commit updated and CLA was signed. Please let me know if more changes required.

[sidolov]

@ivan-koliadynskyy looks like you cherry-picked the first commit and it has two authors right now (ivan_ko and ivan-koliadynskyy). I propose you to make one commit with the correct account and force push the branch.

[ivanko-dev]

@ivan-koliadynskyy looks like you cherry-picked the first commit and it has two authors right now (ivan_ko and ivan-koliadynskyy). I propose you to make one commit with the correct account and force push the branch.

@sidolov , sorry for delay with these changes. Commit updated again and looks like CLA test passed well. Please let me know if more changes required. Thanks.

[sidolov]

@ivan-koliadynskyy , please, take a look at failed tests.

[ivanko-dev]

@sidolov , thanks for hints. Code updated to pass failed tests but I have doubt about unit test for update. Initially I have updated app/code/Magento/Webapi/Model/Authorization/TokenUserContext.php: 140 to use \Magento\Framework\Stdlib\DateTime\DateTime::timestamp() instead of \Magento\Framework\Stdlib\DateTime::strToTime() which is simple wrapper for php strtotime(), because strToTime() noted like deprecated method. Everything looks good in this case but unit test not pass because of lib/internal/Magento/Framework/Stdlib/DateTime/DateTime.php:147. On unit test I have received $this->_localeDate equal null and error that can not call date() on null.
I have used strtotime() directly in TokenUserContext.php: 140 with last update but I am not sure that this is proper approach in this case.

[sidolov]

@ivan-koliadynskyy you are right, you should return back usage of \Magento\Framework\Stdlib\DateTime::strToTime instead of strtotime function.

Regarding the failed test: please, make sure that getCustomerTokenLifetime and getAdminTokenLifetime will always return a positive integer. For example is_numeric(-1) will return true.

[ivanko-dev]

@ivan-koliadynskyy you are right, you should return back usage of \Magento\Framework\Stdlib\DateTime::strToTime instead of strtotime function.

Regarding the failed test: please, make sure that getCustomerTokenLifetime and getAdminTokenLifetime will always return a positive integer. For example is_numeric(-1) will return true.

@sidolov, thank you for hints. Code for time comparison reverted to use \Magento\Framework\Stdlib\DateTime::strToTime and getCustomerTokenLifetime, getAdminTokenLifetime have checking if result value is numeric and positive before response. Unit test on local instance passed well.

[m2-assistant]

Hi @ivan-koliadynskyy, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

[magento-engcom-team]

Hi @sidolov, thank you for the review.
ENGCOM-5987 has been created to process this Pull Request
✳️ @sidolov, could you please add one of the following labels to the Pull Request?

Label	Description
Auto-Tests: Covered	All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered	Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required	Changes in Pull Request does not require coverage by auto-tests

[engcom-Alfa]

✔️ QA Passed

[m2-assistant]

Hi @ivan-koliadynskyy, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.