Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supress warning on absent NameIDFormat #1055

Merged
merged 1 commit into from Mar 7, 2019

Conversation

Projects
None yet
3 participants
@tvdijen
Copy link
Member

tvdijen commented Mar 4, 2019

After updating to 1.17 I started noticing a lot of warnings in the log:
\Mar 4 12:47:16 sv1810934 IDP-BZK[5791]: 3 [c11b0ed290] Unable to generate NameID. Check the userid.attribute option.

SP doesn't ask for a specific NameIDFormat and none is set in the metadata...
current([]) would return false, but we never catch that, so it falls through,..

@tvdijen

This comment has been minimized.

Copy link
Member Author

tvdijen commented Mar 4, 2019

Also see referenced commit above

@tvdijen tvdijen added this to the 1.17 milestone Mar 5, 2019

@foaadnami

This comment has been minimized.

Copy link

foaadnami commented Mar 6, 2019

@tvdijen hopefully the last issue before 1.17 release :) thanks a lot for all the great work !

@tvdijen

This comment has been minimized.

Copy link
Member Author

tvdijen commented Mar 6, 2019

Yes, I expect 1.17 final to be released later this week..

@thijskh

This comment has been minimized.

Copy link
Member

thijskh commented Mar 7, 2019

So this is the history:

  • PR by @ghalse to make this functionality possible.
  • problem is that it does array_shift() on function output which generates notices.
  • I replaced that with current() which does the same as array_shift but can be used with array output.
  • however, I failed to notice that the return value behaviour is different when no items are found (of course, because this is PHP): array_shift will return null, current will return false.
  • so the solution is to let the if statement check for false instead of null.

@tvdijen tvdijen force-pushed the tvdijen-patch-nameidformat branch from 1f1ba1e to f1320d3 Mar 7, 2019

@tvdijen tvdijen force-pushed the tvdijen-patch-nameidformat branch from f1320d3 to b2474de Mar 7, 2019

@thijskh

thijskh approved these changes Mar 7, 2019

@thijskh thijskh merged commit a51b2ef into master Mar 7, 2019

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 32.905%
Details

@tvdijen tvdijen deleted the tvdijen-patch-nameidformat branch Mar 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.