🇺🇦Ukraine @chesn0k

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

Not resolved for id_token

chesn0k → created an issue.

This won’t cause critical errors, but some of the payload might be lost, which could break client-side authorization.

Ok, I'll open a new issue for this.

This bad commit is not compatible with lcobucci/jwt version 5 and above because Lcobucci\JWT\Token\Builder is immutable

All calls like
$builder->withClaim($claim_name, $value);

should be rewritten to
$builder = $builder->withClaim($claim_name, $value);

chesn0k → created an issue.

I think we need to back to this since there will be a release Drupal 10.3 soon and this module will block the update

The changes look good

And so the current implementation of autowiring allows create a plugin instance in this way:

$instance = \Drupal::service(DependencyAutowire::class)->autowireClass(
  new \ReflectionClass($plugin_class),
  [
    'configuration' => $configuration,
    'plugin_id' => $plugin_id,
    'plugin_definition' => $plugin_definition,
  ],
);

Plugin __construct:

public function __construct(
  array $configuration,
  string $plugin_id,
  array $plugin_definition,
  protected readonly RouteMatchInterface $routeMatch,
) {
  parent::__construct($configuration, $plugin_id, $plugin_definition);
}

This is not bad, but I think the __construct method signature is too strict, plugins that not required to extends the base plugin could use property promotion:

public function __construct(
  protected array $configuration,
  protected string $pluginId,
  protected array $pluginDefinition,
  protected readonly RouteMatchInterface $routeMatch,
) {}

I think we should search for arguments using snake_case and camelCase styles this can be implemented inside the dependency autowiring service.

Also should I remove all uses of AutowireTrait in current branch and mark as deprecated?

I think this would require a recursive call, okay let's skip that case for now.

If the Drupal Core provided aliases for the serializer it would look something like this:

services:
  serializer:
    class: Symfony\Component\Serializer\Serializer
    arguments: [{  }, {  }]
  Symfony\Component\Serializer\Normalizer\SerializerInterface: '@serializer'
  Symfony\Component\Serializer\Normalizer\DenormalizerInterface: '@serializer'
  Symfony\Component\Serializer\Normalizer\NormalizerInterface: '@serializer'

Assumed controller constructor:

public function __construct(
    protected NormalizerInterface&DenormalizerInterface $normalizer,
) {}

In this case, we can check if all interfaces of a intersection type refer to the same service. If any of the interfaces is not an alias or refers to another service throw an AutowiringFailedException.

For a union type, we can iterate over all types until the first service found.

What do you think about it?

chesn0k → created an issue.

Plugins should also be autowireable, see #3294266: Allow plugin service wiring via constructor parameter attributes, hopefully we can unify some of the code there as well?

Yes, this seems like a good reason to move autowire into a separate service.

This should also mean we can drop AutowireTrait from ControllerBase?

Yes.

service specifically for handling autowiring

Good option, although I thought that this was the responsibility of the class resolver.

Also, taking another look at the code, I noticed that there is no support for the union and intersection types. Should I put this in a separate issue?

I checked the only file I created settings.php without a space.
I also deleted the installation and reinstalled it and now I can’t get this bug again.

In any case, I now think this is not connected with drupal, but with the IDE (PHPStorm) or the environment

I'm having this issue in a new installation of Drupal.

1 drush si
2 Try opening Announcements

I tracked the response body while calculating the content length, the response body is valid.
If assume that the length calculated is incorrect and add +1 this will solve the issue.

Drupal\Core\StackMiddleware\ContentLength:
$response->headers->set('Content-Length', strlen($content) + 1, TRUE);

I have no idea where to look for the bug next )

chesn0k → changed the visibility of the branch 3409787-add-support-for to active.

chesn0k → changed the visibility of the branch 3409787-add-support-for to hidden.

chesn0k → created an issue.

Add the `system.js_asset` and `system.css_asset` routes to the excluded routes. I think the module should do this during installation or programmatically in EmailTfaSubscriber.

Updated docblock

Removed all use of dynamic properties in this patch.

chesn0k → created an issue.

+1

In this case, it would be logical to use the query param "pass-reset-token" (see Drupal\user\AccountForm line 134) in order to skip TFA.

Added email_tfa cache context

chesn0k → created an issue.