Mise à jour #30005

Échec : InvalidArgumentException : The configuration property settings.prompt.login doesn't exist. dans Drupal\Core\Config\Schema\ArrayElement->get() (ligne 100 de /home/u208064804/domains/bdxfr.e-bordeauxmetropole.org/com-drupal11-build/web/core/lib/Drupal/Core/Config/Schema/ArrayElement.php).

Issue fork openid_connect-3577049

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mably created an issue. See original summary.

mably’s picture

mably commented 
#0 /var/www/html/web/core/lib/Drupal/Core/Config/StorableConfigBase.php(224): Drupal\Core\Config\Schema\ArrayElement->get('settings.prompt...')
#1 /var/www/html/web/core/lib/Drupal/Core/Config/StorableConfigBase.php(259): Drupal\Core\Config\StorableConfigBase->castValue('settings.prompt...', 'login')
#2 /var/www/html/web/core/lib/Drupal/Core/Config/StorableConfigBase.php(259): Drupal\Core\Config\StorableConfigBase->castValue('settings.prompt', Array)
#3 /var/www/html/web/core/lib/Drupal/Core/Config/StorableConfigBase.php(259): Drupal\Core\Config\StorableConfigBase->castValue('settings', Array)
#4 /var/www/html/web/core/lib/Drupal/Core/Config/Config.php(211): Drupal\Core\Config\StorableConfigBase->castValue(NULL, Array)
#5 /var/www/html/web/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php(260): Drupal\Core\Config\Config->save(false)
#6 /var/www/html/web/core/lib/Drupal/Core/Entity/EntityStorageBase.php(525): Drupal\Core\Config\Entity\ConfigEntityStorage->doSave('entraid', Object(Drupal\openid_connect\Entity\OpenIDConnectClientEntity))
#7 /var/www/html/web/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php(239): Drupal\Core\Entity\EntityStorageBase->save(Object(Drupal\openid_connect\Entity\OpenIDConnectClientEntity))
#8 /var/www/html/web/core/lib/Drupal/Core/Entity/EntityBase.php(370): Drupal\Core\Config\Entity\ConfigEntityStorage->save(Object(Drupal\openid_connect\Entity\OpenIDConnectClientEntity))
#9 /var/www/html/web/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php(635): Drupal\Core\Entity\EntityBase->save()
#10 /var/www/html/web/modules/contrib/openid_connect/openid_connect.install(434): Drupal\Core\Config\Entity\ConfigEntityBase->save()
#11 /var/www/html/web/core/includes/update.inc(184): openid_connect_update_30005(Array)
#12 /var/www/html/web/core/includes/batch.inc(300): update_do_one('openid_connect', 30005, Array, Array)
#13 /var/www/html/web/core/includes/batch.inc(139): _batch_process()
#14 /var/www/html/web/core/includes/batch.inc(96): _batch_do()
#15 /var/www/html/web/core/modules/system/src/Controller/DbUpdateController.php(201): _batch_page(Object(Symfony\Component\HttpFoundation\Request))
#16 [internal function]: Drupal\system\Controller\DbUpdateController->handle('start', Object(Symfony\Component\HttpFoundation\Request))
#17 /var/www/html/web/core/lib/Drupal/Core/Update/UpdateKernel.php(115): call_user_func_array(Array, Array)
#18 /var/www/html/web/core/lib/Drupal/Core/Update/UpdateKernel.php(76): Drupal\Core\Update\UpdateKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request))
#19 /var/www/html/web/update.php(27): Drupal\Core\Update\UpdateKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#20 {main}
mably’s picture

mably commented

The root cause is a mismatch between the stored data and the config schema for the prompt setting.

The schema correctly defines prompt as a sequence (indexed array of strings), but the actual stored value is an associative array with string keys — ['login' => 'login'] — because #type => checkboxes produces that format and submitConfigurationForm() never normalizes it. defaultConfiguration() also hardcodes the associative format.

