allow auth adapters to return a single value instead of arrays #651

Closed
wants to merge 2 commits into from

1 participant

@d1rk
Union of RAD member

Change behavior of auth adapters so, that they can return scalar values.

Currently, auth adapters must return arrays, because of a hard-coded unset on $data['password'].

This PR changed the behavior so, that an auth adapter can return a scalar value without being presented with a

Fatal error: Cannot unset string offsets in /libraries/lithium/security/Auth.php on line 160

An updated unit-test is included.

@d1rk
Union of RAD member

feedback on this issue would be appreciated, as i can not update to latest version of lithium on a series of near-production applications. Thanks for your time and support.

@nateabele nateabele commented on the diff Sep 24, 2012
security/Auth.php
}
}
} else {
- unset($data['password']);
+ if (is_array($data) && isset($data['password'])) {
+ unset($data['password']);
+ }
@nateabele
Union of RAD member

Here, you can rewrite the entire if/else block this way:


if ($options['persist'] && is_array($data)) {
    $data = array_intersect_key($data, array_fill_keys($options['persist'], true));
} elseif (is_array($data)) {
    unset($data['password']);
}

Do that, squash the commit, and re-point this at dev, and I'll get it merged right away. Good work on the test case!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@nateabele nateabele commented on the diff Sep 24, 2012
tests/cases/security/AuthTest.php
+ 'test' => array(
+ 'adapter' => $this->_classes['mockAuthAdapter'],
+ )
+ ));
+
+ $user = array(
+ 'id' => '123',
+ 'username' => 'foobar',
+ 'password' => 'not!important',
+ 'email' => 'foo@bar.com',
+ 'insuranceNumer' => 1234567
+ );
+
+ $expected = 123;
+
+ $result = Auth::check('test', $user, array('key_only' => true, 'checkSession' => false));
@nateabele
Union of RAD member

Oh, also, to be consistent with coding standards, 'key_only' should be 'keyOnly'. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@d1rk
Union of RAD member

Thanks for your feedback. Implemented with regard to your feedback in #655

@d1rk d1rk closed this Sep 24, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment