No email address is returned upon first login

[nielsvrbk]

When a user logs in for the first time, using the OpenID Connector, an account without an email address is created in the database. When the user logs in again, another account is created also without an email address.
When an email address is added to one of the accounts, the user is able to reuse the account that has an email address assigned to it.

We are provisioning accounts from Azure.

[LeShaunJ]

Can confirm. Same issue at our company.

[nielsvrbk]

For completeness I should have added some version information...

ownCloud 10.15.0 (stable)
Ontwikkeld door de ownCloud gemeenschap, de source code is gelicenseerd onder de AGPL.

[APP] [developer] [update info]
[OpenID Connect] [ownCloud] 2.3.0 -> 2.3.1

[LeShaunJ]

We've confirmed the root cause is:

openidconnect/lib/Service/AutoProvisioningService.php

Line 166 in 0ad9161

if ($this->client->mode() !== 'email') {

This results in email persistence being skipped 100% of the time if client mode is 'email'.

---

Solution seems be that you'd offset the condition with $force, or remove it entirely:

	if ($force || $this->client->mode() !== 'email') {
		if ($force || $user->canChangeMailAddress()) {
			$currentEmail = $this->client->getUserEmail($userInfo);
			if ($currentEmail && $currentEmail !== $user->getEMailAddress()) {
				$this->logger->debug('AutoProvisioningService: setting e-mail to ' . $currentEmail);
				$user->setEMailAddress($currentEmail);
			}
		}
	}

To be honest, I'm not sure what that condition is meant to prevent. If it's meant to prevent changing an email of a user that already exists (in email mode), this only ever seems to be called when a user:

• Was not found via their email, or
• Was found via email—the same email that would prompt skipping the email update because it is the same email.

It seems as though this was meant to avoid unnecessary steps. So I think my suggestion is likely what the original author probably meant to do in the first place.

[frankuit]

@DeepDiver1975 Hi Thomas, can you please elaborate on why this is beeing closed ? does it need more upvoting ?

[DeepDiver1975]

It needs a pull request. 🤷

[LeShaunJ]

It needs a pull request. 🤷

Can you please elaborate? Typically, an issue is brought forth, discussed, and may ultimately result in a PR; but, closing an issue on the basis that it requires an accompanying PR...? I'm just a little taken aback by that... Is there a CONTRIBUTING.md you can point me to so that perhaps I can open a new issue correctly this time?

[DeepDiver1975]

We need no ticket. Try to fix it and open a pull request. Anybody can do that.

Issue have been closed from an ownCloud project perspective: ownCloud itself will not invest into this. That's why I closed them.

[LeShaunJ]

Done. Please review #333 and release 2.3.3, thanks.