I just had a great talk with @sorenmalling since we plan to work on this in the upcoming days and weeks.
I still think that the approach discussed above would work out and provide a much cleaner foundation for future extensions (like the account impersonation @markusguenther mentioned).
There are (at least) two challenges we currently face though:
1. Account::getRoles()
currently returns an array of Role
objects including their parent roles.
When creating account instances, this is cumbersome, error prone and “expensive” because you’ll always have to use the PolicyService
to create the Role
instances:
$roles = array_map([$this->policyService, 'getRole'], $roleIds);
Changing that to only return a flat array of role ids would be a hell of a breaking change though.
We decided we would rename the getter in the new interface instead to:
interface AccountInterface
{
//...
public function getRoleIdentifiers(): RoleIdentifiers;
}
The Account
implementation could then do the conversion lazily like:
class Account implements AccountInterface {
public function getRoles(): array
{
return array_map([$this->policyService, 'getRole'], iterator_to_array($this->getRoleIdentifiers()));
}
}
2. We’ll lose information about the authentication provider
Account::getAuthenticationProviderName()
would no longer be able to work reliably since we’d only persist the Realm
as described above.
I’d say that we could just do
/* @deprecated use PersistedAccount or AccountInterface */
class Account {
/**
* @deprecated bla bla, this will return the Realm, use getRealm() instead
*/
public function getAuthenticationProviderName(): string
{
return (string)$this->getRealm();
}
}
Next steps
- Rebase the existing PR to current master
- Introduce the new interfaces and classes as described above
- Split
neos_flow_security_account
table:- rename
authenticationprovidername
=>realm
- extract
credentialssource
to new table (e.g.neos_flow_security_usernameandpassword
as suggested above – note: these are credentials but specific ones for username & password authentication, so that name makes sense to me) - extract
lastsuccessfulauthenticationdate
andfailedauthenticationcount
into a separate table (potentially into a separate package even… to be discussed) - provide corresponding doctrine migrations for migrating structure and data
- rename
- Revert some changes on the
Account
entity to make it backwards-compatible again, add deprecation annotations and implement “adapter” methods likegetRoles()
andgetCredentialSources()
as described above - Remove all/most setters from
Account
since something likesetCredentialsSource()
can’t work any longer as before - Introduce
PersistedAccountRepository
(as suggested replacement for to be deprecatedAccountRepository
) andUsernameAndPasswordRepository
(to be used inAccount:: getCredentialsSource()
and in Username / Password providers)