RFC: AccountInterface and Authentication rework

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 and failedauthenticationcount into a separate table (potentially into a separate package even… to be discussed)
    • provide corresponding doctrine migrations for migrating structure and data
  • Revert some changes on the Account entity to make it backwards-compatible again, add deprecation annotations and implement “adapter” methods like getRoles() and getCredentialSources() as described above
  • Remove all/most setters from Account since something like setCredentialsSource() can’t work any longer as before
  • Introduce PersistedAccountRepository (as suggested replacement for to be deprecated AccountRepository) and UsernameAndPasswordRepository (to be used in Account:: getCredentialsSource() and in Username / Password providers)
3 Likes