Primary/Notification email address implementation plan

[blizzz]

This is part of #26866

My original intent was

For this work it is reasonable to take Different mail for notifications into account which has to be implemented for autmn this year and is based on this feature.

Without this we have the mail address that might be provided by the user backend and might be immutable. Also we have the batch of additional email addresses. In order to have another special email address that will be used primarily an additional user setting is suggested: primaryEmail. By default (i.e. not set) it the main email is being used. The user may pick a different email from the verified, additional ones. The selected address will be then set es primaryEmail.

In order to be transparent for all other clients there will be no additional API methods, but the implementation of IUser::getEMailAddress() will be changed to return the primaryAddress. Accordingly IUser::setEMailAddress() will try to update email on the user backend, if possible. If the primaryAddress is set or if setting the backend's email did not succeed, the primaryAddress preference is being set.

If the necessity persists to be able to fetch the backend's email address, the IUser API will need to be extended. This is to ensure that the primary address will be effective with any app, supported and third party.

And I thought this is a quick win, but it turns out this has more side effects than originally anticipated. My drafted idea now:

getEmailAddress

is fairly easy

setEmailAddress

is partly surprising and more complex

here it has to be said that the name allow_user_to_change_display_name is misleading as it controls whether a user may or may not set their email address in the UI. This is enforced in the controllers only.

Another aspect is that at the moment an (old style) ::changeUser event is triggered, when the email address has changed. This should continue to run, but only for when the system mail has changed.

If there is interest (as in: a use case) we can introduce an event for the primaryMail change, too.

more methods

IUser should be equipped with methods to specifically fetch the system or primary mail when a clear value is necessary. For instance in the user:info occ command, which should right away make use of it.

The users management page may show the primary address as well, I would consider it not critical there.

Addendum

This requires implementation of local mail verification which is still on the todo list.

Opinions?

@Pytal @skjnldsv @nickvergessen @ChristophWurst @icewind1991

[nickvergessen]

So "primary" is user selected, "system" is LDAP source or not changable because of config.
The question is if there are usecases where we would need to get a specific one, or even both or something.
I didn't look at the root issue to see if that is possible in some way, but for now the suggestion seems to work.

[ChristophWurst]

IUser should be equipped with methods to specifically fetch the system or primary mail when a clear value is necessary

In Mail we use the user's email address as template argument for %EMAIL% when email addresses and usernames are built for provisioned accounts. For those features the system mail doesn't make any sense. Ref https://github.com/nextcloud/mail/blob/574f1f010106cb850c6a4f0eeff3a0a815593a10/lib/Db/Provisioning.php#L142. Hence a dedicated method that only gives you the user's email address or null would be preferred.

For the account form we get the email address like this: https://github.com/nextcloud/mail/blob/574f1f010106cb850c6a4f0eeff3a0a815593a10/lib/Controller/PageController.php#L168 -> what value can we expect there int the future? Should such code also use getEMailAddress instead?

[blizzz]

So "primary" is user selected, "system" is LDAP source or not changable because of config.

Right, I skipped explaining terminology for shagginess.

"Primary email" is the user-chosen email address, that should be used for notifications and things.

The "system email" is that one set the backend or administrator (scenario for local users is when allow_user_to_change_display_name is set to false). For local users, when this can be changed (default), we'd just update this. In the other cases, the primary mail is being used for a coexistence, as the system mail might not be simply changed.

[blizzz]

In Mail we use the user's email address as template argument for %EMAIL% when email addresses and usernames are built for provisioned accounts. For those features the system mail doesn't make any sense. Ref https://github.com/nextcloud/mail/blob/574f1f010106cb850c6a4f0eeff3a0a815593a10/lib/Db/Provisioning.php#L142. Hence a dedicated method that only gives you the user's email address or null would be preferred.

Right now what you are getting is the system mail (the only mail we have as speak).

For provisioned accounts imo the only useful value to use is the mail set by admin/backend i.e. system mail. So my suggestion would be to switch for the specific system mail getter.

For the account form we get the email address like this: https://github.com/nextcloud/mail/blob/574f1f010106cb850c6a4f0eeff3a0a815593a10/lib/Controller/PageController.php#L168 -> what value can we expect there int the future? Should such code also use getEMailAddress instead?

What you're using there is the current implementation of IUser::getEMailAddress(). Imo it makes sense to switch to this method, yes.

[blizzz]

here it has to be said that the name allow_user_to_change_display_name is misleading as it controls whether a user may or may not set their email address in the UI. This is enforced in the controllers only.

It appears I neglected the fact that this method is being used by other user backends as well to actually set the mail. In this case the user backend has the authority and shall set or overwrite the system email indeed.

For APIs and apps not to break, the current situation must continue to work, so the allow_user_to_change_display_name must not happen within IUser. It seems to be safer to adjust the User provisioning API's usage to follow the flow described above instead (it still can be hidden in an extra method in the IUser implementation to keep the logic at the right place).

[blizzz]

While implementing, I simplified the setEmailAddress to be an alias to the set the system email address (best backwards compatibility). For the reasons stated above ^

Only the user should be able to set the primary/notification email, hence i adjusted the API Controller to do exactly this.