diff --git a/src/backend/clients/database/SqliteDatabaseClient.test.ts b/src/backend/clients/database/SqliteDatabaseClient.test.ts index 95493c67c..ce35921d0 100644 --- a/src/backend/clients/database/SqliteDatabaseClient.test.ts +++ b/src/backend/clients/database/SqliteDatabaseClient.test.ts @@ -27,7 +27,7 @@ import { DatabaseClientFactory } from './index.js'; import { SqliteDatabaseClient } from './SqliteDatabaseClient.js'; /** Highest schema version the migration table can reach. */ -const CURRENT_SCHEMA_VERSION = 86; +const CURRENT_SCHEMA_VERSION = 87; /** * These suites migrate real files on disk. Idle they finish in well under a diff --git a/src/backend/clients/database/SqliteDatabaseClient.ts b/src/backend/clients/database/SqliteDatabaseClient.ts index 7cd22b5ba..4bae677d3 100644 --- a/src/backend/clients/database/SqliteDatabaseClient.ts +++ b/src/backend/clients/database/SqliteDatabaseClient.ts @@ -120,6 +120,7 @@ const AVAILABLE_MIGRATIONS: [number, string[]][] = [ [83, ['0088_apps-index-url.sql']], [84, ['0089_team-require-2fa.sql']], [85, ['0090_user-app-permission-index.sql']], + [86, ['0091_icloud-dot-significance.sql']], ]; export class SqliteDatabaseClient extends AbstractDatabaseClient { diff --git a/src/backend/clients/database/migrations/mysql/mysql_mig_46.sql b/src/backend/clients/database/migrations/mysql/mysql_mig_46.sql new file mode 100644 index 000000000..f425a1994 --- /dev/null +++ b/src/backend/clients/database/migrations/mysql/mysql_mig_46.sql @@ -0,0 +1,49 @@ +-- Copyright (C) 2024-present Puter Technologies Inc. +-- +-- This file is part of Puter. +-- +-- Puter is free software: you can redistribute it and/or modify +-- it under the terms of the GNU Affero General Public License as published +-- by the Free Software Foundation, either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU Affero General Public License for more details. +-- +-- You should have received a copy of the GNU Affero General Public License +-- along with this program. If not, see . + +-- Recompute `clean_email` for Apple's domains: dots are significant there, so +-- rows written while they were stripped still collapse two mailboxes into one. +-- Only rows still holding exactly what the old rule produced, so a NULL or a +-- legacy un-lowercased value is left alone -- rewriting those would fold `+tag` +-- or case into a key another row already owns. Restores dots and nothing else. + +UPDATE `user` +SET `clean_email` = CONCAT(SUBSTRING_INDEX(SUBSTRING_INDEX(LOWER(`email`), '@', 1), '+', 1), '@', SUBSTRING_INDEX(LOWER(`email`), '@', -1)) +WHERE `email` IS NOT NULL + AND LOCATE('@', `email`) > 1 + AND ( + LOWER(`email`) LIKE '%@icloud.com' + OR LOWER(`email`) LIKE '%@me.com' + OR LOWER(`email`) LIKE '%@mac.com' + ) + AND `clean_email` = CONCAT(REPLACE(SUBSTRING_INDEX(SUBSTRING_INDEX(LOWER(`email`), '@', 1), '+', 1), '.', ''), '@', SUBSTRING_INDEX(LOWER(`email`), '@', -1)) + AND `clean_email` <> CONCAT(SUBSTRING_INDEX(SUBSTRING_INDEX(LOWER(`email`), '@', 1), '+', 1), '@', SUBSTRING_INDEX(LOWER(`email`), '@', -1)); + +-- The same collapse decided who a pending invite belongs to, so the holder +-- of the undotted address claims invites addressed to the dotted one. The +-- typed address rides along as `invitedAddress` whenever it differed. +UPDATE `share` +SET `recipient_email` = CONCAT(SUBSTRING_INDEX(SUBSTRING_INDEX(LOWER(JSON_UNQUOTE(JSON_EXTRACT(`data`, '$.invitedAddress'))), '@', 1), '+', 1), '@', SUBSTRING_INDEX(LOWER(JSON_UNQUOTE(JSON_EXTRACT(`data`, '$.invitedAddress'))), '@', -1)) +WHERE JSON_UNQUOTE(JSON_EXTRACT(`data`, '$.invitedAddress')) IS NOT NULL + AND LOCATE('@', JSON_UNQUOTE(JSON_EXTRACT(`data`, '$.invitedAddress'))) > 1 + AND ( + LOWER(`recipient_email`) LIKE '%@icloud.com' + OR LOWER(`recipient_email`) LIKE '%@me.com' + OR LOWER(`recipient_email`) LIKE '%@mac.com' + ) + AND `recipient_email` = CONCAT(REPLACE(SUBSTRING_INDEX(SUBSTRING_INDEX(LOWER(JSON_UNQUOTE(JSON_EXTRACT(`data`, '$.invitedAddress'))), '@', 1), '+', 1), '.', ''), '@', SUBSTRING_INDEX(LOWER(JSON_UNQUOTE(JSON_EXTRACT(`data`, '$.invitedAddress'))), '@', -1)) + AND `recipient_email` <> CONCAT(SUBSTRING_INDEX(SUBSTRING_INDEX(LOWER(JSON_UNQUOTE(JSON_EXTRACT(`data`, '$.invitedAddress'))), '@', 1), '+', 1), '@', SUBSTRING_INDEX(LOWER(JSON_UNQUOTE(JSON_EXTRACT(`data`, '$.invitedAddress'))), '@', -1)); diff --git a/src/backend/clients/database/migrations/postgres/postgres_mig_35.sql b/src/backend/clients/database/migrations/postgres/postgres_mig_35.sql new file mode 100644 index 000000000..876bb26e6 --- /dev/null +++ b/src/backend/clients/database/migrations/postgres/postgres_mig_35.sql @@ -0,0 +1,49 @@ +-- Copyright (C) 2024-present Puter Technologies Inc. +-- +-- This file is part of Puter. +-- +-- Puter is free software: you can redistribute it and/or modify +-- it under the terms of the GNU Affero General Public License as published +-- by the Free Software Foundation, either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU Affero General Public License for more details. +-- +-- You should have received a copy of the GNU Affero General Public License +-- along with this program. If not, see . + +-- Recompute `clean_email` for Apple's domains: dots are significant there, so +-- rows written while they were stripped still collapse two mailboxes into one. +-- Only rows still holding exactly what the old rule produced, so a NULL or a +-- legacy un-lowercased value is left alone -- rewriting those would fold `+tag` +-- or case into a key another row already owns. Restores dots and nothing else. + +UPDATE "user" +SET clean_email = (split_part(split_part(lower(email), '@', 1), '+', 1) || '@' || split_part(lower(email), '@', 2)) +WHERE email IS NOT NULL + AND position('@' in email) > 1 + AND ( + lower(email) LIKE '%@icloud.com' + OR lower(email) LIKE '%@me.com' + OR lower(email) LIKE '%@mac.com' + ) + AND clean_email = (replace(split_part(split_part(lower(email), '@', 1), '+', 1), '.', '') || '@' || split_part(lower(email), '@', 2)) + AND clean_email IS DISTINCT FROM (split_part(split_part(lower(email), '@', 1), '+', 1) || '@' || split_part(lower(email), '@', 2)); + +-- The same collapse decided who a pending invite belongs to, so the holder +-- of the undotted address claims invites addressed to the dotted one. The +-- typed address rides along as `invitedAddress` whenever it differed. +UPDATE "share" +SET recipient_email = (split_part(split_part(lower((data->>'invitedAddress')), '@', 1), '+', 1) || '@' || split_part(lower((data->>'invitedAddress')), '@', 2)) +WHERE (data->>'invitedAddress') IS NOT NULL + AND position('@' in (data->>'invitedAddress')) > 1 + AND ( + lower(recipient_email) LIKE '%@icloud.com' + OR lower(recipient_email) LIKE '%@me.com' + OR lower(recipient_email) LIKE '%@mac.com' + ) + AND recipient_email = (replace(split_part(split_part(lower((data->>'invitedAddress')), '@', 1), '+', 1), '.', '') || '@' || split_part(lower((data->>'invitedAddress')), '@', 2)) + AND recipient_email IS DISTINCT FROM (split_part(split_part(lower((data->>'invitedAddress')), '@', 1), '+', 1) || '@' || split_part(lower((data->>'invitedAddress')), '@', 2)); diff --git a/src/backend/clients/database/migrations/sqlite/0091_icloud-dot-significance.sql b/src/backend/clients/database/migrations/sqlite/0091_icloud-dot-significance.sql new file mode 100644 index 000000000..6abce2b3f --- /dev/null +++ b/src/backend/clients/database/migrations/sqlite/0091_icloud-dot-significance.sql @@ -0,0 +1,73 @@ +-- Copyright (C) 2024-present Puter Technologies Inc. +-- +-- This file is part of Puter. +-- +-- Puter is free software: you can redistribute it and/or modify +-- it under the terms of the GNU Affero General Public License as published +-- by the Free Software Foundation, either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU Affero General Public License for more details. +-- +-- You should have received a copy of the GNU Affero General Public License +-- along with this program. If not, see . + +-- Recompute `clean_email` for Apple's domains: dots are significant there, so +-- rows written while they were stripped still collapse two mailboxes into one. +-- Only rows still holding exactly what the old rule produced, so a NULL or a +-- legacy un-lowercased value is left alone -- rewriting those would fold `+tag` +-- or case into a key another row already owns. Restores dots and nothing else. + +UPDATE `user` +SET `clean_email` = (CASE WHEN instr(substr(lower(`email`), 1, instr(`email`, '@') - 1), '+') > 0 + THEN substr(substr(lower(`email`), 1, instr(`email`, '@') - 1), 1, + instr(substr(lower(`email`), 1, instr(`email`, '@') - 1), '+') - 1) + ELSE substr(lower(`email`), 1, instr(`email`, '@') - 1) + END || substr(lower(`email`), instr(`email`, '@'))) +WHERE `email` IS NOT NULL + AND instr(`email`, '@') > 1 + AND ( + lower(`email`) LIKE '%@icloud.com' + OR lower(`email`) LIKE '%@me.com' + OR lower(`email`) LIKE '%@mac.com' + ) + AND `clean_email` = (replace(CASE WHEN instr(substr(lower(`email`), 1, instr(`email`, '@') - 1), '+') > 0 + THEN substr(substr(lower(`email`), 1, instr(`email`, '@') - 1), 1, + instr(substr(lower(`email`), 1, instr(`email`, '@') - 1), '+') - 1) + ELSE substr(lower(`email`), 1, instr(`email`, '@') - 1) + END, '.', '') || substr(lower(`email`), instr(`email`, '@'))) + AND `clean_email` <> (CASE WHEN instr(substr(lower(`email`), 1, instr(`email`, '@') - 1), '+') > 0 + THEN substr(substr(lower(`email`), 1, instr(`email`, '@') - 1), 1, + instr(substr(lower(`email`), 1, instr(`email`, '@') - 1), '+') - 1) + ELSE substr(lower(`email`), 1, instr(`email`, '@') - 1) + END || substr(lower(`email`), instr(`email`, '@'))); + +-- The same collapse decided who a pending invite belongs to, so the holder +-- of the undotted address claims invites addressed to the dotted one. The +-- typed address rides along as `invitedAddress` whenever it differed. +UPDATE `share` +SET `recipient_email` = (CASE WHEN instr(substr(lower(json_extract(`data`, '$.invitedAddress')), 1, instr(json_extract(`data`, '$.invitedAddress'), '@') - 1), '+') > 0 + THEN substr(substr(lower(json_extract(`data`, '$.invitedAddress')), 1, instr(json_extract(`data`, '$.invitedAddress'), '@') - 1), 1, + instr(substr(lower(json_extract(`data`, '$.invitedAddress')), 1, instr(json_extract(`data`, '$.invitedAddress'), '@') - 1), '+') - 1) + ELSE substr(lower(json_extract(`data`, '$.invitedAddress')), 1, instr(json_extract(`data`, '$.invitedAddress'), '@') - 1) + END || substr(lower(json_extract(`data`, '$.invitedAddress')), instr(json_extract(`data`, '$.invitedAddress'), '@'))) +WHERE json_extract(`data`, '$.invitedAddress') IS NOT NULL + AND instr(json_extract(`data`, '$.invitedAddress'), '@') > 1 + AND ( + lower(`recipient_email`) LIKE '%@icloud.com' + OR lower(`recipient_email`) LIKE '%@me.com' + OR lower(`recipient_email`) LIKE '%@mac.com' + ) + AND `recipient_email` = (replace(CASE WHEN instr(substr(lower(json_extract(`data`, '$.invitedAddress')), 1, instr(json_extract(`data`, '$.invitedAddress'), '@') - 1), '+') > 0 + THEN substr(substr(lower(json_extract(`data`, '$.invitedAddress')), 1, instr(json_extract(`data`, '$.invitedAddress'), '@') - 1), 1, + instr(substr(lower(json_extract(`data`, '$.invitedAddress')), 1, instr(json_extract(`data`, '$.invitedAddress'), '@') - 1), '+') - 1) + ELSE substr(lower(json_extract(`data`, '$.invitedAddress')), 1, instr(json_extract(`data`, '$.invitedAddress'), '@') - 1) + END, '.', '') || substr(lower(json_extract(`data`, '$.invitedAddress')), instr(json_extract(`data`, '$.invitedAddress'), '@'))) + AND `recipient_email` <> (CASE WHEN instr(substr(lower(json_extract(`data`, '$.invitedAddress')), 1, instr(json_extract(`data`, '$.invitedAddress'), '@') - 1), '+') > 0 + THEN substr(substr(lower(json_extract(`data`, '$.invitedAddress')), 1, instr(json_extract(`data`, '$.invitedAddress'), '@') - 1), 1, + instr(substr(lower(json_extract(`data`, '$.invitedAddress')), 1, instr(json_extract(`data`, '$.invitedAddress'), '@') - 1), '+') - 1) + ELSE substr(lower(json_extract(`data`, '$.invitedAddress')), 1, instr(json_extract(`data`, '$.invitedAddress'), '@') - 1) + END || substr(lower(json_extract(`data`, '$.invitedAddress')), instr(json_extract(`data`, '$.invitedAddress'), '@'))); diff --git a/src/backend/services/auth/OIDCService.test.ts b/src/backend/services/auth/OIDCService.test.ts index 6ef3024f8..7ce4272a5 100644 --- a/src/backend/services/auth/OIDCService.test.ts +++ b/src/backend/services/auth/OIDCService.test.ts @@ -23,6 +23,7 @@ import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { runWithContext } from '../../core/context.js'; import { PuterServer } from '../../server.js'; import { setupTestServer } from '../../testUtil.js'; +import { cleanEmail } from '../../util/email.js'; import { generate_identifier } from '../../util/identifier.js'; import { generateDefaultFsentries } from '../../util/userProvisioning.js'; import { @@ -390,6 +391,31 @@ describe('OIDCService.createUserFromOIDC', () => { expect(result.raced).toBe(true); }); + it('does not resolve a dotted Apple address onto its undotted namesake', async () => { + const stem = `js${crypto.randomBytes(4).toString('hex')}`; + const victim = `${stem}@icloud.com`; + const attacker = `${stem.slice(0, 2)}.${stem.slice(2)}@icloud.com`; + + await server.stores.user.create({ + username: `v-${crypto.randomBytes(4).toString('hex')}`, + uuid: crypto.randomUUID(), + password: 'hashed', + email: victim, + clean_email: cleanEmail(victim), + email_confirmed: true, + }); + + // Apple allocates the exact string, so these are two mailboxes. + expect(cleanEmail(attacker)).not.toBe(cleanEmail(victim)); + await expect( + oidc().findUserByEmail(attacker, { force: true }), + ).resolves.toBeNull(); + + // The undotted spelling is still the account it names. + const found = await oidc().findUserByEmail(victim, { force: true }); + expect(found?.email).toBe(victim); + }); + it('leaves no orphan account behind when the identity was linked first', async () => { // The sub is already bound to another account, so `link()` throws after // this call has created its own user. That account can never be signed diff --git a/src/backend/util/email.test.ts b/src/backend/util/email.test.ts index 580a51c43..f02ffe351 100644 --- a/src/backend/util/email.test.ts +++ b/src/backend/util/email.test.ts @@ -61,8 +61,21 @@ describe('cleanEmail', () => { expect(cleanEmail('foo.bar@googlemail.com')).toBe('foobar@gmail.com'); }); - it('drops dots for icloud too', () => { - expect(cleanEmail('foo.bar@icloud.com')).toBe('foobar@icloud.com'); + // Apple allocates the exact string: both of these can have owners. + it('keeps dots for icloud, which treats them as significant', () => { + expect(cleanEmail('foo.bar@icloud.com')).toBe('foo.bar@icloud.com'); + expect(cleanEmail('foobar@icloud.com')).toBe('foobar@icloud.com'); + for (const domain of ['icloud.com', 'me.com', 'mac.com']) { + expect(cleanEmail(`a.b@${domain}`)).not.toBe( + cleanEmail(`ab@${domain}`), + ); + } + }); + + it('still drops `+` for icloud, which does fold subaddressing', () => { + expect(cleanEmail('foo.bar+tag@icloud.com')).toBe( + 'foo.bar@icloud.com', + ); }); it('keeps `+` for yahoo, which treats it as significant', () => { diff --git a/src/backend/util/email.ts b/src/backend/util/email.ts index 8fe4a7a9b..8dd0406d8 100644 --- a/src/backend/util/email.ts +++ b/src/backend/util/email.ts @@ -46,7 +46,8 @@ const RULES: Record void> = { /** Rules each provider's semantics allow. Unlisted domains get none. */ const PROVIDERS: Record = { gmail: { rules: ['dots_dont_matter', 'remove_subaddressing'] }, - icloud: { rules: ['dots_dont_matter', 'remove_subaddressing'] }, + // Dots are significant at Apple: `j.smith@` and `jsmith@` are two mailboxes. + icloud: { rules: ['remove_subaddressing'] }, outlook: { rules: ['remove_subaddressing'] }, proton: { rules: ['remove_subaddressing'] }, fastmail: { rules: ['remove_subaddressing'] },