nbgrp / onelogin-saml-bundle

OneLogin SAML Symfony Bundle
BSD 3-Clause "New" or "Revised" License
43 stars 13 forks source link

Allow to override AuthRegistry #34

Closed podhy closed 1 year ago

podhy commented 1 year ago

This PR allows to change AuthRegistry implementation.

It changes getDefinition to findDefinition in AuthRegistryCompilerPass which allows to search for aliases.

codecov-commenter commented 1 year ago

Codecov Report

Merging #34 (3c38c95) into 1.3 (907a594) will not change coverage. The diff coverage is 100.00%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Impacted Files Coverage Δ
...ncyInjection/Compiler/AuthRegistryCompilerPass.php 100.00% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 907a594...3c38c95. Read the comment docs.

a-menshchikov commented 1 year ago

Hi @podhy. Could you explain why do you need to get the definition by the alias?

podhy commented 1 year ago

Hi @a-menshchikov , I would like to replace AuthRegistry with my own implementation, which can return SAML configuration which is defined in external source.

I know I can replace implementation using CompilerPassInterface in Kernel, but this is more elegant.

a-menshchikov commented 1 year ago

@podhy you can override default AuthRegistry with your implementation by the following config:

services:
  # ...
  Nbgrp\OneloginSamlBundle\Onelogin\AuthRegistryInterface:
    class: 'App\YourAuthRegistry'

Or you cannot doing this way?

podhy commented 1 year ago

@a-menshchikov yes this was first thing I made. I added into services.yaml

Nbgrp\OneloginSamlBundle\Onelogin\AuthRegistryInterface: '@App\AuthRegistry'

But this throws following exception:

image

My PR fixes this and allows to override registry

Tested on Symfony 6.2.6 - but this is Symfony version independent

a-menshchikov commented 1 year ago

@podhy you right about an error — you replace service ID "Nbgrp\OneloginSamlBundle\Onelogin\AuthRegistryInterface" by the alias to your service with the same name and this leads the error when the AuthRegistryCompilerPass try to get definition of the service this such ID.

You just need to override class for "Nbgrp\OneloginSamlBundle\Onelogin\AuthRegistryInterface" service ID. Try the config I suggested above.

a-menshchikov commented 1 year ago

@podhy I close this PR because it seems you tried to override the AuthRegistry in a wrong way.