Issue/94 - Multiple Authenticators #95
Conversation
bubba1e
changed the title
|
Definitely a nice addition, but I think we need some tests to cover this change |
|
The tests now cover multiple authenticators. |
|
|
||
| $_SERVER['PHP_AUTH_USER'] = '[email protected]'; | ||
|
|
||
| $response = Director::test($url, $data, null, 'PUT'); |
There was a problem hiding this comment.
We shouldn't shoe-horn a fairly fundamental authentication test into another unrelated test.
Please can you create a new test function like testApiAccess which adds additional authenticators and then asserts that the second authenticator works even if the first fails?
| Director::config()->set('alternate_base_url', $this->baseURI); | ||
| RestfulServer::config()->set('authenticators', [ | ||
| BasicRestfulAuthenticator::class, | ||
| EmailOnlyAuthenticator::class, |
There was a problem hiding this comment.
We shouldn't add this authenticator to all tests if they don't need/want it as it may result in unintended side-affects in future tests. Just add it to the specific test for this situation.
| } | ||
|
|
||
| /** @var null|Member $member */ | ||
| $member = Member::get()->find('Email', $_SERVER['PHP_AUTH_USER']); |
There was a problem hiding this comment.
I don't think this is a very good way to perform the test because we could see tests passing that shouldn't if (for example) the first authenticator has a bug that means passwords are not validated properly.
You could either create an actual mock instance of an authenticator (assuming there's an interface to mock) or create some kind of hard-coded token authenticator (eg: if AUTH_USER === 'SOME-TOKEN' return a user).
Ideally this user will be a new user added to the fixtures too.
|
Thanks for the work on this @nykkl - but we need to make the test a bit more specific / thorough I think. |
|
I also feel this should target the |
3 participants
Reference Issues
Multiple Authenticators
Changes
$authenticatorvariable was changed to an array and renamed to$authenticatorsto allow multiple authenticators