England, UK
Account created on 9 August 2011, over 12 years ago
#

Recent comments

πŸ‡¬πŸ‡§United Kingdom smaz England, UK

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;
  }
πŸ‡¬πŸ‡§United Kingdom smaz England, UK

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

πŸ‡¬πŸ‡§United Kingdom smaz England, UK

+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?

πŸ‡¬πŸ‡§United Kingdom smaz England, UK

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

πŸ‡¬πŸ‡§United Kingdom smaz England, UK

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!

πŸ‡¬πŸ‡§United Kingdom smaz England, UK

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.

πŸ‡¬πŸ‡§United Kingdom smaz England, UK

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.

Production build https://api.contrib.social 0.61.6-2-g546bc20