Roles unassigned due to array to string conversion

@dieuwe

I have the same issue, but it's openid_connect module issue too (the mentioned PHP warnings).
I'm using D10 + openid_connect v 3.0.0-alpha2 + openid_connect_windows_aad v 2.0.0-beta5.

I think we should leave the group_mapping settings for the Windows AAD client - as you said, this is for overriding the OpenID experimental role management. Unfortunately, this override also does not work due to the nested array of roles.

For people having this issue with the latest versions of both modules, I suggest moving to the experimental mapping, and implementing the following hook as a quick solution in aad:

/**
 * Implements hook_openid_connect_userinfo_alter().
 */
function MODULENAME_openid_connect_userinfo_alter(array &$userinfo, array $context) {
  if ($context['plugin_id'] !== 'windows_aad') {
    return;
  }

  $userinfo['groups'] = array_column($userinfo['groups'], 'id');
}

This will get rid of the nested array. This does not impact AAD module, but you MUST keep the map_ad_groups_to_roles option because without it, the groups won't be retrieved at all and you will get an error in the hook because of missing groups.

The code below is in web/modules/contrib/openid_connect_windows_aad/src/Plugin/OpenIDConnectClient/WindowsAad.php

// If AD group to Drupal role mapping has been enabled then attach group
// data from a graph API if configured to do so.
if ($this->configuration['map_ad_groups_to_roles']) {
  $userinfo['groups'] = $this->retrieveGroupInfo($access_token);
}

The only place the groups are used in AAD is for the mapping, so there's no other place. Also, I would keep the mapping to manual, and keep the textarea empty to allow the open ID connect module do its job. Move your mappings to the parent module and everything should work well. And, in the meantime, the AAD module could be reworked or people can come up with ideas/suggestions.

The textfields in the experimental open id general settings for roles mappings might not be enough for AAD because of the long hashed ids. If you have multiple groups that you want to map to the same role, that can be a problem unless you override the value from settings local.

The quick solution in #4 helped in my case, thanks @bbu23 very good instructions!

I think the problem has already been fixed in 🐛 AD Groups no longer mapping to Drupal roles Fixed .

Thanks for the new release @webflo!

I also tried the latest openid_connect_windows_aad 2.0.0-beta6 (with openid_connect 3.0.0-alpha2) and the same PHP error still persists when logging in as an Azure user.
Warning: Array to string conversion
in
Drupal\openid_connect\OpenIDConnect->saveUserinfo()
here -> openid_connect/src/OpenIDConnect.php

If I use the quick fix from #4 I get a new error with the openid_connect_windows_aad 2.0.0-beta6:
TypeError: Cannot access offset of type string on string in function openid_connect_windows_aad_openid_connect_userinfo_save() (line 70

At this point it looks like I can only get the logging in part to work without errors if I use openid_connect_windows_aad 2.0.0-beta5 and the quick fix from #4.

Maybe there's still something wrong with the role mapping (or something else) at least in my end. I have set the role mapping in the general (experimental) OpenID connect settings and I tried with and without them in OpenID AAD settings. My project is still D9 (I'm in the process of updating it to D10).

@hartsak you are right. This is a different issue than 🐛 AD Groups no longer mapping to Drupal roles Fixed .

The AD-group mapping in openid_connect_windows_aad is a few more features and options than the openid_connect module. Therefore I don't want to remove it.

Not sure if the "groups" array is considered as "API" of this module. Custom modules may use the properties in the nested array. Making it flat is an BC-break and could cause issues. But we have to it anyways I guess, or openid_connect adds a patch and skips the nested array during processing.

Any opinions? I would like to add a seperate "add_groups" key which contains the nested array and "groups" becomes a flat list.

@webflo I think having two keys, one with nested and one with flat groups is a very good idea. This would allow us to choose between ad groups mapping or parent module groups mapping.

Yes, I think that the "Manual mappings" textarea from openid_connect_windows_aad module works better for the long hashes used in Azure groups. Having those two keys sounds like a good idea if it allows us to continue using the textarea in the AAD settings for role mappings.

Unfortunately my project is currently in a state where I can't really do any debugging until I get the D10 update to progress further. I think I can still use it for testing some patches etc. when they are available if it is of any help!

I'm seeing this issue as well on a new D10 build. Happy to test out any future patches.

Implemented the aad_groups key as described in comment #8.

This is a great start!
I'm not sure what is the best approach for people who want to use the mapping in the parent module. Because the groups are not loaded unless we choose the option "Map user's AD groups to Drupal roles". So, maybe one option would be to add a third radio in the "Method for mapping AD groups to roles" field that says that the mapping will be done by the parent. Currently, if I want to use the parent mapping and we check the checkbox from AAD, I select "Manual" and leave the AAD mapping empty in order to skip the submodule and allow the parent to take over.

This patch works for me without any problems. Can we close this issue?

I don't get the error message anymore either.

Nice! It seems that the errors are gone when I use the patch from #12.
I tried with openid_connect_windows_aad 2.0.0-beta6 and without the "quick fix" from #4.

Thanks for the fix!

@bbu23 Thanks for the suggestion. The UI can be adjusted in a feature release.

Automatically closed - issue fixed for 2 weeks with no activity.