Skip to content

Symfony 3: updates to @famoser's fork#25

Open
sneakyvv wants to merge 11 commits intormoreas:masterfrom
sneakyvv:master
Open

Symfony 3: updates to @famoser's fork#25
sneakyvv wants to merge 11 commits intormoreas:masterfrom
sneakyvv:master

Conversation

@sneakyvv
Copy link
Copy Markdown

@sneakyvv sneakyvv commented May 2, 2017

famoser's fork causes the following error

FatalThrowableError in ShibbolethListener.php line 54:
Type error: Argument 6 passed to KULeuven\ShibbolethBundle\Security\ShibbolethListener::__construct() must be an instance of Symfony\Component\HttpKernel\Log\LoggerInterface, instance of Symfony\Bridge\Monolog\Logger given, called in /var/www/html/var/cache/dev/appDevDebugProjectContainer.php on line 3498

Symfony\Component\HttpKernel\Log\LoggerInterface is deprecated since Symfony 2.2 (http://api.symfony.com/2.8/Symfony/Component/HttpKernel/Log/LoggerInterface.html) and has been removed in Symfony 3.0, causing this error.

It should use Psr\Log\LoggerInterface instead.

Also, Symfony\Component\Security\Core\SecurityContextInterface is declared, but never used.

Since I was not able to create an issue in famoser's fork, I created a separate fork for this.

@sneakyvv sneakyvv mentioned this pull request May 2, 2017
@rmoreas
Copy link
Copy Markdown
Owner

rmoreas commented Jun 21, 2017

Thanks for the contribution!
Just wondering how are we gonna handle backwards compatibility? Probably there are people still using Symfony 2.x

@sneakyvv
Copy link
Copy Markdown
Author

I guess you would either need two "master branches", i.e. master-sf3 and master-sf2, or if perhaps tagging/versioning the current state would also work.

PS: I suppose you reacted to a notification about my lastest commit (cc508e7). That should probably not be included in the bundle, as it is a fix for my project specifically. I'm needing the u-number instead of the whole email address as a user name. However, since my fork is a fork of your fork, I didn't manage to require it as a separate branch in my composer.json file, so I pushed it to my master branch.
Perhaps we can solve this issue by introducing some kind of hook which would allow altering the username attribute before using it to fetch a user via the UserProvider?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants