Fix/remove settings cache (#2694)

In this PR we remove the general SettingService cache, as it will not
work across multiple horizontal unleash instances, events are not
published across.

We also fix the CORS origin to: 
- Access-Control-Allow-Origin set to "*" if no Origin is configured
- Access-Control-Allow-Origin set to "*" if any Origin is configured to
"*"
- - Access-Control-Allow-Origin set to array and have the "cors"
middleware to return an exact match on the user provided Origin.

Co-authored-by: Fredrik Oseberg <fredrik.no@gmail.com>

frontend/src/component/admin/cors/CorsForm.tsx

@@ -6,6 +6,7 @@ import { useUiConfigApi } from 'hooks/api/actions/useUiConfigApi/useUiConfigApi'
 import useToast from 'hooks/useToast';
 import { formatUnknownError } from 'utils/formatUnknownError';
 import { useId } from 'hooks/useId';
+import { fontSize } from '@mui/system';
 
 interface ICorsFormProps {
     frontendApiOrigins: string[] | undefined;
@@ -35,8 +36,23 @@ export const CorsForm = ({ frontendApiOrigins }: ICorsFormProps) => {
             <Box sx={{ display: 'grid', gap: 1 }}>
                 <label htmlFor={inputFieldId}>
                     Which origins should be allowed to call the Frontend API?
-                    Add only one origin per line.
+                    Add only one origin per line. The CORS specification does
+                    not support wildcard for subdomains, it needs to be a fully
+                    qualified domain, including the protocol.
+                    <br />
+                    <br />
+                    If you specify "*" it will be the chosen origin.
+                    <br />
+                    <br />
+                    Example:
                 </label>
+
+                <code style={{ fontSize: '0.7em' }}>
+                    https://www.example.com
+                    <br />
+                    https://www.example2.com
+                </code>
+
                 <TextField
                     id={inputFieldId}
                     aria-describedby={helpTextId}

frontend/src/component/admin/cors/CorsHelpAlert.tsx

@@ -17,6 +17,15 @@ export const CorsHelpAlert = () => {
                 An asterisk (<code>*</code>) may be used to allow API calls from
                 any origin.
             </p>
+            <br />
+            <p>
+                Be aware that changes here will take up to two minutes to be
+                updated. In addition, there is a maxAge on the
+                Access-Control-Allow-Origin header that will instruct browsers
+                to cache this header for some time. The cache period is set to
+                the maxium that the browser allows (2h for Chrome, 24h for
+                Firefox).
+            </p>
         </Alert>
     );
 };

src/lib/middleware/cors-origin-middleware.test.ts

@@ -1,86 +1,152 @@
-import { allowRequestOrigin } from './cors-origin-middleware';
+import { resolveOrigin } from './cors-origin-middleware';
 import FakeSettingStore from '../../test/fixtures/fake-setting-store';
-import SettingService from '../services/setting-service';
 import { createTestConfig } from '../../test/config/test-config';
 import FakeEventStore from '../../test/fixtures/fake-event-store';
 import { randomId } from '../util/random-id';
-import { frontendSettingsKey } from '../types/settings/frontend-settings';
+import FakeProjectStore from '../../test/fixtures/fake-project-store';
+import { ProxyService, SettingService } from '../../lib/services';
+import { ISettingStore } from '../../lib/types';
+import { frontendSettingsKey } from '../../lib/types/settings/frontend-settings';
+import { minutesToMilliseconds } from 'date-fns';
 
-const createSettingService = (frontendApiOrigins: string[]): SettingService => {
+const createSettingService = (
+    frontendApiOrigins: string[],
+): { proxyService: ProxyService; settingStore: ISettingStore } => {
     const config = createTestConfig({ frontendApiOrigins });
 
     const stores = {
         settingStore: new FakeSettingStore(),
         eventStore: new FakeEventStore(),
+        projectStore: new FakeProjectStore(),
     };
 
-    return new SettingService(stores, config);
+    const services = {
+        settingService: new SettingService(stores, config),
+    };
+
+    return {
+        //@ts-ignore
+        proxyService: new ProxyService(config, stores, services),
+        settingStore: stores.settingStore,
+    };
 };
 
-test('allowRequestOrigin', () => {
+test('resolveOrigin', () => {
     const dotCom = 'https://example.com';
     const dotOrg = 'https://example.org';
 
-    expect(allowRequestOrigin('', [])).toEqual(false);
-    expect(allowRequestOrigin(dotCom, [])).toEqual(false);
-    expect(allowRequestOrigin(dotCom, [dotOrg])).toEqual(false);
-
-    expect(allowRequestOrigin(dotCom, [dotCom, dotOrg])).toEqual(true);
-    expect(allowRequestOrigin(dotCom, [dotOrg, dotCom])).toEqual(true);
-    expect(allowRequestOrigin(dotCom, [dotCom, dotCom])).toEqual(true);
-
-    expect(allowRequestOrigin(dotCom, ['*'])).toEqual(true);
-    expect(allowRequestOrigin(dotCom, [dotOrg, '*'])).toEqual(true);
-    expect(allowRequestOrigin(dotCom, [dotCom, dotOrg, '*'])).toEqual(true);
+    expect(resolveOrigin([])).toEqual('*');
+    expect(resolveOrigin(['*'])).toEqual('*');
+    expect(resolveOrigin([dotOrg])).toEqual([dotOrg]);
+    expect(resolveOrigin([dotCom, dotOrg])).toEqual([dotCom, dotOrg]);
+    expect(resolveOrigin([dotOrg, '*'])).toEqual('*');
 });
 
 test('corsOriginMiddleware origin validation', async () => {
-    const service = createSettingService([]);
+    const { proxyService } = createSettingService([]);
     const userName = randomId();
     await expect(() =>
-        service.setFrontendSettings({ frontendApiOrigins: ['a'] }, userName),
+        proxyService.setFrontendSettings(
+            { frontendApiOrigins: ['a'] },
+            userName,
+        ),
     ).rejects.toThrow('Invalid origin: a');
+    proxyService.destroy();
 });
 
 test('corsOriginMiddleware without config', async () => {
-    const service = createSettingService([]);
+    const { proxyService, settingStore } = createSettingService([]);
     const userName = randomId();
-    expect(await service.getFrontendSettings()).toEqual({
+    expect(await proxyService.getFrontendSettings(false)).toEqual({
         frontendApiOrigins: [],
     });
-    await service.setFrontendSettings({ frontendApiOrigins: [] }, userName);
-    expect(await service.getFrontendSettings()).toEqual({
+    await proxyService.setFrontendSettings(
+        { frontendApiOrigins: [] },
+        userName,
+    );
+    expect(await proxyService.getFrontendSettings(false)).toEqual({
         frontendApiOrigins: [],
     });
-    await service.setFrontendSettings({ frontendApiOrigins: ['*'] }, userName);
-    expect(await service.getFrontendSettings()).toEqual({
+    await proxyService.setFrontendSettings(
+        { frontendApiOrigins: ['*'] },
+        userName,
+    );
+    expect(await proxyService.getFrontendSettings(false)).toEqual({
         frontendApiOrigins: ['*'],
     });
-    await service.delete(frontendSettingsKey, userName);
-    expect(await service.getFrontendSettings()).toEqual({
+    await settingStore.delete(frontendSettingsKey);
+    expect(await proxyService.getFrontendSettings(false)).toEqual({
         frontendApiOrigins: [],
     });
+    proxyService.destroy();
 });
 
 test('corsOriginMiddleware with config', async () => {
-    const service = createSettingService(['*']);
+    const { proxyService, settingStore } = createSettingService(['*']);
     const userName = randomId();
-    expect(await service.getFrontendSettings()).toEqual({
+    expect(await proxyService.getFrontendSettings(false)).toEqual({
         frontendApiOrigins: ['*'],
     });
-    await service.setFrontendSettings({ frontendApiOrigins: [] }, userName);
-    expect(await service.getFrontendSettings()).toEqual({
+    await proxyService.setFrontendSettings(
+        { frontendApiOrigins: [] },
+        userName,
+    );
+    expect(await proxyService.getFrontendSettings(false)).toEqual({
         frontendApiOrigins: [],
     });
-    await service.setFrontendSettings(
+    await proxyService.setFrontendSettings(
         { frontendApiOrigins: ['https://example.com', 'https://example.org'] },
         userName,
     );
-    expect(await service.getFrontendSettings()).toEqual({
+    expect(await proxyService.getFrontendSettings(false)).toEqual({
         frontendApiOrigins: ['https://example.com', 'https://example.org'],
     });
-    await service.delete(frontendSettingsKey, userName);
-    expect(await service.getFrontendSettings()).toEqual({
+    await settingStore.delete(frontendSettingsKey);
+    expect(await proxyService.getFrontendSettings(false)).toEqual({
         frontendApiOrigins: ['*'],
     });
+    proxyService.destroy();
+});
+
+test('corsOriginMiddleware with caching enabled', async () => {
+    jest.useFakeTimers();
+
+    const { proxyService } = createSettingService([]);
+
+    const userName = randomId();
+    expect(await proxyService.getFrontendSettings()).toEqual({
+        frontendApiOrigins: [],
+    });
+
+    //setting
+    await proxyService.setFrontendSettings(
+        { frontendApiOrigins: ['*'] },
+        userName,
+    );
+
+    //still get cached value
+    expect(await proxyService.getFrontendSettings()).toEqual({
+        frontendApiOrigins: [],
+    });
+
+    jest.advanceTimersByTime(minutesToMilliseconds(2));
+
+    jest.useRealTimers();
+
+    /*
+    This is needed because it is not enough to fake time to test the
+    updated cache, we also need to make sure that all promises are 
+    executed and completed, in the right order. 
+    */
+    await new Promise<void>((resolve) =>
+        process.nextTick(async () => {
+            const settings = await proxyService.getFrontendSettings();
+
+            expect(settings).toEqual({
+                frontendApiOrigins: ['*'],
+            });
+            resolve();
+        }),
+    );
+    proxyService.destroy();
 });

src/lib/middleware/cors-origin-middleware.ts

@@ -2,32 +2,32 @@ import { RequestHandler } from 'express';
 import cors from 'cors';
 import { IUnleashConfig, IUnleashServices } from '../types';
 
-export const allowRequestOrigin = (
-    requestOrigin: string,
-    allowedOrigins: string[],
-): boolean => {
-    return allowedOrigins.some((allowedOrigin) => {
-        return allowedOrigin === requestOrigin || allowedOrigin === '*';
-    });
+export const resolveOrigin = (allowedOrigins: string[]): string | string[] => {
+    if (allowedOrigins.length === 0) {
+        return '*';
+    }
+    if (allowedOrigins.some((origin: string) => origin === '*')) {
+        return '*';
+    } else {
+        return allowedOrigins;
+    }
 };
 
 // Check the request's Origin header against a list of allowed origins.
 // The list may include '*', which `cors` does not support natively.
 export const corsOriginMiddleware = (
-    { settingService }: Pick<IUnleashServices, 'settingService'>,
+    { proxyService }: Pick<IUnleashServices, 'proxyService'>,
     config: IUnleashConfig,
 ): RequestHandler => {
     return cors(async (req, callback) => {
         try {
             const { frontendApiOrigins = [] } =
-                await settingService.getFrontendSettings();
+                await proxyService.getFrontendSettings();
             callback(null, {
-                origin: allowRequestOrigin(
-                    req.header('Origin'),
-                    frontendApiOrigins,
-                ),
+                origin: resolveOrigin(frontendApiOrigins),
                 maxAge: config.accessControlMaxAge,
                 exposedHeaders: 'ETag',
+                credentials: true,
             });
         } catch (error) {
             callback(error);

src/lib/routes/admin-api/config.ts

@@ -24,12 +24,15 @@ import { extractUsername } from '../../util/extract-user';
 import NotFoundError from '../../error/notfound-error';
 import { SetUiConfigSchema } from '../../openapi/spec/set-ui-config-schema';
 import { createRequestSchema } from '../../openapi/util/create-request-schema';
+import { ProxyService } from 'lib/services';
 
 class ConfigController extends Controller {
     private versionService: VersionService;
 
     private settingService: SettingService;
 
+    private proxyService: ProxyService;
+
     private emailService: EmailService;
 
     private readonly openApiService: OpenApiService;
@@ -41,19 +44,22 @@ class ConfigController extends Controller {
             settingService,
             emailService,
             openApiService,
+            proxyService,
         }: Pick<
             IUnleashServices,
             | 'versionService'
             | 'settingService'
             | 'emailService'
             | 'openApiService'
+            | 'proxyService'
         >,
     ) {
         super(config);
         this.versionService = versionService;
         this.settingService = settingService;
         this.emailService = emailService;
         this.openApiService = openApiService;
+        this.proxyService = proxyService;
 
         this.route({
             method: 'get',
@@ -92,7 +98,7 @@ class ConfigController extends Controller {
         res: Response<UiConfigSchema>,
     ): Promise<void> {
         const [frontendSettings, simpleAuthSettings] = await Promise.all([
-            this.settingService.getFrontendSettings(),
+            this.proxyService.getFrontendSettings(false),
             this.settingService.get<SimpleAuthSettings>(simpleAuthSettingsKey),
         ]);
 
@@ -133,7 +139,7 @@ class ConfigController extends Controller {
         res: Response<string>,
     ): Promise<void> {
         if (req.body.frontendSettings) {
-            await this.settingService.setFrontendSettings(
+            await this.proxyService.setFrontendSettings(
                 req.body.frontendSettings,
                 extractUsername(req),
             );

src/lib/server-impl.ts

@@ -55,6 +55,7 @@ async function createApp(
         metricsMonitor.stopMonitoring();
         stores.clientInstanceStore.destroy();
         services.clientMetricsServiceV2.destroy();
+        services.proxyService.destroy();
         await db.destroy();
     };
 

src/lib/services/index.ts

@@ -109,6 +109,7 @@ export const createServices = (
         featureToggleServiceV2,
         clientMetricsServiceV2,
         segmentService,
+        settingService,
     });
 
     const edgeService = new EdgeService(stores, config);