lightSAML / SpBundle

SAML2 SP Symfony Bundle based on LightSAML
https://www.lightsaml.com/SP-Bundle/
MIT License
66 stars 70 forks source link

Use username mapper to resolve username when creating a default user #40

Closed denniscoorn closed 7 years ago

denniscoorn commented 7 years ago

This pull request originates from the situation where I had the force option enabled in the firewall and then wanted to control the username resolving by setting my own list of attributes on the light_saml_sp.username_mapper configuration.

This wasn't working and it turned out that the username mapper was never used when the LightsSamlSpAuthenticationProvider wants to create a default user. In fact, the logic used in the createDefaultUser method looks very similar to the logic of the SimpleUsernameMapper. As far as my interpretation of this bundle goes there's no necessity why this logic should be duplicated and could possibly cause unexpected behavior, as described above.

That's why I'm proposing to use the username mapper for resolving the username when a default user should be created. In my opinion this change has three main benefits:

Would like to know what you think of it!

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.05%) to 98.98% when pulling 325b99a75da098f043d61b60f6e52f40b22addfe on Karify:feature/create-default-user-username-mapper into 2cb00e7bbfdb68c6eb7117cf1a35756e9cbddbe7 on lightSAML:master.

tmilos commented 7 years ago

Yes, we could use it, had ago such idea but lost it. Thanks