🇬🇧United Kingdom @smaz

Can confirm the suggestion in #2 works & allows you to have secret keys outside of config files.

@noregrebt & anyone else not able to update to D10:

After initially installing the module with composer in D9, I:
* moved the module to the /web/modules/custom folder
* applied this MR as a patch https://git.drupalcode.org/project/shopify/-/merge_requests/33/diffs - you can download it as a patch from the 'code' dropdown button top right
* removed the module from composer
* added the module to my git repository

This allowed me to update to D10 & it's been about 8 months, have had no issues.

+1 for another alpha release, have also been looking to a bug that was fixed 8 months ago.

If a new release is blocked/waiting on certain issues, is there a list of those available anywhere so we can help with them?

Cheers :)

I finally got some time to sit down with this one today & webflo is correct, but anyone overriding the key in the old way will need to make a change:

Before, we would override the client_secret with the following:

$config['openid_connect.client.windows_aad']['settings']['client_secret'] = "abc123...";

After the module started supporting the key module, the client_secret is intended to be the ID of a key instead. That key gets loaded & the value retrieved - but because of our override, it wasn't able to load the key so caused the error.

$this->keyRepository->getKey($this->configuration['client_secret'])->getKeyValue();

You'll need to create a key at admin/config/system/keys (if you haven't already / one hasn't already been setup by an update hook), then update your Open ID client config (admin/config/people/openid-connect) to set your Windows AAD client to use that key.

Then in settings.php, replace the above config line with

$config['key.key.openid_connect_windows_aad_key']['key_provider_settings']['key_value'] = "abc123...";

(you may need to change the config name if your key is named something different)

You can still override endpoints / client ID in the old way:

$config['openid_connect.client.windows_aad']['settings']['authorization_endpoint_wa'] = "https://login.microsoftonline.com/{tenantId}/oauth2/v2.0/authorize";
$config['openid_connect.client.windows_aad']['settings']['client_id'] = "{clientId}";
$config['openid_connect.client.windows_aad']['settings']['token_endpoint_wa'] =  "https://login.microsoftonline.com/{tenantId}/oauth2/v2.0/token";

Closing as won't fix, but if I get a chance I might try and submit a patch to handle the error here - the key may not exist if it's been deleted for example:

  protected function getRequestOptions(string $authorization_code, string $redirect_uri): array {
    $options = parent::getRequestOptions($authorization_code, $redirect_uri);
    $options['form_params']['client_secret'] = $this->keyRepository->getKey($this->configuration['client_secret'])->getKeyValue();
    return $options;
  }

Patch fixed the issue for me!

+1 for this working. I applied the patch in #6 to 8.x-1.3 via composer - #10 didn't apply for me.

+1 as umami maintainer for 'Directions', if we're going with the approach of just changing the label I've created an MR for that:

https://git.drupalcode.org/project/drupal/-/merge_requests/4244

I guess this could also be added to 10.1.x too? Not sure if I need to do another MR for that?

smaz → made their first commit to this issue’s fork.

I've been using the patch from #7 for a couple of months on a production site without issue.

I was having problems where entities from the paragraphs module wouldn't be shown on a preview, but that all works after this patch.

Patch looks ok to me, unless this should be a setting to allow it or something? Marking as RTBC for now though!

I've just run into this when working on Drupal 9 - removing drush (composer remove drush/drush), requiring core-dev then re-adding drush works fine & gets around the issue. I'm not sure there's anything core needs to do here, though perhaps the documentation could suggest to remove & re-add drush.

I've just tried this without specifying a Drupal version for the recommended project (so currently uses D10), and was able to install drush before core-dev without any issues, so perhaps this is specifically an issue with D8/D9.

I've been using patch #9 in production for around 2 months now on a very active site & had no issues.

Having a look at the code in more detail, just a couple of minor nitpicks:

+/**
+ * Implements hook_entity_type_alter().
+ */
+function access_unpublished_entity_type_alter(array &$entity_types) {
+  // Provide link templates for the access-tokens route on all applicable
+  // entity types.
+  /** @var \Drupal\Core\Entity\EntityTypeInterface[] $entity_types */
+  foreach ($entity_types as $entity_type) {
+    if (AccessUnpublished::applicableEntityType($entity_type) &&
+        $entity_type->hasLinkTemplate('canonical')) {
+      if (!$entity_type->hasLinkTemplate('access-tokens')) {
+        $entity_type->setLinkTemplate('access-tokens', $entity_type->getLinkTemplate('canonical') . '/access-tokens');
+      }
+    }
+  }
+}

AccessUnpublished::applicableEntityType already checks for the entity type having the canonical link template, so we probably don't need to check that here too?

+ * Build the access token overview form for the provied entity.
Typo in provided

* Subscriber for entity access_unpublish routes.
I think this should be access_unpublished

Apart from that, all seems ok - will try to update the merge request with the above.