When Drupal 11's strict config validation encounters the string key login, it tries to resolve the property path settings.prompt.login against the schema — which only defines numeric sequence items (settings.prompt.0, settings.prompt.1, etc.). This causes the InvalidArgumentException.

An additional complication: contrib plugins like openid_connect_windows_aad store prompt as a plain string (e.g. <noprompt>) with their own radios form element and read it back as a string in getUrlOptions(). The base class must handle both formats gracefully.

The fix normalizes the data to match the schema, with guards for contrib string values:

1. OpenIDConnectClientBase.php

  • defaultConfiguration(): return ['login'] instead of ['login' => 'login']
  • submitConfigurationForm(): normalize checkboxes output to an indexed array with array_values(array_filter(...))
  • #default_value: add an (array) cast so contrib string values don't crash buildConfigurationForm()
  • getUrlOptions(): remove the now-unnecessary array_filter() wrapper since stored data is already clean

2. openid_connect.install

  • openid_connect_update_30005(): set ['login'] (indexed) instead of ['login' => 'login'] (associative), and only for clients that don't already have a prompt value — contrib plugins that store prompt as a string are left untouched
  • openid_connect_requirements(): add an is_array() guard before array_filter() so contrib string values don't crash the status page

mably opened merge request !184

mably’s picture

mably commented
Status: Active » Needs review
ericgsmith’s picture

ericgsmith commented

Code changes look good and explanation in #3 is clear and correct.

Initially I thought there might be a bug - but then when I was checking the note around openid_connect_windows_aad if found that while that module defines prompt as a string, it didn't ship with an update hook to set the default value - so I had the schema in place for that plugin but not an existing value. So with this patch applied I got:

>  [notice] Update started: openid_connect_update_30005
>  [error]  The configuration property settings.prompt.0 doesn't exist. 
>  [error]  Update failed: openid_connect_update_30005

Saving my openid_connect_windows_aad client settings prior to update db allow the update to run as expected.

I don't specifically think that openid_connect needs to cater for that scenario - but its an interesting problem of the base class introducing something that is already defined by other plugins.

richard.thomas’s picture

richard.thomas commented

We updated to 8.x-1.5 and noticed this also might be broken out of the box after the update hook runs? It looks like the numeric value from the config entity and the keyed value from defaultConfiguration() combine so we end up with ?prompt=login%20login in the redirected URL which breaks (at least on Entra ID).

richard.thomas opened merge request !185

richard.thomas’s picture

richard.thomas commented

Pushed up a 1.x version of the changes in a separate MR https://git.drupalcode.org/project/openid_connect/-/merge_requests/185.

One issue is the merging of the default plugin configuration 'prompt' setting with what's in the config. By default this was merging the arrays together (if we're going to use a numeric array), meaning there wasn't a way to disable the default login option or have no prompt options selected at all. I've updated this MR to just overwrite the entire prompt array if it's set in the configuration.

dpi’s picture

dpi
Location Perth, Australia
commented

So yes, OIDC module is colliding with openid_connect_windows_aad.

But, OIDC should be namespacing out all third party config into its own sub mapping of the config, but if that isnt possible (requires rearchitecture), then doing its best to not re-use the keys used by plugins is ideal.

Unfortunately AAD uses a string, which is incompatible with the newly introduced prompt key in OIDC. Preferably, OIDC would rename its key. Its going to be a mess if OIDC's migration is broken. AAD also doesnt even have an opportunity to jump in and sort itself out before OIDC's migration runs.

Further, we shouldnt be booting the plugin with ->getPlugin() in a migration, and especially without handling failures. We have no idea if the plugin exists or is not broken. It should be entirely possible to migrate configuration by only dealing with the only the configFactory service.

Going to push an alternative to above MR.

dpi opened merge request !186

dpi’s picture

dpi
Location Perth, Australia
commented

Pushed another new MR.

I should also mention that sure, we could "guess" and cast any existing "prompt" values to an array, but we have no idea how people are actually using it downstream. Sure, it would work for AAD, because we can see and interrogate the code. But people overriding AAD, or people putting together their own plugins, could be doing all kinds of custom things.

