🇦🇺Australia @cafuego

Gracias! :-)

cafuego → created an issue.

Okay, I think I know where you're coming from.

I had a look at the PR and I *think* it's probably nicer to not throw an access denied exception but instead return a new 403 response:

return new BridgeResponse([], 403);

If you're going to add a new alter hook, you're going to have to add documentation for it in oauth2_server.api.php ;-)

The code and service for OAuth2DrupalAuthProvider both look fine to me.

@PrabuEla that backtrace looks like it might be an issue with bshaffer/oauth2-server-php. Which version of that library is installed?

I think this can be closed; the phpcs tests now run with PHP 8.2 and pass.

I did a wee bit of refactoring to use the request object instead of global $_GET but that's it. Merged, cheers :-)

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

I just hit this error with the 2.0.0 version. I did *not* use a `drush uli` login URL, but tried to login with my username, password and 2FA token.

This happens when I enable the module (and js_cookie) via config/core.extensions.yml and do not include any other config.

The remaining test fails are not my responsibility :-)

It's a bit of scope creep, but may it would be good to also add in support for the `DurationSeconds` and `ExternalId` parameters.

cafuego → created an issue.

This should work, but by the time you have 50 clients, the permissions form would be a 100MB page load, so there are some potential practical obstacles ;-)

I have a similiar issue for my use case and what I've done is link a taxonomy vocabulary to my users. The taxonomy terms match sites that uses should be allowed to access. To grant access, I tag a user. To remove access, I untag them. A different vocabulary controls roles.

To achieve that, I've written a small custom module that maps the vocabularies to custom claims in `hook_oauth2_server_user_claims_alter()`. The client side of things can then check the claims and grant access and permissions based on that.

cafuego → created an issue.

Fine, so with SESv2 is actually works as-is. Never mind.

cafuego → created an issue.

It never ends, huh?

Re-done as MR (so the tests can run) and merged. Thank you!

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

This seems to have been fixed as part of the giant Drupal 10 compat PR.

I'm not sure how you ended up with so many rows in the `__scopes` tables; I have exactly 0 in both.

However, since the current cron hook *does* limit by the expires field on both oauth2_server_token and oauth2_server_authorization_code it is entirely reasonable to create an index on those fields.

All merged. Thanks!

Thanks for the patch @dpacassi. I'm merging the older 🐛 Oauth2Controller uses a LoggerChannelFactory argument instead of LoggerChannelFactoryInterface Active instead, they got in first ;-)

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

I've merged a large Drupal 10 compat patch set and this patch either needs a refactor or is no longer needed.

This patch no longer applies after I merged the large Drupal 10 auto-compat MR. It will need a re-factor (if you refactor it to a merge requests, we can run the tests! :-)

I am closing this as a duplicate of 🐛 Deprecated function: trim(): Passing null to parameter #1 ($string) of type string is deprecated in OAuth2DrupalAuthProvider->applies() Needs review . Also patches or merge requests should be against the latest dev branch, not against an (old) release tag.

Merged, thank you.

Nice, that does the trick. Thank you :-)

Merged, thank you.

Thanks everyone, merge request is merged.

Thanks @bojanz 🙏🏼

I've started seeing this on recent builds that used composer 2.7.0 or newer as root in a docker image.

In that case, the issue is that composer no longer runs plugins (like say the drupal-scaffold plugin) as root unless you set COMPOSER_ALLOW_SUPERUSER=1 in the environment.

This is in regards to the https://www.drupal.org/project/oauth2_server → project.

I contacted bojanz via the d.o contact form, but have not had a response.

Your patch definitely fixes the issue at hand. It still applies to newer versions of tome as well, so I expect they cause the same problem if not patched.

I'd try with newer versions of tome, but v1.12 won't install as-is in Drupal 9 and once I do make it install by bodging the info yaml, the drush commands seem to not get loaded.

cafuego → created an issue.

I have a patch for a work-around, but I'm not sure if this isn't a bad idea :-)

class AmazonSesMailQueue extends QueueWorkerBase implements ContainerFactoryPluginInterface {
  use HandlerTrait;

  /**
   * {@inheritdoc}
   */
  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
    $instance = new static(
      $configuration,
      $plugin_id,
      $plugin_definition
    );

    // Only set the handler if queueing is enabled to avoid an error when
    // trying to run without config.
    $enabled = \Drupal::config('amazon_ses.settings')->get('queue');
    if ($enabled) {
      $instance->setHandler($container->get('amazon_ses.handler'));
    }

    return $instance;
  }

cafuego → created an issue.

I may need to add in a bit more work, the test tab also WSODs currently. May as well include that.

The error message about quota is a bit deceptive given it shows on the statistics tab, but the quota function is the one that fails, so ¯\_(ツ)_/¯

cafuego → created an issue.

cafuego → created an issue.

The patch from #11 seems to work fine for me with oauth2-server-php v1.14.1

You shouldn't need to both pin the library to an old version *and* patch the Drupal module to support the new one.

@v.koval For the record, my patch only works on the beta if you also apply the other patches I listed in the comment.

I enabled domain, manually deleted field_domain_admin from the user entity, ran cron enough times that all users would be processed given a batch size of 50, then uninstalled domain. The error now longer occurs. (It only ever occurred during a cron run for me)

I have a set of patches that works for 2.0.0-beta4 (which I use because part of the refactor of 2.0.x-dev has broken a thing I need) with Drupal 10 and PHP 8.2:

• The MR from comment 9
• The MR from comment 14
• The patch from issue #3366668
• https://www.drupal.org/files/issues/2023-10-27/oauth2_server.d10.patch →

We upgraded to DER 3 and the problem went away.

cafuego → created an issue.

Update: There are in fact no triggers in the database at all 🫠 so a conditional should sort this.

cafuego → created an issue.

Getting the same thing here on a glossary page; my work-around is to use an input format without Onomasticon enabled for the vocab descriptions.

The patch from #11 seems to apply just fine to the 2.0.x branch. Whether it makes the module work with D10 is another issue :-)

I think it can probably do with a bit of a refactor. As-is it will request new temporary credentials each time it makes an API call, but the temporary credentials it receives are (by default) valid for an hour, so it shouldn't need to.

My feature is not pretty, but assuming the roles are set up with the correct trust policy, this works!

cafuego → created an issue.

This issue is now making my drush calls fail on Drupal 10.1 because my skeleton settings.php used require_once to grab pretty much *all* actual config from a place I manage with config-as-code tools. When I run drush I get:

In Settings.php line 171:                                                    
    Missing $settings['hash_salt'] in settings.php.

Of course, when Drupal is accessed through the web server, it bootstraps fine and everything works as expected.

cafuego → created an issue.

Works as advertised. Thank you :-)

The patch applies, but I still get deprecation warnings on openssl_free functions in the jwk controller.

I've pushed a fix for that to the issue fork.

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

Made a PR with a simple variable rename.

larowlan → credited cafuego → .

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