EZP-31079: Added way to use login or email (or both) during authentication

[Nattfarinn]

Question	Answer
JIRA issue	EZP-31079
Bug/Improvement	no
New feature	yes
Target version	master
BC breaks	yes
Tests pass	yes
Doc needed	yes

Requires:

• EZP-31079: Replaced loadUserByCredentials witch checkUserChredentials ezplatform-admin-ui#1259
• EZP-31079: Replaced loadUserByCredentials witch checkUserChredentials ezplatform-user#60

security.yaml:

security:
    providers:
        ezplatform:
            chain:
                providers: [ezplatform_username, ezplatform_email]

        ezplatform_username:
            id: ezpublish.security.user_provider.username

        ezplatform_email:
            id: ezpublish.security.user_provider.email

TODO:

• Implement feature / fix a bug.
• Implement tests.
• Fix new code according to Coding Standards ($ composer fix-cs).
• Ask for Code Review.

[gggeek]

Q: apart from the missing documentation that would explain how this feature works in a way that mortal users can understand it, it seems to be missing some enforcing of email unicity:

1. when login-by-email is allowed, the system should enforce email unicity at account creation. When login-by-email is not allowed, this does not need to be enforced
2. when login-by-email is allowed, the user_provider.email should throw if there are 2 users with the same email. I am not 100% sure what should happen if 2 users with the same email are found when only user_provider.login is enabled, and the user_provider.email is called (and the underlying service method as well)

[Nattfarinn]

@gggeek It's under Code Review and QA right now. :)

Q: apart from the missing documentation that would explain how this feature works in a way that mortal users can understand it

I see heart emoji there, so I think @DominikaK just waits for the "Release the DocTeam" command. :)

when email-by-login is allowed, the user_provider.email should throw if there are 2 users with the same email. I am not 100% sure what should happen if 2 users with the same email are found when only user_provider.login is enabled, and the user_provider.email is called (and the underlying service method as well)

It throws exception in case of ambiguity (loadByEmail method, analogous to loadByLogin). If you have two users on the same email you can still use login (in case of both providers enabled). So I am not sure about:

when login-by-email is allowed, the system should enforce email unicity at account creation. When login-by-email is not allowed, this does not need to be enforced

I don't think existence of provider (or not) should determine about email uniqueness. We'll think about different configuration switch to turn it on or off. So you may not wan't to login by email at all but still enable "one email per user" rule.

[gggeek]

So you may not want to login by email at all but still enable "one email per user" rule. => agree. It will be nice to be able to enforce this

About the correct behaviour in case of 2 accounts sharing the same email:

in my experience, it is more common to allow either log in only by email or log in only by login, than allowing to login using both.
(a simple example of the complications one gets into when allowing login-by-both: what if I pick as my login your email ?)

It is also not uncommon to start out in a configuration where double emails are allowed, and later on the requirements change and login-by-email is enabled and login-by-login is disabled. In this scenario, what would happen to the two accounts that share the same email try to log in? Iirc in ez4 an 'optimistic' route was taken, and the 1st of the two accounts was used for authenticating. Now we have the option of implementing a better logic. I'd say that 'safer' is better, so having LoadUserByEmail throwing is ok. On the other hand, we would then have some users who are unable to log in any more... at the very least we should introduce some tools that help site admins to identify and fix those cases.

[Nattfarinn]

in my experience, it is more common to allow either log in only by email or log in only by login, than allowing to login using both.
(a simple example of the complications one gets into when allowing login-by-both: what if I pick as my login your email ?)

Login has priority over email. So if you use an email address as a login, you'll still be able to login even if you've turned off login by email value.

what would happen to the two accounts that share the same email try to log in?

You'll not be able to login in case of ambiguity. It has to be fixed (or, in case of login provider is also enabled, log in by username).

at the very least we should introduce some tools that help site admins to identify and fix those cases.

Definitely. I'm in the middle of writing simple command that will search User database for duplicates (and help you fix them).

[gggeek]

Login has priority over email. So if you use an email address as a login, you'll still be able to login even if you've turned off login by email value. => which is a kind of nice way to allow me to dos you ;-)

[Nattfarinn]

@gggeek Email uniqueness, username restrictions and helper command to audit User database will be covered by another PR.

[adamwojs]

Could you please double-check if the database index for the e-mail column is defined?

[webhdx]

@Nattfarinn What do we do with known issues? Are you going to resolve them in separate pull request as those seem to be little bit out of scope? Or we intentionally leave them for the time being but it seems to be quite risky.