Problem/Motivation

We have noticed in our site that, after updating to 2.1.0, the attributes we were adding to the CAS response have stopped working and the following error is on the code:

Warning: foreach() argument must be of type array|object, true given in Drupal\cas_server\Controller\TicketValidationController->generateSuccessResponse() (line 383 of /var/www/html/web/modules/contrib/cas_server/src/Controller/TicketValidationController.php) 

debuggin the error: we see that in L382 of the mentioned file we have:
$value = is_array($value) ?: [$value];

so, if we are passing to the attributes (eg: via a custom alter event, like we do), certain values, if the values is not an array will be OK (cause then $value will be an array) BUT if $value is already an array, then it will be converted to bool (cause is_array will return true instead of $value)

Steps to reproduce

Test with some additional attributes of type array; those will be removed and the error will raise

Proposed resolution

Change L382 to:
$value = is_array($value) ? $value : [$value];
(will do the MR in a bit)

Remaining tasks

  • ✅ File an issue
  • ✅ Addition/Change/Update/Fix
  • ✅ Testing to ensure no regression
  • ✅ Automated unit testing coverage
  • ✅ Automated functional testing coverage
  • ➖ UX/UI designer responsibilities
  • ➖ Readability
  • ➖ Accessibility
  • ➖ Performance
  • ➖ Security
  • ➖ Documentation
  • ✅ Code review by maintainers
  • ➖ Full testing and approval
  • ➖ Credit contributors
  • ➖ Review with the product owner
  • ➖ Release notes snippet
  • ❌ Release

Issue fork cas_server-3521968

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

gorkagr created an issue. See original summary.

gorkagr’s picture

Status: Active » Needs review

MR with the change created, can be reviewed :)

elc’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

Whoops, my bad. That certainly fixes the problem. Are you able to run with a patched version for now?

It very much sounds like we should add a test to trigger this same situation to prevent such a fault again.

gorkagr’s picture

Yes, deploying the patch on the MR right now on our site :)
that will do the trick :D

elc’s picture

Issue summary: View changes
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs tests

Could only think to add an array to the attributes. Is there anything else to alter into the attributes to press the limits?

  • elc committed ac39919f on 2.1.x authored by gorkagr
    [#3521968] by gorkagr, elc: Fix wrong use of "?:".
    

  • elc committed 48e3147d on 3.0.x authored by gorkagr
    [#3521968] by gorkagr, elc: Fix wrong use of "?:".
    
elc’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.