Closes#340. Phase 1 of six under #332. Nothing a customer can see.
The nullable password hash
One line of SQL, and it is not the work. The work is that every read of customers.password_hash is now a question rather than a fact. Three places compared against it with bcrypt — login, change password, the email change — and all three now ask first, through one shared passwordMatches.
That function exists because the alternative is worse than a wrong answer. bcrypt.compare throws on a null hash rather than returning false, so any call site that forgot the check would answer a sign-in attempt with a 500 instead of a refusal. On the login route that is also an oracle: it would happen for exactly the accounts that have no password. One function rather than three null checks means the question is asked identically everywhere, and a fourth site cannot forget to ask it.
Nothing writes a null yet. The first passwordless accounts arrive with #342, which is why this lands ahead of them.
The down migration deliberately does not restore NOT NULL. Re-adding it fails outright once a passwordless customer exists, and a down migration that destroys accounts to satisfy a constraint is far worse than a column that is merely more permissive than it needs to be.
The identities table
A table rather than columns on customers, because one customer may eventually hold more than one. Columns would make a second provider a migration and a third an embarrassment.
Its important column is provider_sub, and the comment on it is the security posture of the whole feature in one place: never the email. An email is a display value its owner can change and a provider may reassign; a subject is opaque and stable for the life of the account. Matching on the email would strand a customer who changed theirs and, far worse, hand their account to whoever inherited the old address.
Unique across (provider, provider_sub), not the subject alone.
Configuration
The redirect URI derives from PUBLIC_URL, the same single source the WebAuthn relying party id uses and for the same reason: Google compares it as an exact string and answers a mismatch with a message that says nothing about which half is wrong. The tests are mostly about what must not end up in it — a trailing slash on PUBLIC_URL is an easy way to produce a URI one character off the registered one.
The config also reports whether it is enabled, so a developer without credentials gets a storefront that works and simply does not offer the button. Absent rather than disabled, the same call #41 made for a browser without WebAuthn.
Environment validation refuses to boot on one credential without the other, matching the SMTP pair. Half-configured is the case worth catching, because the failure otherwise arrives the moment a customer presses the button.
QA is set to empty on purpose
docker-compose.qa.yml sets both to empty with a long comment, so it does not read as an oversight.
Google refuses a redirect URI whose host is not under a domain whose ownership has been proved by DNS, and nobody can prove ownership of anything under bermudalamb.synology.me — Synology owns the registrable domain above it. Same wall as #285.
So QA cannot run Google sign-in at all until #313 moves it to qa.redefined-designs.com, at which point it is two stack variables and one console entry, with no code change either way. Tracked in #345.
That issue lists account deletion as confirming with the password. It does not — the route takes none, and the confirmation is a modal in the account page. Deletion needed no change here.
Testing
New unit coverage for the redirect derivation, the null-hash comparison, and the environment rules.
Verified locally: backend tsc clean for src and tests, 550 unit tests pass, lint clean apart from warnings that predate this branch. The integration suite needs a database this machine has no Docker for.
Closes #340. Phase 1 of six under #332. Nothing a customer can see.
## The nullable password hash
One line of SQL, and it is not the work. The work is that every read of `customers.password_hash` is now a question rather than a fact. Three places compared against it with bcrypt — login, change password, the email change — and all three now ask first, through one shared `passwordMatches`.
That function exists because the alternative is worse than a wrong answer. **`bcrypt.compare` throws on a null hash rather than returning false**, so any call site that forgot the check would answer a sign-in attempt with a 500 instead of a refusal. On the login route that is also an oracle: it would happen for exactly the accounts that have no password. One function rather than three null checks means the question is asked identically everywhere, and a fourth site cannot forget to ask it.
Nothing writes a null yet. The first passwordless accounts arrive with #342, which is why this lands ahead of them.
**The down migration deliberately does not restore `NOT NULL`.** Re-adding it fails outright once a passwordless customer exists, and a down migration that destroys accounts to satisfy a constraint is far worse than a column that is merely more permissive than it needs to be.
## The identities table
A table rather than columns on `customers`, because one customer may eventually hold more than one. Columns would make a second provider a migration and a third an embarrassment.
Its important column is `provider_sub`, and the comment on it is the security posture of the whole feature in one place: **never the email**. An email is a display value its owner can change and a provider may reassign; a subject is opaque and stable for the life of the account. Matching on the email would strand a customer who changed theirs and, far worse, hand their account to whoever inherited the old address.
Unique across `(provider, provider_sub)`, not the subject alone.
## Configuration
The redirect URI derives from `PUBLIC_URL`, the same single source the WebAuthn relying party id uses and for the same reason: Google compares it as an exact string and answers a mismatch with a message that says nothing about which half is wrong. The tests are mostly about what must *not* end up in it — a trailing slash on `PUBLIC_URL` is an easy way to produce a URI one character off the registered one.
The config also reports whether it is `enabled`, so a developer without credentials gets a storefront that works and simply does not offer the button. Absent rather than disabled, the same call #41 made for a browser without WebAuthn.
Environment validation refuses to boot on one credential without the other, matching the SMTP pair. Half-configured is the case worth catching, because the failure otherwise arrives the moment a customer presses the button.
## QA is set to empty on purpose
`docker-compose.qa.yml` sets both to empty with a long comment, so it does not read as an oversight.
Google refuses a redirect URI whose host is not under a domain whose ownership has been proved by DNS, and nobody can prove ownership of anything under `bermudalamb.synology.me` — Synology owns the registrable domain above it. Same wall as #285.
**So QA cannot run Google sign-in at all until #313 moves it to `qa.redefined-designs.com`**, at which point it is two stack variables and one console entry, with no code change either way. Tracked in #345.
## A correction to #332
That issue lists account deletion as confirming with the password. It does not — the route takes none, and the confirmation is a modal in the account page. Deletion needed no change here.
## Testing
New unit coverage for the redirect derivation, the null-hash comparison, and the environment rules.
Verified locally: backend `tsc` clean for src and tests, 550 unit tests pass, lint clean apart from warnings that predate this branch. The integration suite needs a database this machine has no Docker for.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Nothing a customer can see. The schema change and the configuration land on their own so the widest-reaching edit in the project can be reviewed for what it is rather than buried inside a feature.
The password hash becomes nullable. That is one line and it is not the work; the work is that every read of the column is now a question rather than a fact. Three places compared against it with bcrypt, and all three now ask first through one shared function.
That function exists because the alternative is worse than a wrong answer. bcrypt.compare throws on a null hash rather than returning false, so any call site that forgot the check would answer a sign-in attempt with a 500 instead of a refusal. On the login route that is also an oracle, because it would happen for exactly the accounts that have no password. One function rather than a null check repeated three times means the question is asked identically everywhere and a fourth site cannot forget to ask it.
Nothing writes a null yet. The first accounts without a password arrive with the sign-up path, which is why this is landed ahead of them.
The identities table is a table rather than columns on customers, because one customer may eventually hold more than one. Columns would make a second provider a migration and a third an embarrassment.
Its important column is the provider subject, and the comment on it is the whole security posture of the feature in one place: never the email. An email is a display value its owner can change and a provider may reassign; a subject is opaque and stable for the life of the account. Matching on the email would strand a customer who changed theirs and, far worse, hand their account to whoever inherited the old address. Unique across the provider and subject together, not the subject alone.
The down migration drops the table and deliberately does not restore the NOT NULL. Re-adding it fails outright once a passwordless customer exists, and a down migration that destroys accounts to satisfy a constraint is far worse than a column that is merely more permissive than it needs to be.
The redirect URI is derived from PUBLIC_URL, the same single source the WebAuthn Relying Party ID uses and for the same reason: Google compares it as an exact string and answers a mismatch with a message that says nothing about which half is wrong. Deriving it means the value is correct by construction anywhere the email links already are. The tests are mostly about what must not end up in it, since a trailing slash on PUBLIC_URL is an easy way to produce a URI that is one character from the registered one.
The config also reports whether it is enabled at all, so a developer without credentials gets a storefront that works and simply does not offer the button, rather than one that offers it and fails. Absent rather than disabled, the same choice made for a browser without WebAuthn.
Environment validation refuses to boot on one credential without the other, matching how the SMTP pair is handled. Half-configured is the case worth catching because the failure otherwise arrives at the moment a customer presses the button.
The QA compose file sets both to empty, and the comment there says why at length rather than leaving it to look like an oversight. Google refuses a redirect URI whose host is not under a domain whose ownership has been proved by DNS, and nobody can prove ownership of anything under bermudalamb.synology.me because Synology owns the registrable domain above it. That is the same wall #285 hit with Cloudflare. So QA cannot run this at all until #313 moves it to a subdomain of the real domain, at which point it is two stack variables and one console entry, with no code change either way.
Also corrects the record in #332, which lists account deletion as confirming with a password. It does not; the route takes none and the confirmation is a modal in the account page. Deletion needed no change here.
Verified: backend tsc clean for src and tests, 550 unit tests pass including new coverage of the config derivation, the null-hash comparison and the environment rules; lint clean apart from warnings that predate this branch. The integration suite needs a database this machine has no Docker for.
Closes#340
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #340. Phase 1 of six under #332. Nothing a customer can see.
The nullable password hash
One line of SQL, and it is not the work. The work is that every read of
customers.password_hashis now a question rather than a fact. Three places compared against it with bcrypt — login, change password, the email change — and all three now ask first, through one sharedpasswordMatches.That function exists because the alternative is worse than a wrong answer.
bcrypt.comparethrows on a null hash rather than returning false, so any call site that forgot the check would answer a sign-in attempt with a 500 instead of a refusal. On the login route that is also an oracle: it would happen for exactly the accounts that have no password. One function rather than three null checks means the question is asked identically everywhere, and a fourth site cannot forget to ask it.Nothing writes a null yet. The first passwordless accounts arrive with #342, which is why this lands ahead of them.
The down migration deliberately does not restore
NOT NULL. Re-adding it fails outright once a passwordless customer exists, and a down migration that destroys accounts to satisfy a constraint is far worse than a column that is merely more permissive than it needs to be.The identities table
A table rather than columns on
customers, because one customer may eventually hold more than one. Columns would make a second provider a migration and a third an embarrassment.Its important column is
provider_sub, and the comment on it is the security posture of the whole feature in one place: never the email. An email is a display value its owner can change and a provider may reassign; a subject is opaque and stable for the life of the account. Matching on the email would strand a customer who changed theirs and, far worse, hand their account to whoever inherited the old address.Unique across
(provider, provider_sub), not the subject alone.Configuration
The redirect URI derives from
PUBLIC_URL, the same single source the WebAuthn relying party id uses and for the same reason: Google compares it as an exact string and answers a mismatch with a message that says nothing about which half is wrong. The tests are mostly about what must not end up in it — a trailing slash onPUBLIC_URLis an easy way to produce a URI one character off the registered one.The config also reports whether it is
enabled, so a developer without credentials gets a storefront that works and simply does not offer the button. Absent rather than disabled, the same call #41 made for a browser without WebAuthn.Environment validation refuses to boot on one credential without the other, matching the SMTP pair. Half-configured is the case worth catching, because the failure otherwise arrives the moment a customer presses the button.
QA is set to empty on purpose
docker-compose.qa.ymlsets both to empty with a long comment, so it does not read as an oversight.Google refuses a redirect URI whose host is not under a domain whose ownership has been proved by DNS, and nobody can prove ownership of anything under
bermudalamb.synology.me— Synology owns the registrable domain above it. Same wall as #285.So QA cannot run Google sign-in at all until #313 moves it to
qa.redefined-designs.com, at which point it is two stack variables and one console entry, with no code change either way. Tracked in #345.A correction to #332
That issue lists account deletion as confirming with the password. It does not — the route takes none, and the confirmation is a modal in the account page. Deletion needed no change here.
Testing
New unit coverage for the redirect derivation, the null-hash comparison, and the environment rules.
Verified locally: backend
tscclean for src and tests, 550 unit tests pass, lint clean apart from warnings that predate this branch. The integration suite needs a database this machine has no Docker for.🤖 Generated with Claude Code