Merge pull request #4110 from HeyPuter/juancastro/icloud-dot-significance

Treat dots as significant on Apple's mail domains

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

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 {

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 <https://www.gnu.org/licenses/>.
+
+-- 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));

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 <https://www.gnu.org/licenses/>.
+
+-- 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));

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 <https://www.gnu.org/licenses/>.
+
+-- 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'), '@')));

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

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', () => {

src/backend/util/email.ts

@@ -46,7 +46,8 @@ const RULES: Record<RuleName, (p: Parts) => void> = {
 /** Rules each provider's semantics allow. Unlisted domains get none. */
 const PROVIDERS: Record<string, { rules: RuleName[] }> = {
     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'] },