Archive/screentinker
1
0
Fork 0
mirror of https://github.com/screentinker/screentinker.git synced 2026-08-13 13:53:12 -06:00
Code Issues Actions 5 Packages Projects Releases Wiki Activity

Customer Entra tenants: verify the domain, then be believed

Browse source 
The previous fix let the instance-wide Microsoft button work and left the
customer-facing path broken, which is the worst way round. An organization that
brings its own Entra tenant would publish the TXT record, watch its domain go
green, and still be refused at login with `email_unverified` -- because Entra
sends no such claim and rowToProvider pinned the assumption off for every org
provider.

Requiring a claim Microsoft does not emit is not a security control, it is an
outage. What makes it safe to stop requiring it is the proof that already gates
these providers: the callback confines an org provider to its DNS-verified
domains, and an address only reaches the check after passing that. Whoever
controls a domain's DNS controls its mail, which is the same trust that makes a
verification link meaningful.

So the assumption is DERIVED from proof -- `verified.length > 0` -- rather than
pinned off. A provider that has verified nothing still assumes nothing, which is
belt and braces: emailAllowedForProvider already refuses it, since an empty
allow-list matches no domain, but deriving it here means a future reordering of
those checks cannot silently widen it.

It is never a column, and there is no column for it to be read from. An
organization must not be able to switch this on for itself; it is a consequence
of DNS proof, not a setting. A test asserts both -- that the value is derived
next to `source: 'org'`, and that no `assume_email_verified` exists in the
schema.

Domain confinement is untouched. An explicit `email_verified: false` is still
refused from anyone.

Mutations all fail the tests: assuming unconditionally, never assuming, and
reading it from the row.
This commit is contained in:
ScreenTinker 2026-08-11 23:05:25 -05:00
parent 77d41ae73e
commit e5e5b75b85
3 changed files with 75 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
README.md
View file

@ -425,6 +425,13 @@ https://yourdomain.com/api/auth/oidc/<generated-slug>/callback
The slug is generated rather than chosen so two customers cannot collide on — or guess — each
other's. A domain may be claimed by only one organization; a second claim is refused.
A customer bringing **Microsoft/Entra** registers a single-tenant application in their own directory
and uses `https://login.microsoftonline.com/<their-tenant-guid>/v2.0` as the issuer. Because Entra
does not send `email_verified`, an organization's provider is trusted to assert addresses **once it
has verified a domain** — the DNS proof is what stands in for the claim, and the provider is confined
to those domains regardless. A provider that has verified nothing assumes nothing, and an explicit
`email_verified: false` is refused whoever sends it.
⚠️ **A provider may only authenticate emails inside the domains it has VERIFIED.** An organization
supplies its own issuer and client ID, so it controls that identity provider completely and could
otherwise assert any address at all — including another company's, or an administrator's. Confining

42
server/lib/oidc-providers.js
View file

