Skip to content

Only filter attributes if there is actually a list of allowed values to filter on#2095

Merged
tvdijen merged 1 commit intosimplesamlphp:simplesamlphp-2.2from
nathanjrobertson:attlimit_attribute_filter_bug
May 13, 2024
Merged

Only filter attributes if there is actually a list of allowed values to filter on#2095
tvdijen merged 1 commit intosimplesamlphp:simplesamlphp-2.2from
nathanjrobertson:attlimit_attribute_filter_bug

Conversation

@nathanjrobertson
Copy link
Copy Markdown
Contributor

Fixes https://github.com/simplesamlphp/simplesamlphp/actions/runs/9028875799/job/24810192242#step:11:19 as referenced in #1971 (comment)

Warning: PHP Warning:  Undefined array key "/^mail$/" in /home/runner/work/simplesamlphp/simplesamlphp/modules/core/src/Auth/Process/AttributeLimit.php on line 181
Warning: PHP Warning:  Undefined array key "/^mail$/" in /home/runner/work/simplesamlphp/simplesamlphp/modules/core/src/Auth/Process/AttributeLimit.php on line 181

The issue the warning is complaining about is handled in the first line of the filterAttributeValues() function anyway (it handles the second argument being null), so the warning this PR fixes was harmless and condition already handled. In any case, this PR removes the noise of the warning, but should have no other impact, as the behaviour is unchanged.

In the tests, the condition occurs when using SP metadata (ie. $config = ['default' => true, ... ];), as $allowedAttributeRegex is a simple list, whereas when coming out of config files it's a key/value map.

@codecov
Copy link
Copy Markdown

codecov bot commented May 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 44.83%. Comparing base (e4d8d9b) to head (0ee8726).
Report is 1 commits behind head on simplesamlphp-2.2.

Additional details and impacted files
@@                 Coverage Diff                  @@
##             simplesamlphp-2.2    #2095   +/-   ##
====================================================
  Coverage                44.83%   44.83%           
- Complexity                3836     3837    +1     
====================================================
  Files                      161      161           
  Lines                    12824    12825    +1     
====================================================
+ Hits                      5749     5750    +1     
  Misses                    7075     7075           

@tvdijen tvdijen merged commit b7f0dd2 into simplesamlphp:simplesamlphp-2.2 May 13, 2024
tvdijen pushed a commit that referenced this pull request May 13, 2024
tvdijen pushed a commit that referenced this pull request May 13, 2024
@nathanjrobertson nathanjrobertson deleted the attlimit_attribute_filter_bug branch May 13, 2024 10:06
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants