Make Entra prove the address, not just the directory

Reported by Dickson Massawe as GHSA-2rfh-v3j2-2jg7.

Pinning the tenant was half an answer. It defeats the classic
cross-tenant nOAuth, where a stranger's own directory asserts your
address, because a foreign tenant carries a different tid. It does
nothing about the same attack from inside the pinned tenant: Entra's
email claim is user-mutable — a B2B guest's otherMails among its sources
— so a colleague or an invited guest could present an administrator's
address and have their subject bound to that account.

Tenant-pinning answers "which directory said this". It never answered
"does this person own that address". xms_edov is Microsoft's own answer
to the second, and their guidance says to require it wherever email
identifies an account. Absent counts as unverified, which is the only
safe reading given it is absent by default.

Nobody is locked out by this, which is worth saying because it looked
like a breaking change until I read SocialAuthenticator::resolve in
order. An account already linked resolves by subject at step 3, before
trust is consulted at all — those keep working untouched. A first-time
link to an existing account is refused with the message that already
exists for exactly this case, which names the way through: sign in with
your password and connect the provider from your settings. A brand-new
account is still created; it goes to the approval queue rather than
auto-approving.

The settings screen and docs/testing-social-login.md now tell an
operator to add the claim, and there is an upgrade note.

The tests exercise fromSocialite() on raw claims, which nothing did
before: tests/Feature/Auth/SocialLoginTest.php builds a SocialIdentity by
hand and so never reaches this mapping. That is how the branch could
trust a tenant match alone with a full suite passing.
This commit is contained in:
ignacionelson
2026-09-08 19:00:46 -03:00
parent 3244be6bac
commit ab5fa2da8b
4 changed files with 164 additions and 2 deletions
+20
View File
@@ -15,6 +15,17 @@ when a version is cut.
**Fixed**
- **Microsoft sign-in now needs one more claim before it will trust an address.** Naming your tenant
says which directory vouched for a sign-in; it never said the person owns the address they
presented. Inside your own tenant a member or an invited guest could present a colleague's — an
administrator's — and have their Microsoft account attached to it. ProjectSend now also requires
the `xms_edov` claim, which is Microsoft's own answer to that question.
*Who this affected:* only installations with Microsoft sign-in enabled. **There is something to do
— see the upgrade notes.** Nobody is locked out: accounts already linked to Microsoft keep working
untouched, because they are matched on the account itself rather than on the address.
Reported by Dickson Massawe.
- **Changing your own email address now asks for your password.** It did not, and that address is
where a password reset is sent — so anybody who got hold of a signed-in session could point the
account at their own inbox, request a reset, and keep the account for good. Deleting your account
@@ -48,6 +59,15 @@ when a version is cut.
or `1` — including `no`, `off`, and a misspelling — used to switch the CAPTCHA off on the login
and registration forms. Only an explicit `true` or `1` does now; everything else leaves it on.
### Upgrade notes
- **If you use Microsoft sign-in, add the `xms_edov` optional claim to your app registration.** In
the Entra portal: your app registration → Token configuration → Add optional claim → ID →
`xms_edov`. Until you do, Microsoft sign-in keeps working and keeps creating new accounts, but it
will no longer attach itself to an account that already exists — the person is told to sign in
with their password and connect Microsoft from their settings instead. People already signed in
with Microsoft are not affected.
## 2.4.0 — 8 September 2026
+35 -2
View File
@@ -39,8 +39,40 @@ final readonly class SocialIdentity
* | LinkedIn | `email_verified` claim |
* | OpenID Connect | `email_verified` claim, from the ID token |
* | GitHub | An address at all Socialite's GithubProvider replaces `email` with the result of `getEmailByToken()`, which only ever returns one that is **primary and verified** |
* | Microsoft | The token's `tid` matching the configured tenant. Entra does not emit a usable `email_verified`, and its `email` claim is user-mutable pinning the tenant is what makes it mean anything (this is the *nOAuth* class of bug) |
* | Microsoft | The token's `tid` matching the configured tenant **and** `xms_edov` see below |
* | Facebook | Nothing. The Graph API has no equivalent claim, so an address from Facebook is never treated as verified |
*
* ### Microsoft takes two claims, not one
*
* Entra emits no usable `email_verified`, and its `email` claim is
* user-mutable populated from `otherMails`/proxyAddresses for a B2B
* guest, among other places. Pinning the tenant was the first answer
* and it is half of one: it defeats the classic cross-tenant *nOAuth*,
* where a stranger's own tenant asserts your address, because a
* foreign tenant carries a different `tid`.
*
* It does nothing about the same attack from *inside* the pinned
* tenant. A colleague, or a guest somebody invited, could shape their
* `email` claim to an administrator's address and have their subject
* bound to that account (GHSA-2rfh-v3j2-2jg7). Tenant-pinning answers
* "which directory said this", never "does this person own that
* address".
*
* `xms_edov` is Microsoft's own answer to the second question the
* optional claim meaning the tenant has verified it owns the email's
* domain and their guidance says to require it wherever `email`
* identifies an account. Absent is treated as unverified, which is the
* only safe reading: it is absent by default, so anything else would
* be no check at all.
*
* **What an installation has to do.** The claim must be added to the
* app registration (Token configuration optional claims `xms_edov`
* on the ID token). Until it is, Microsoft sign-in still works and
* still creates new accounts it simply stops silently attaching
* itself to accounts that already exist, and says so, pointing the
* person at signing in with a password and connecting the provider
* from their settings. Accounts already linked are unaffected: they
* resolve by subject, before this is consulted at all.
*/
public static function fromSocialite(
SocialProvider $provider,
@@ -72,7 +104,8 @@ final readonly class SocialIdentity
|| ($raw['email_verified'] ?? null) === 'true',
SocialProvider::Github => true,
SocialProvider::Microsoft => is_string($settings->tenant_id)
&& ($raw['tid'] ?? null) === $settings->tenant_id,
&& ($raw['tid'] ?? null) === $settings->tenant_id
&& (($raw['xms_edov'] ?? null) === true || ($raw['xms_edov'] ?? null) === 'true'),
SocialProvider::Facebook => false,
},
name: is_string($user->getName()) && trim($user->getName()) !== ''
@@ -197,6 +197,11 @@ function ProviderCard({ provider, open, onToggle }: { provider: ProviderSettings
'Your own tenant, not "common". Microsoft lets a user change the email address on their account, so a sign-in is only trustworthy when the token came from the tenant you named here.',
)}
</p>
<p className="text-muted-foreground text-sm">
{t(
'Also add the "xms_edov" optional claim to your app registration, under Token configuration. Naming the tenant says which directory vouched for the sign-in; that claim says the directory checked the person really owns the address. Without it, someone else inside your tenant could sign in with a colleague\'s address, so ProjectSend will create new accounts but never attach a Microsoft sign-in to an account that already exists.',
)}
</p>
<InputError message={form.errors.tenant_id} />
</div>
)}
@@ -0,0 +1,104 @@
<?php
declare(strict_types=1);
use App\Modules\Identity\Social\SocialIdentity;
use App\Modules\Identity\Social\SocialProvider;
use App\Modules\Identity\Social\SocialSettings;
use Laravel\Socialite\Two\User as SocialiteUser;
/**
* What each provider's word on an address is worth.
*
* `emailVerified` is what licenses binding a new identity to an account
* that already exists, which is where v1 was taken over. Everything
* upstream of it is a claim somebody else wrote.
*
* This exercises fromSocialite() directly, on raw claims. The tests in
* tests/Feature/Auth/SocialLoginTest.php build a SocialIdentity by hand
* and so never reach this mapping which is why the Microsoft branch
* could trust a tenant match alone with a full suite passing
* (GHSA-2rfh-v3j2-2jg7).
*/
const TENANT = 'e1f2a3b4-0000-0000-0000-abcdefabcdef';
/** @param array<string, mixed> $claims */
function microsoftIdentity(array $claims, ?string $tenantId = TENANT): ?SocialIdentity
{
$user = new SocialiteUser;
$user->map(['id' => 'attacker-subject', 'email' => $claims['email'] ?? null, 'name' => 'Someone']);
$user->setRaw($claims);
$settings = new SocialSettings(['provider' => SocialProvider::Microsoft->value, 'tenant_id' => $tenantId]);
return SocialIdentity::fromSocialite(SocialProvider::Microsoft, $user, $settings);
}
/*
|--------------------------------------------------------------------------
| Entra: a tenant match is not a verified address
|--------------------------------------------------------------------------
|
| Pinning the tenant defeats the classic cross-tenant nOAuth, and that is
| all it defeats. Inside the tenant, `email` is user-mutable and can be
| populated from unverified sources a B2B guest's otherMails among them
| so a colleague or an invited guest could present the victim's address.
| Microsoft's own guidance names the control: xms_edov.
*/
test('a tenant match alone is not enough', function () {
$identity = microsoftIdentity([
'tid' => TENANT,
'email' => 'admin@example.test',
]);
expect($identity?->emailVerified)->toBeFalse();
});
test('the domain-owner claim is what makes it verified', function () {
$identity = microsoftIdentity([
'tid' => TENANT,
'email' => 'admin@example.test',
'xms_edov' => true,
]);
expect($identity?->emailVerified)->toBeTrue();
});
test('the claim is read when Entra sends it as a string', function () {
// Optional claims have been observed arriving as strings rather than
// JSON booleans, and the branch beside this one already allows for the
// same on email_verified.
expect(microsoftIdentity(['tid' => TENANT, 'email' => 'a@example.test', 'xms_edov' => 'true'])?->emailVerified)
->toBeTrue();
});
test('a false or junk claim is not verified', function (mixed $value) {
expect(microsoftIdentity(['tid' => TENANT, 'email' => 'a@example.test', 'xms_edov' => $value])?->emailVerified)
->toBeFalse();
})->with([
'false' => [false],
'"false"' => ['false'],
'null' => [null],
'empty' => [''],
'a typo' => ['ture'],
'zero' => [0],
]);
test('the domain-owner claim alone, from the wrong tenant, is not enough', function () {
// Both halves. xms_edov says the tenant owns the domain; the tid says
// it is *our* tenant. A foreign tenant that owns its own domain is
// still a stranger.
$identity = microsoftIdentity([
'tid' => 'some-other-tenant',
'email' => 'admin@example.test',
'xms_edov' => true,
]);
expect($identity?->emailVerified)->toBeFalse();
});
test('an installation with no tenant pinned verifies nothing', function () {
expect(microsoftIdentity(['tid' => TENANT, 'email' => 'a@example.test', 'xms_edov' => true], tenantId: null)?->emailVerified)
->toBeFalse();
});