It sounds like some of the array-merging aspects may also be applicable.

If this MR is under consideration for rejection for renaming key aspect, please consider: how will downstream handle a migration of their potentially broken/data-loss config. And, in the least, the update strategy should be converted to config factory. Manipulating config entities in .install isnt permitted.

dpi opened merge request !187

dpi closed merge request !187

dpi changed the visibility of the branch d12--next-major-ci to hidden.

cafuego’s picture

cafuego commented

But, OIDC should be namespacing out all third party config into its own sub mapping of the config, but if that isnt possible (requires rearchitecture), then doing its best to not re-use the keys used by plugins is ideal.

Given v3 is still in alpha, this seems like a good time to make that change. It's too late for 1.x.

I think it's completely fine for openid_connect to use arbitrary settings keys, and as long as they're documented any plugins can work around them or at least not try to re-implement them on their own :-)

And I guess that #3577142: prompt schema mismatch with parent plugin did just that, so I'll shut up now 😂

pfrilling made their first commit to this issue’s fork.

pfrilling opened merge request !193

pfrilling’s picture

pfrilling
Location Minster, OH
commented

Apologies everyone for the botched update hook and thanks for all the feedback. I truly appreciate it.

I decided to merge the changes from MR 184 and MR 186 into the new MR 193. I tested that patch along with the patch in #3577142: prompt schema mismatch with parent plugin for openid_connect_windows_aad. With those two patches applied, the new update hook 30006 runs as expected in my project.

I think it makes the most sense to keep the `prompt` option with this upstream project as it is part of the spec and it should have been a setting in this module for some time now.

Please review and let me know if you have any feedback.

richarddavies’s picture

richarddavies commented

I followed your instructions and used both patches (the new MR 193 and MR 46 from #3577142). I can now confirm that all of the update hooks were successful this time and I'm no longer getting an error on the status page.

solideogloria’s picture

solideogloria commented
Status: Needs review » Reviewed & tested by the community

The changes in MR !193 fix the issue.

dpi’s picture

dpi
Location Perth, Australia
commented

Given v3 is still in alpha, this seems like a good time to make that change.

When the alpha has more 3x usage (22k sites) than the one it replaces, a significant portion of Drupal users, I think it loses any "alpha" safety net. It's defacto supported.

As much as I'd hate to be in that situation myself, its brought on by v3 being the only d11 compatible version.

solideogloria’s picture

solideogloria commented

When the alpha has more 3x usage (22k sites) than the one it replaces, a significant portion of Drupal users, I think it loses any "alpha" safety net. It's defacto supported.

Agreed. It's the primary branch that is used -- by almost 75% of OpenID Connect users (~22,000 sites).

ttnt’s picture

ttnt commented

Good day,
Will a new release be made or should we run these 2 patches to keep functionality on the latest version?
Is it perhaps safer to stay on the previous alpha until a release is made?
It is unclear to me how best to proceed right now (regarding risk of breaking things).
Thanks

solideogloria’s picture

solideogloria commented

I'm running with the two patches, because the latest release includes security fixes.

Still, it would be awesome if this could be merged and released!

pfrilling closed merge request !184

pfrilling closed merge request !186

  • pfrilling committed 0f3f6ca6 on 3.x 
    fix: #3577049 Update.php fails with InvalidArgumentException : The...
solideogloria’s picture

solideogloria commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

ttnt’s picture

ttnt commented

Thanks for the release, I could update from alpha6 to alpha8 without issues.
All seems (?) to be working as expected.
No additional patches were needed for my setup.

gravisrs’s picture

gravisrs commented

My case might also be connected.

If I leave "prompt" settings empty my query contains: ...&prompt=0%200%200%200&....

which are imploded 4 element nulls array - "0 0 0 0"

steinmb’s picture

steinmb commented

@gravisrs: Thank for the feedback, go ahead and open a separate issue as this is now closed.

Status: Fixed » Closed (fixed)

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