fix: when payload type is 'json' validate value on toggle variable validation (#1704)

* fix: when payload type is 'json' validate value on toggle variable validation

* test: add missing feature toggle creation with variant type json

Ref https://github.com/Unleash/unleash/pull/1704#discussion_r896476042

* refractor: remove verbose comment on validateJsonString

Ref https://github.com/Unleash/unleash/pull/1704#discussion_r896482210

* test: add missing feature toggle creation with variant type string

Ref https://github.com/Unleash/unleash/pull/1704#discussion_r896476042

* refractor: move variant value joi validation

Ref https://github.com/Unleash/unleash/pull/1704#discussion_r896478563

src/lib/schema/feature-schema.ts

@@ -1,6 +1,7 @@
 import joi from 'joi';
 import { ALL_OPERATORS } from '../util/constants';
 import { nameType } from '../routes/util';
+import { validateJsonString } from '../util/validateJsonString';
 
 export const nameSchema = joi
     .object()
@@ -27,6 +28,26 @@ export const strategiesSchema = joi.object().keys({
     parameters: joi.object(),
 });
 
+const variantValueSchema = joi
+    .string()
+    .required()
+    // perform additional validation
+    // when provided 'type' is 'json'
+    .when('type', {
+        is: 'json',
+        then: joi.custom((val, helper) => {
+            const isValidJsonString = validateJsonString(val);
+            if (isValidJsonString === false) {
+                return helper.error('invalidJsonString');
+            }
+            return val;
+        }),
+    })
+    .messages({
+        invalidJsonString:
+            "'value' must be a valid json string when 'type' is json",
+    });
+
 export const variantsSchema = joi.object().keys({
     name: nameType,
     weight: joi.number().min(0).max(1000).required(),
@@ -35,7 +56,7 @@ export const variantsSchema = joi.object().keys({
         .object()
         .keys({
             type: joi.string().required(),
-            value: joi.string().required(),
+            value: variantValueSchema,
         })
         .optional(),
     stickiness: joi.string().default('default'),

src/lib/util/validateJsonString.test.ts

@@ -0,0 +1,22 @@
+import { validateJsonString } from './validateJsonString';
+
+test('should return true for valid json string', () => {
+    const input = '{"test":1,"nested":[{"test1":{"testinner":true}}]}';
+    expect(validateJsonString(input)).toBe(true);
+});
+
+test('should return false for invalid json string (missing starting {)', () => {
+    // missing starting {
+    const input = '"test":1,"nested":[{"test1":{"testinner":true}}]}';
+    expect(validateJsonString(input)).toBe(false);
+});
+
+test('should return false for invalid json string (plain string)', () => {
+    const input = 'not a json';
+    expect(validateJsonString(input)).toBe(false);
+});
+
+test('should return false for invalid json string (null as a string)', () => {
+    const input = 'null';
+    expect(validateJsonString(input)).toBe(false);
+});

src/lib/util/validateJsonString.ts

@@ -0,0 +1,12 @@
+export const validateJsonString = (value: string): boolean => {
+    // from https://stackoverflow.com/a/20392392
+    try {
+        const parsedStr = JSON.parse(value);
+        if (parsedStr && typeof parsedStr === 'object') {
+            return true;
+        }
+    } catch (err) {}
+
+    // an error is considered a non valid json
+    return false;
+};

src/test/e2e/api/admin/feature.e2e.test.ts

@@ -261,6 +261,76 @@ test('creates new feature toggle with createdBy unknown', async () => {
     });
 });
 
+test('create new feature toggle with variant type json', async () => {
+    return app.request
+        .post('/api/admin/features')
+        .send({
+            name: 'com.test.featureWithJson',
+            variants: [
+                {
+                    name: 'variantTestJson',
+                    weight: 1,
+                    payload: {
+                        type: 'json',
+                        value: '{"test": true, "user": [{"jsonValid": 1}]}',
+                    },
+                    weightType: 'variable',
+                },
+            ],
+        })
+        .set('Content-Type', 'application/json')
+        .expect(201);
+});
+
+test('create new feature toggle with variant type string', async () => {
+    return app.request
+        .post('/api/admin/features')
+        .send({
+            name: 'com.test.featureWithString',
+            variants: [
+                {
+                    name: 'variantTestString',
+                    weight: 1,
+                    payload: {
+                        type: 'string',
+                        value: 'my string # here',
+                    },
+                    weightType: 'variable',
+                },
+            ],
+        })
+        .set('Content-Type', 'application/json')
+        .expect(201);
+});
+
+test('refuses to create a new feature toggle with variant when type is json but value provided is not a valid json', async () => {
+    return app.request
+        .post('/api/admin/features')
+        .send({
+            name: 'com.test.featureInvalidValue',
+            variants: [
+                {
+                    name: 'variantTest',
+                    weight: 1,
+                    payload: {
+                        type: 'json',
+                        value: 'this should be a # valid json', // <--- testing value
+                    },
+                    weightType: 'variable',
+                },
+            ],
+        })
+        .set('Content-Type', 'application/json')
+        .expect(400)
+        .expect((res) => {
+            expect(res.body.isJoi).toBe(true);
+            expect(res.body.details[0].type).toBe('invalidJsonString');
+            expect(res.body.details[0].message).toBe(
+                `'value' must be a valid json string when 'type' is json`,
+            );
+        });
+});
+
 test('require new feature toggle to have a name', async () => {
     expect.assertions(0);
     return app.request