Merge pull request #2493 from caprover/fix/custom-domain-uniqueness

Reject duplicate custom domains across apps

src/datastore/AppsDataStore.ts

@@ -491,26 +491,54 @@ class AppsDataStore {
     addCustomDomainForApp(appName: string, customDomain: string) {
         const self = this
 
-        return this.getAppDefinition(appName).then(function (app) {
-            app.customDomain = app.customDomain || []
+        return this.ensureCustomDomainIsAvailable(appName, customDomain)
+            .then(function () {
+                return self.getAppDefinition(appName)
+            })
+            .then(function (app) {
+                app.customDomain = app.customDomain || []
 
-            if (app.customDomain.length > 0) {
-                for (let idx = 0; idx < app.customDomain.length; idx++) {
-                    if (app.customDomain[idx].publicDomain === customDomain) {
-                        throw ApiStatusCodes.createError(
-                            ApiStatusCodes.ILLEGAL_PARAMETER,
-                            `App already has customDomain: ${customDomain} attached to app ${appName}`
-                        )
+                if (app.customDomain.length > 0) {
+                    for (let idx = 0; idx < app.customDomain.length; idx++) {
+                        if (
+                            app.customDomain[idx].publicDomain === customDomain
+                        ) {
+                            throw ApiStatusCodes.createError(
+                                ApiStatusCodes.ILLEGAL_PARAMETER,
+                                `App already has customDomain: ${customDomain} attached to app ${appName}`
+                            )
+                        }
                     }
                 }
-            }
 
-            app.customDomain.push({
-                publicDomain: customDomain,
-                hasSsl: false,
+                app.customDomain.push({
+                    publicDomain: customDomain,
+                    hasSsl: false,
+                })
+
+                return self.saveApp(appName, app)
             })
+    }
 
-            return self.saveApp(appName, app)
+    ensureCustomDomainIsAvailable(appName: string, customDomain: string) {
+        return this.getAppDefinitions().then(function (apps) {
+            for (const existingAppName of Object.keys(apps)) {
+                if (existingAppName === appName) continue
+
+                const domainOwner = (
+                    apps[existingAppName].customDomain || []
+                ).find(
+                    (domain) =>
+                        domain.publicDomain.toLowerCase() ===
+                        customDomain.toLowerCase()
+                )
+                if (domainOwner) {
+                    throw ApiStatusCodes.createError(
+                        ApiStatusCodes.ILLEGAL_PARAMETER,
+                        `Custom domain ${customDomain} is already attached to app ${existingAppName}`
+                    )
+                }
+            }
         })
     }
 

src/user/ServiceManager.ts

@@ -289,6 +289,11 @@ class ServiceManager {
         const self = this
 
         return Promise.resolve()
+            .then(function () {
+                return self.dataStore
+                    .getAppsDataStore()
+                    .ensureCustomDomainIsAvailable(appName, customDomain)
+            })
             .then(function () {
                 const rootDomain = self.dataStore.getRootDomain()
 

tests/CustomDomainUniqueness.test.ts

@@ -0,0 +1,47 @@
+import AppsDataStore from '../src/datastore/AppsDataStore'
+
+describe('custom domain ownership', () => {
+    test('rejects a domain already attached to another app', async () => {
+        const appsDataStore = new AppsDataStore({} as any, 'captain')
+        jest.spyOn(appsDataStore, 'getAppDefinitions').mockResolvedValue({
+            'first-app': {
+                customDomain: [
+                    {
+                        publicDomain: 'Shared.Example.Test',
+                        hasSsl: false,
+                    },
+                ],
+            } as any,
+            'second-app': { customDomain: [] } as any,
+        })
+
+        await expect(
+            appsDataStore.ensureCustomDomainIsAvailable(
+                'second-app',
+                'shared.example.test'
+            )
+        ).rejects.toMatchObject({
+            captainErrorType: 1110,
+            apiMessage:
+                'Custom domain shared.example.test is already attached to app first-app',
+        })
+    })
+
+    test('allows the app that already owns the domain', async () => {
+        const appsDataStore = new AppsDataStore({} as any, 'captain')
+        jest.spyOn(appsDataStore, 'getAppDefinitions').mockResolvedValue({
+            'first-app': {
+                customDomain: [
+                    { publicDomain: 'shared.example.test', hasSsl: false },
+                ],
+            } as any,
+        })
+
+        await expect(
+            appsDataStore.ensureCustomDomainIsAvailable(
+                'first-app',
+                'shared.example.test'
+            )
+        ).resolves.toBeUndefined()
+    })
+})