@ -74,11 +74,18 @@ function fromEnv(env, slug) {
* already exempts it from domain confinement — and the Microsoft entry is additionally pinned to one
* tenant GUID, so only that directory can issue tokens for it.
*
* Two limits keep this from becoming the hole the strict check was closing:
* An organization's own provider may assume too, but only once it has DNS-verified a domain — the
* callback confines it to those domains, so it can only ever speak for names it proved it controls.
* Requiring the claim from it as well meant a customer's Entra tenant went green on domain
* verification and then failed the login anyway.
*
* Three limits keep this from becoming the hole the strict check was closing:
* - an EXPLICIT `email_verified: false` is always refused. Assuming only ever covers an omitted
* claim, never a provider actively saying the address is unverified;
* - org-configured providers can never set it (rowToProvider pins it false, and nothing reads it
* from the database), so the account-takeover path stays shut.
* - for an org provider it is DERIVED from proof (`verified.length > 0`) and is never a column, so
* a customer cannot switch it on for themselves;
* - domain confinement is unchanged and still runs first, so an org provider that assumes still
* cannot assert an address outside a domain it has proven.
*/
function emailIsVerified(claims, provider) {
const asserted = (claims || {}).email_verified;
@ -211,6 +218,9 @@ function db() {
}
function rowToProvider(row, secretbox) {
// Resolved once: it decides both which addresses this provider may assert and, below, whether it
// has proven anything at all.
const verified = verifiedDomainsFor(row.id);
return {
slug: row.slug,
name: row.name,
@ -226,14 +236,26 @@ function rowToProvider(row, secretbox) {
: null,
scopes: row.scopes || DEFAULT_SCOPES,
/*
* ⚠️ NEVER settable for an organization's provider, and deliberately not read from the row.
* DERIVED from proof, never read from a column.
*
* A customer chooses this provider, so it speaks for the party it vouches for. Letting an org
* assume verification would hand back exactly the takeover primitive the strict check exists to
* stop. Domain confinement narrows WHICH addresses it may assert; this keeps the assertion
* itself honest.
* Entra ID v2 omits email_verified, so demanding it refused every customer who brought their own
* Microsoft tenant — the domain went green and the login still failed. Requiring a claim
* Microsoft does not send is not a security control, it is an outage.
*
* What makes it safe to stop requiring it is the proof that already gates this provider: the
* callback confines it to DNS-verified domains, and an address is only reached here after
* passing that. Whoever controls a domain's DNS controls its mail, which is the same trust that
* makes a verification link meaningful in the first place.
*
* So the assumption is tied to having proven SOMETHING. A provider with no verified domain
* assumes nothing — belt and braces, because emailAllowedForProvider already refuses it (an
* empty allow-list matches no domain), and this way a future refactor that reorders those checks
* cannot silently widen it.
*
* ⚠️ Still never a column. An org must not be able to switch this on for itself; it is a
* consequence of DNS proof, not a setting.
*/
assumeEmailVerified: false,
assumeEmailVerified: verified.length > 0,
source: 'org',
organizationId: row.organization_id,
/*
@ -244,7 +266,7 @@ function rowToProvider(row, secretbox) {
* Reading the typed column here would reduce the whole verification feature to a decoration:
* a tenant could type any company's domain and immediately assert addresses in it.
*/
emailDomains: verifiedDomainsFor(row.id).join(','),
emailDomains: verified.join(','),
};
}

44
server/test/oidc-email-verified-policy.test.js
View file

@ -10,9 +10,13 @@
*
* The rule these tests pin down:
* - `email_verified: true` -> believed, always
* - claim ABSENT + operator-chosen -> believed (Microsoft, or an opted-in generic provider)
* - claim ABSENT + org-configured -> refused
* - claim ABSENT + operator-chosen -> believed (Microsoft, or an opted-in provider)
* - claim ABSENT + org with a VERIFIED domain -> believed (it proved DNS control)
* - claim ABSENT + org with NO verified domain -> refused (it has proven nothing)
* - `email_verified: false` -> refused, whoever asked
*
* The org case is a consequence of DNS proof, never a setting: an organization must not be able to
* turn it on for itself, so it is derived and never read from a column.
*/
const { test } = require('node:test');
@ -22,10 +26,18 @@ const { emailIsVerified, list } = require('../lib/oidc-providers');
const MS_ENV = { MICROSOFT_CLIENT_ID: 'client-abc', MICROSOFT_TENANT_ID: 'ffffffff-1111-2222-3333-444444444444' };
const microsoft = () => list(MS_ENV).find((p) => p.slug === 'microsoft');
const google = () => list({ GOOGLE_CLIENT_ID: 'g-abc' }).find((p) => p.slug === 'google');
const orgProvider = { slug: 'acme7f3', source: 'org', organizationId: 'org-1', assumeEmailVerified: false };
// An org provider as rowToProvider builds it: `assumeEmailVerified` follows from whether any domain
// has actually been DNS-verified.
const orgProvider = (verifiedDomains = []) => ({
slug: 'acme7f3', source: 'org', organizationId: 'org-1',
emailDomains: verifiedDomains.join(','),
assumeEmailVerified: verifiedDomains.length > 0,
});
const orgUnproven = orgProvider(); // configured, nothing verified yet
const orgProven = orgProvider(['bytetinker.net']); // TXT published, domain green
test('an explicit true is believed from any provider', () => {
for (const p of [microsoft(), google(), orgProvider]) {
for (const p of [microsoft(), google(), orgUnproven, orgProven]) {
assert.equal(emailIsVerified({ email_verified: true }, p), true, `${p.slug} should accept an explicit true`);
}
});
@ -42,14 +54,26 @@ test('Google stays strict — it does send the claim, so there is nothing to ass
assert.equal(emailIsVerified({ email: 'someone@example.com' }, g), false);
});
test('an ORG-configured provider may never assume, even if the object claims it can', () => {
assert.equal(emailIsVerified({ email: 'a@b.c' }, orgProvider), false);
// Belt and braces: a tampered/hand-built org object must not be able to opt itself in through
// the database, which is why rowToProvider pins the field rather than reading a column.
test('an org provider that has proven a domain may assume — the customer-Entra case', () => {
// Entra sends no email_verified. Before this, the domain went green and the login still failed.
assert.equal(emailIsVerified({ email: 'dan@bytetinker.net' }, orgProven), true);
});
test('an org provider that has proven NOTHING assumes nothing', () => {
assert.equal(emailIsVerified({ email: 'dan@bytetinker.net' }, orgUnproven), false);
});
test('the org assumption is DERIVED from proof, never readable from the row', () => {
const src = require('fs').readFileSync(require.resolve('../lib/oidc-providers'), 'utf8');
assert.match(src, /assumeEmailVerified: false,\s*\n\s*source: 'org'/,
'rowToProvider must hard-code assumeEmailVerified:false next to source:org');
assert.match(src, /assumeEmailVerified: verified\.length > 0,\s*\n\s*source: 'org'/,
'rowToProvider must derive it from the verified-domain list, next to source:org');
// The whole point: an organization must not be able to switch this on for itself.
assert.doesNotMatch(src, /assumeEmailVerified: *row\./, 'must never be read from the org row');
// ...and there must be no column for it to be read FROM. Checked against the schema rather than
// this file, where `OIDC_<SLUG>_ASSUME_EMAIL_VERIFIED` is a legitimate operator-set env var.
const schema = require('fs').readFileSync(require.resolve('../db/database'), 'utf8');
assert.doesNotMatch(schema, /assume_email_verified/i,
'org_sso_providers must have no assume_email_verified column');
});
test('an EXPLICIT false is refused even where absence would be assumed', () => {