Fix optional configuration in Google provider #53
Conversation
|
Perfect sense 👍 @weaverryan Btw, probably we should use Symfony OptionsResolver component here to make error messages more obvious? UPD: Or maybe we just can fix it with default values for those nodes? |
|
Changed the PR to add default values 👍 |
| ->info('Optional value for sending access_type parameter. More detail: https://developers.google.com/identity/protocols/OpenIDConnect#authenticationuriparameters') | ||
| ->end() | ||
| ->scalarNode('hosted_domain') | ||
| ->defaultValue('') |
There was a problem hiding this comment.
Not sure, but maybe we need null instead for access_type and hosted_domain options?
There was a problem hiding this comment.
Hum, I don't know, I used the defaults proposed in the README here.
Let me know what is you preference 😉
| ->info('Optional value for sending access_type parameter. More detail: https://developers.google.com/identity/protocols/OpenIDConnect#authenticationuriparameters') | ||
| ->end() | ||
| ->scalarNode('hosted_domain') | ||
| ->defaultValue('') | ||
| ->info('Optional value for sending hd parameter. More detail: https://developers.google.com/identity/protocols/OpenIDConnect#hd-param') | ||
| ->end() | ||
| ->arrayNode('user_fields') |
There was a problem hiding this comment.
We need a default value (an empty array) for the user_fields option too, right?
There was a problem hiding this comment.
I tested without defining default value for user_fields and it worked fine. So I suppose arrayNode / prototype make default value useless
|
Thanks for reporting and fixing the bug! I added a bit more work on #54 - I would appreciate it if you tell me if it makes sense for you :). When I merge that, I'll tag a new release. Thanks! |
Avoid notice
Undefined index: access_typeon this line when optional configuration is not given