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

local_o365 breaks platform update if idptype is AUTH_OIDC_IDP_IDP_TYPE_OTHER #2486

Closed
fabianbatioja opened this issue Jan 29, 2024 · 2 comments · Fixed by #2506 or #2507
Closed

local_o365 breaks platform update if idptype is AUTH_OIDC_IDP_IDP_TYPE_OTHER #2486

fabianbatioja opened this issue Jan 29, 2024 · 2 comments · Fixed by #2506 or #2507
Assignees
Labels
Issue type - bug Bugs in existing code that needs to be fixed. Plugin - local_o365 Status - PR ready / pending release Dev is done and PR ready. Will be included in the next release.
Milestone

Comments

@fabianbatioja
Copy link

When trying to install a new plugin that has capabilities while we have local_o365 and auth_oidc installed and with idptype in AUTH_OIDC_IDP_TYPE_OTHER, it breaks the installation and breaks the platform update because the get_app_token() method does not support the case where idptype is AUTH_OIDC_IDP_IDP_TYPE_OTHER.

In the following code you can see the select that only supports the AUTH_OIDC_IDP_TYPE_AZURE_AD and AUTH_OIDC_IDP_TYPE_MICROSOFT cases, but not AUTH_OIDC_IDP_IDP_TYPE_OTHER which is a valid case:

switch (get_config('auth_oidc', 'idptype')) {

This is the stack trace:

Default exception handler: Exception - http_build_query(): Argument #1 ($data) must be of type array, null given Debug: 
Error code: generalexceptionmessage
* line 106 of /local/o365/classes/oauth2/apptoken.php: TypeError thrown
* line 106 of /local/o365/classes/oauth2/apptoken.php: call to http_build_query()
* line 52 of /local/o365/classes/oauth2/apptoken.php: call to local_o365\oauth2\apptoken::get_app_token()
* line 178 of /local/o365/classes/oauth2/token.php: call to local_o365\oauth2\apptoken::get_for_new_resource()
* line 95 of /local/o365/classes/utils.php: call to local_o365\oauth2\token::instance()
* line 67 of /local/o365/classes/utils.php: call to local_o365\utils::get_app_or_system_token()
* line 663 of /local/o365/classes/observers.php: call to local_o365\utils::is_connected()
* line ? of unknownfile: call to local_o365\observers::handle_capability_change()
* line 155 of /lib/classes/event/manager.php: call to call_user_func()
* line 75 of /lib/classes/event/manager.php: call to core\event\manager::process_buffers()
* line 835 of /lib/classes/event/base.php: call to core\event\manager::dispatch()
* line 1430 of /lib/accesslib.php: call to core\event\base->trigger()
* line 1203 of /lib/accesslib.php: call to assign_capability()
* line 2343 of /lib/accesslib.php: call to assign_legacy_capabilities()
* line 638 of /lib/upgradelib.php: call to update_capabilities()
* line 773 of /lib/upgradelib.php: call to upgrade_component_updated()
* line 1975 of /lib/upgradelib.php: call to upgrade_plugins()
* line 203 of /admin/cli/upgrade.php: call to upgrade_noncore()

!!! Exception - http_build_query(): Argument #1 ($data) must be of type array, null given !!!
!! 
Error code: generalexceptionmessage !!
!! Stack trace: * line 106 of /local/o365/classes/oauth2/apptoken.php: TypeError thrown
* line 106 of /local/o365/classes/oauth2/apptoken.php: call to http_build_query()
* line 52 of /local/o365/classes/oauth2/apptoken.php: call to local_o365\oauth2\apptoken::get_app_token()
* line 178 of /local/o365/classes/oauth2/token.php: call to local_o365\oauth2\apptoken::get_for_new_resource()
* line 95 of /local/o365/classes/utils.php: call to local_o365\oauth2\token::instance()
* line 67 of /local/o365/classes/utils.php: call to local_o365\utils::get_app_or_system_token()
* line 663 of /local/o365/classes/observers.php: call to local_o365\utils::is_connected()
* line ? of unknownfile: call to local_o365\observers::handle_capability_change()
* line 155 of /lib/classes/event/manager.php: call to call_user_func()
* line 75 of /lib/classes/event/manager.php: call to core\event\manager::process_buffers()
* line 835 of /lib/classes/event/base.php: call to core\event\manager::dispatch()
* line 1430 of /lib/accesslib.php: call to core\event\base->trigger()
* line 1203 of /lib/accesslib.php: call to assign_capability()
* line 2343 of /lib/accesslib.php: call to assign_legacy_capabilities()
* line 638 of /lib/upgradelib.php: call to update_capabilities()
* line 773 of /lib/upgradelib.php: call to upgrade_component_updated()
* line 1975 of /lib/upgradelib.php: call to upgrade_plugins()
* line 203 of /admin/cli/upgrade.php: call to upgrade_noncore()
 !!
@weilai-irl
Copy link
Collaborator

Hi @fabianbatioja

The local_o365 plugin is only supposed to work if the IdP type is set to either Azure AD or Microsoft Identity Platform. It will not work with non-Microsoft IdPs. If you are using auth_oidc with IdP type set to AUTH_OIDC_IDP_TYPE_OTHER, the only thing that will work is SSO integration, and all other plugins in the suites, including local_o365 should be uninstalled.

I agree although it's not how it's designed to work, technically a Moodle site can have both auth_oidc and local_o365 plugins installed, and set AUTH_OIDC_IDP_TYPE_OTHER as IdP type. I can add some logic to local_o365 to let is fail gracefully in this case. This will happen in the next one or two release.

Regards,
Lai

@weilai-irl weilai-irl self-assigned this Jan 29, 2024
@weilai-irl weilai-irl added Issue type - bug Bugs in existing code that needs to be fixed. Plugin - local_o365 Status - queued / not yet started The request is clear, but the work has yet to be scheduled. labels Jan 29, 2024
@weilai-irl weilai-irl added Status - PR ready / pending release Dev is done and PR ready. Will be included in the next release. and removed Status - queued / not yet started The request is clear, but the work has yet to be scheduled. labels Feb 22, 2024
@weilai-irl weilai-irl added this to the 2024-01 milestone Feb 22, 2024
@weilai-irl
Copy link
Collaborator

Hi @fabianbatioja

The fix to the issue has been included in the release from today.

Thank you very much for reporting the issue.

Regards,
Lai

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment