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

bugfix: fix token expiry check in admin settings #646

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

matthewhilton
Copy link
Contributor

@matthewhilton matthewhilton commented Oct 24, 2024

Issue:
There were a few issues found after more thorough testing of #636:

  1. The class wasn't included (oops!)
  2. There is a core bug which means on newer php versions if it didn't have an action link it would throw an exception -> https://tracker.moodle.org/browse/MDL-83537
  3. The admin_setting_check doesn't work unless it is in the top level settings page and not in the client settings definition area, because of the way internally it searches through the admin tree to find the check.

Fixes:

  1. Moved token expiry check to top level. Realistically all client sdks could/should implement this so no point having it only in the Azure client section.
  2. Added action link as an fix until MDL-83537 is implemented
  3. Fixed a spelling error

Testing/screenshots:
Where token expiry is implemented - in Azure client code:
2024-10-24_10-20

Where token expiry is not implemented:
2024-10-24_10-21

settings.php Show resolved Hide resolved
@rhell4
Copy link
Contributor

rhell4 commented Oct 25, 2024

Looks good to me.

@rhell4 rhell4 merged commit 306a48d into MOODLE_402_STABLE Oct 25, 2024
20 checks passed
@rhell4 rhell4 deleted the fix-check-api-azure-42 branch October 25, 2024 03:28
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