Problems upgrading openid_connect 8.x-1.4 to 3.x

I've been debugging a bit in openid_connect.

It seems when moving from v1 to v2 the config data changed names, anyway my config from v1 isn't visible anymore in v2.

So I recreated my config. After that. I have my own client that derives from OpenIDConnectClientBase and that doesn't seem to be able to get its config data, e.g. the parentEntityId (which seems to be filled in in plugin initialization) is "", while the config actually has a value for it.
Strange thing is that I can see my OIDC config data in the $configuration variable which is coming from OpenIDConnectClientBase.

Thanks for the information... maybe an entry could be made in the readme file to explain upgrades.

I'm still not out of the woods with the problem but I pinpointed where it is.

But first, I need my own OIDC client since I need a small extension to support a "login_hint" feature. So what I do is make a new client that extends OpenIDConnectClientBase, which is empty besides a copy of the authorize() method from OpenIDConnectClientBase to which I add 2 lines. But for testing purposes I even just extended OpenIDConnectClientBase and removed all methods.

Then I get the error "Symfony\Component\Routing\Exception\InvalidParameterException: Parameter "openid_connect_client" for route "openid_connect.redirect_controller_redirect" must match "[^/]++" ("" given) to generate a corresponding URL. in Drupal\Core\Routing\UrlGenerator->doGenerate() (line 203 of /var/www/html/www/core/lib/Drupal/Core/Routing/UrlGenerator.php)." when I go to my site.
Reason for this is because in the method below the "$this->parentEntityId" is an empty string.

  public function authorize(string $scope = 'openid email', array $additional_params = []): Response {
    $language_none = \Drupal::languageManager()
      ->getLanguage(LanguageInterface::LANGCODE_NOT_APPLICABLE);

    $redirect_uri = Url::fromRoute(
      'openid_connect.redirect_controller_redirect',
      [
        'openid_connect_client' => $this->parentEntityId,
      ],
      [
        'absolute' => TRUE,
        'language' => $language_none,
      ]
    )->toString(TRUE);

    $url_options = [
      'query' => [
        'client_id' => $this->configuration['client_id'],
        'response_type' => 'code',
        'scope' => $scope,
        'redirect_uri' => $redirect_uri->getGeneratedUrl(),
        'state' => $this->stateToken->generateToken(),
      ],
    ];

    if (!empty($additional_params)) {
      $url_options['query'] = array_merge($url_options['query'], $additional_params);
    }

    $endpoints = $this->getEndpoints();
    // Clear _GET['destination'] because we need to override it.
    $this->requestStack->getCurrentRequest()->query->remove('destination');
    $authorization_endpoint = Url::fromUri($endpoints['authorization'], $url_options)->toString(TRUE);

    $this->loggerFactory->get('openid_connect_' . $this->pluginId)->debug('Send authorize request to @url', ['@url' => $authorization_endpoint->getGeneratedUrl()]);
    $response = new TrustedRedirectResponse($authorization_endpoint->getGeneratedUrl());
    // We can't cache the response, since this will prevent the state to be
    // added to the session. The kill switch will prevent the page getting
    // cached for anonymous users when page cache is active.
    $this->pageCacheKillSwitch->trigger();

    return $response;
  }

I compared what the URLs were in openid_connect v1.2.0 and what solves it for me in v3 is this. Replacing $this->parentEntityId by $this->getPluginId() in the authorize() method.

    $redirect_uri = Url::fromRoute(
      'openid_connect.redirect_controller_redirect',
      [
        'openid_connect_client' => $this->getPluginId(),
      ],
      [
        'absolute' => TRUE,
        'language' => $language_none,
      ]
    )->toString(TRUE);

$this->getPluginID() results in my case in "acm-idm" (the system name I used for the plugin, and also the id of the OIDC server). And the URLs I get out of it are:

redirect URI = "https://mysite.be/openid-connect/acm-idm"
authorization url = "https://authentication-server.be/op/v1/auth?client_id=e243df5e-9f0d-4977...

The strange thing is also that on the client config screen the Redirect URL is calculated as "https://mysite.be/openid-connect/acm_idm"... which in my case is wrong: the acm_idm is the system name of the client config (which can't have dashes, while the actual string should "https://mysite.be/openid-connect/acm-idm" (dash instead of underscore), BUT it does find "acm_idm" while during the actual execution that part is always empty. The reason for the discrepancy is that the formbase for the config screen has its own getRedirectUrl() method which calculates the string with the system name of the client config.

So I still have 2 problems:
1. The parentEntityId doesn't get filled in properly. I see it being filled in in the initializePlugin() method of OpenIDConnectClientCollection, but I see no evidence of it actually happening.
2. If parentEntityId would be filled in properly I can't make it work since I need "acm-id" as part of the URIs, and system names in Drupal can't contain a dash, so I can't make an OIDC client system name that I can use with my OIDC server. And I'm only a user of the OIDC server, so I can't change anything to the setup there.

Some more debugging.

Number 1. got solved by running "drush updatedb", my bad.

For number 2. I would suggest to introduce a new field and use that instead of the system name of the instance of the plugin in the redirect URL. For backwards compatibility: when the new field is empty the system name could be used anyway. Sole reason is that Drupal system names can't use '-', and some OIDC providers (e.g. the one of the Belgian government) do use a dash ("acm-idm").
For the name of the name field, no idea what it should be called. Right now the redirect parameter is filled in in "parentEntityId" and it gets taken from "client_id", which it not really is in OIDC terminology.

Short term work-around for me is to override the parameter for the redirect URL in my own client, which would actually revert it a bit more like v1.

I'm still using patches for this one, but it's becoming a huge patch.

The problem is that we have some government sites that use "acm-idm" as slug in the openid_connect redirect URL, and you can't have a system name in Drupal with a "-" in it (in the very old openid_connect 1.x module, you could get the "right" redirect URL with a "-" in it because the redirect used the plugin-name and not the config system name). I have about 20 Drupal sites integrating with the same openid_connect server and I'm not in control of the redirect URLs.

So it's only a problem when you integrate with an openid_connect server using -'s in the redirect URL.

Attached patch works for me on openid_connect 3.0.0-alpha2 (I can reroll it).

It decouples the config name with the name used in the redirect controller.
Because of that some wiring magic is also needed to convert the other way around: to match config with incoming slug in the redirect controller.

Probably a coincidence. I also adapted #20 for 3.0.0-alpha6. I have been using this patch in production now on 1 site for 2 weeks, no-one is complaining so far.

I did add a small piece from another issue as the edits to the same code made me nervous. This is now the only patch I use for 3.0.0-alpha6

Which exact version do I test?

Seems I was looking at a wrong branch.

I tested the 3 dev version. The provider slug is working. For the Belgian government we usually have 'acm-idm' as slug, you can change that slug when you request the integration; but all the old integrations still use 'acm-idm' of course. And I didn't want to go back and change old integrations.