From b0626d46bc7f97184e3d9d34a672cf968503b81b Mon Sep 17 00:00:00 2001 From: Fredrik Strand Oseberg Date: Wed, 19 Oct 2022 12:29:00 +0200 Subject: [PATCH] fix: respect environment if set on context (#2206) When using the frontend api (embedded proxy) we should allow the use to self-define the environment on the proxy. --- src/lib/proxy/create-context.test.ts | 43 ++++++++++++++++++++++-- src/lib/proxy/create-context.ts | 6 ++++ src/lib/routes/proxy-api/index.ts | 6 ++-- src/test/e2e/api/proxy/proxy.e2e.test.ts | 41 ++++++++++++++++++++++ 4 files changed, 89 insertions(+), 7 deletions(-) diff --git a/src/lib/proxy/create-context.test.ts b/src/lib/proxy/create-context.test.ts index 6e6b4ff4aa..8ba87dbbf7 100644 --- a/src/lib/proxy/create-context.test.ts +++ b/src/lib/proxy/create-context.test.ts @@ -1,6 +1,6 @@ // Copy of https://github.com/Unleash/unleash-proxy/blob/main/src/test/create-context.test.ts. -import { createContext } from './create-context'; +import { createContext, enrichContextWithIp } from './create-context'; test('should remove undefined properties', () => { const context = createContext({ @@ -55,13 +55,50 @@ test('will not blow up if properties is an array', () => { properties: ['some'], }); - // console.log(context); - expect(context.userId).toBe('123'); expect(context).not.toHaveProperty('tenantId'); expect(context).not.toHaveProperty('region'); }); +test('will respect environment set in context', () => { + const context = createContext({ + userId: '123', + tenantId: 'some-tenant', + environment: 'development', + region: 'eu', + properties: ['some'], + }); + + expect(context.environment).toBe('development'); +}); + +test('will not set environment to be development if not set in context', () => { + const context = createContext({ + userId: '123', + tenantId: 'some-tenant', + region: 'eu', + properties: ['some'], + }); + + expect(context.environment).toBe(undefined); +}); + +test('will enrich context with ip', () => { + const query = {}; + const ip = '192.168.10.0'; + const result = enrichContextWithIp(query, ip); + + expect(result.remoteAddress).toBe(ip); +}); + +test('will not change environment when enriching', () => { + const query = { environment: 'production' }; + const ip = '192.168.10.0'; + const result = enrichContextWithIp(query, ip); + + expect(result.environment).toBe('production'); +}); + test.skip('will not blow up if userId is an array', () => { const context = createContext({ userId: ['123'], diff --git a/src/lib/proxy/create-context.ts b/src/lib/proxy/create-context.ts index 41430a8dd1..3d6adccdb6 100644 --- a/src/lib/proxy/create-context.ts +++ b/src/lib/proxy/create-context.ts @@ -32,3 +32,9 @@ export function createContext(value: any): Context { return cleanContext; } + +// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types +export const enrichContextWithIp = (query: any, ip: string): Context => { + query.remoteAddress = query.remoteAddress || ip; + return createContext(query); +}; diff --git a/src/lib/routes/proxy-api/index.ts b/src/lib/routes/proxy-api/index.ts index 5cee80a53c..d68bfdc6a1 100644 --- a/src/lib/routes/proxy-api/index.ts +++ b/src/lib/routes/proxy-api/index.ts @@ -9,7 +9,7 @@ import { ProxyFeaturesSchema, } from '../../openapi/spec/proxy-features-schema'; import { Context } from 'unleash-client'; -import { createContext } from '../../proxy/create-context'; +import { enrichContextWithIp } from '../../proxy/create-context'; import { ProxyMetricsSchema } from '../../openapi/spec/proxy-metrics-schema'; import { ProxyClientSchema } from '../../openapi/spec/proxy-client-schema'; import { createResponseSchema } from '../../openapi/util/create-response-schema'; @@ -168,8 +168,6 @@ export default class ProxyController extends Controller { private static createContext(req: ApiUserRequest): Context { const { query } = req; - query.remoteAddress = query.remoteAddress || req.ip; - query.environment = req.user.environment; - return createContext(query); + return enrichContextWithIp(query, req.ip); } } diff --git a/src/test/e2e/api/proxy/proxy.e2e.test.ts b/src/test/e2e/api/proxy/proxy.e2e.test.ts index a6f34c248c..aabcea615f 100644 --- a/src/test/e2e/api/proxy/proxy.e2e.test.ts +++ b/src/test/e2e/api/proxy/proxy.e2e.test.ts @@ -538,6 +538,47 @@ test('should filter features by constraints', async () => { .expect((res) => expect(res.body.toggles).toHaveLength(0)); }); +test('should be able to set environment as a context variable', async () => { + const frontendToken = await createApiToken(ApiTokenType.FRONTEND); + const featureName = 'featureWithEnvironmentConstraint'; + await createFeatureToggle({ + name: featureName, + enabled: true, + strategies: [ + { + name: 'default', + constraints: [ + { + contextName: 'environment', + operator: 'IN', + values: ['staging'], + }, + ], + parameters: {}, + }, + ], + }); + + await app.request + .get('/api/frontend?environment=staging') + .set('Authorization', frontendToken.secret) + .expect('Content-Type', /json/) + .expect(200) + .expect((res) => { + expect(res.body.toggles).toHaveLength(1); + expect(res.body.toggles[0].name).toBe(featureName); + }); + + await app.request + .get('/api/frontend') + .set('Authorization', frontendToken.secret) + .expect('Content-Type', /json/) + .expect(200) + .expect((res) => { + expect(res.body.toggles).toHaveLength(0); + }); +}); + test('should filter features by project', async () => { const projectA = 'projectA'; const projectB = 'projectB';