From 3803beb5ead91186f8280a883a0a6786ce6532bd Mon Sep 17 00:00:00 2001 From: iuliandanea Date: Mon, 30 Sep 2024 15:28:00 +0300 Subject: [PATCH 1/2] PER-9717 [back-end] Endpoint to retrieve environment configuration New endpoint feature-flags that will return a list of globally enabled flag names if the user is not authenticated or a regular user and all feature flags if the user is an administrator Functional testing was added for both scenarios --- .../api/src/middleware/authentication.test.ts | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/packages/api/src/middleware/authentication.test.ts b/packages/api/src/middleware/authentication.test.ts index 7b37561..59b5a6a 100644 --- a/packages/api/src/middleware/authentication.test.ts +++ b/packages/api/src/middleware/authentication.test.ts @@ -441,3 +441,38 @@ describe("extractUserIsAdminFromAuthToken", () => { expect(request.body.admin).toBe(false); }); }); + +describe("extractUserIsAdminFromAuthToken", () => { + test("is admin will be true if there is an valid auth token", async () => { + const request = { + body: {}, + get: (_: string) => "Bearer test", + } as Request; + jest + .spyOn(fusionAuthClient, "introspectAccessToken") + .mockImplementationOnce(async () => successfulIntrospectionResponse); + await extractUserIsAdminFromAuthToken(request, {} as Response, () => {}); + expect(request.body.admin).toBe(true); + }); + + test("is admin will be false if there is no auth token", async () => { + const request = { + body: {}, + get: (_: string) => "", + } as Request; + await extractUserIsAdminFromAuthToken(request, {} as Response, () => {}); + expect(request.body.admin).toBe(false); + }); + + test("is admin will be false if there is an invalid auth token", async () => { + const request = { + body: {}, + get: (_: string) => "Bearer test", + } as Request; + jest + .spyOn(fusionAuthClient, "introspectAccessToken") + .mockImplementationOnce(async () => failedIntrospectionResponse); + await extractUserIsAdminFromAuthToken(request, {} as Response, () => {}); + expect(request.body.admin).toBe(false); + }); +}); From 2f70ca177b270974789a722812ae0e8a60274c6e Mon Sep 17 00:00:00 2001 From: iuliandanea Date: Tue, 22 Oct 2024 12:27:35 +0300 Subject: [PATCH 2/2] PER-9718 [back-end] Endpoint for admins to create environment configuration New endpoint to create a feature-flag. There are validations on the name and description + the name of the flag has to be unique. Unit and functional tests are covering all scenarios --- .../api/src/feature_flag/controller.test.ts | 193 +++++++++++++++++- packages/api/src/feature_flag/controller.ts | 27 ++- packages/api/src/feature_flag/models.ts | 7 + .../queries/create_feature_flag.sql | 15 ++ .../api/src/feature_flag/service/create.ts | 41 ++++ .../api/src/feature_flag/validators.test.ts | 78 +++++++ packages/api/src/feature_flag/validators.ts | 19 ++ 7 files changed, 369 insertions(+), 11 deletions(-) create mode 100644 packages/api/src/feature_flag/queries/create_feature_flag.sql create mode 100644 packages/api/src/feature_flag/service/create.ts create mode 100644 packages/api/src/feature_flag/validators.test.ts create mode 100644 packages/api/src/feature_flag/validators.ts diff --git a/packages/api/src/feature_flag/controller.test.ts b/packages/api/src/feature_flag/controller.test.ts index 45d8d96..a88def9 100644 --- a/packages/api/src/feature_flag/controller.test.ts +++ b/packages/api/src/feature_flag/controller.test.ts @@ -1,25 +1,29 @@ import request from "supertest"; import type { NextFunction, Request } from "express"; import { logger } from "@stela/logger"; +import createError from "http-errors"; import { db } from "../database"; import { app } from "../app"; -import { extractUserIsAdminFromAuthToken } from "../middleware"; -import type { FeatureFlagRequest } from "./models"; +import { + extractUserIsAdminFromAuthToken, + verifyAdminAuthentication, +} from "../middleware"; +import type { CreateFeatureFlagRequest, FeatureFlagRequest } from "./models"; jest.mock("../database"); jest.mock("../middleware"); jest.mock("@stela/logger"); -describe("GET /feature-flags", () => { - const agent = request(app); +const loadFixtures = async (): Promise => { + await db.sql("fixtures.create_test_feature_flags"); +}; - const loadFixtures = async (): Promise => { - await db.sql("fixtures.create_test_feature_flags"); - }; +const clearDatabase = async (): Promise => { + await db.query("TRUNCATE feature_flag CASCADE"); +}; - const clearDatabase = async (): Promise => { - await db.query("TRUNCATE feature_flag CASCADE"); - }; +describe("GET /feature-flags", () => { + const agent = request(app); beforeEach(async () => { await clearDatabase(); @@ -110,3 +114,172 @@ describe("GET /feature-flags", () => { expect(response.body).toEqual(expected); }); }); + +describe("POST /feature-flag", () => { + const agent = request(app); + beforeEach(async () => { + (verifyAdminAuthentication as jest.Mock).mockImplementation( + (req: Request, __, next: NextFunction) => { + (req.body as CreateFeatureFlagRequest).emailFromAuthToken = + "test@permanent.org"; + (req.body as CreateFeatureFlagRequest).adminSubjectFromAuthToken = + "6b640c73-4963-47de-a096-4a05ff8dc5f5"; + next(); + } + ); + jest.restoreAllMocks(); + jest.clearAllMocks(); + await loadFixtures(); + await clearDatabase(); + }); + + afterEach(async () => { + jest.restoreAllMocks(); + jest.clearAllMocks(); + await clearDatabase(); + }); + + test("should respond with a 200 status code", async () => { + await agent + .post("/api/v2/feature-flags") + .send({ + name: "TEST", + description: "description", + }) + .expect(200); + }); + + test("should respond with a 400 if feature flag already exists", async () => { + await agent + .post("/api/v2/feature-flags") + .send({ + name: "TEST", + description: "description", + }) + .expect(200); + + // trying to create again the same feature flag should fail + await agent + .post("/api/v2/feature-flags") + .send({ + name: "TEST", + description: "description", + }) + .expect(400); + }); + + test("should respond with 401 status code if lacking admin authentication", async () => { + (verifyAdminAuthentication as jest.Mock).mockImplementation( + (_: Request, __, next: NextFunction) => { + next(new createError.Unauthorized("You aren't logged in")); + } + ); + await agent.post("/api/v2/feature-flags").expect(401); + }); + + test("should respond with 400 status code if missing emailFromAuthToken", async () => { + (verifyAdminAuthentication as jest.Mock).mockImplementation( + (req: Request, __, next: NextFunction) => { + (req.body as CreateFeatureFlagRequest).adminSubjectFromAuthToken = + "6b640c73-4963-47de-a096-4a05ff8dc5f5"; + next(); + } + ); + await agent + .post("/api/v2/feature-flags") + .send({ + name: "TEST", + description: "description", + }) + .expect(400); + }); + + test("should respond with 400 status code if emailFromAuthToken is not a string", async () => { + (verifyAdminAuthentication as jest.Mock).mockImplementation( + (req: Request, __, next: NextFunction) => { + (req.body as { emailFromAuthToken: number }).emailFromAuthToken = 123; + (req.body as CreateFeatureFlagRequest).adminSubjectFromAuthToken = + "6b640c73-4963-47de-a096-4a05ff8dc5f5"; + next(); + } + ); + await agent + .post("/api/v2/feature-flags") + .send({ + name: "TEST", + description: "description", + }) + .expect(400); + }); + + test("should respond with 400 status name if code is missing", async () => { + await agent + .post("/api/v2/feature-flags") + .send({ + description: "description", + }) + .expect(400); + }); + + test("should respond with 400 status code if name is not a string", async () => { + await agent + .post("/api/v2/feature-flags") + .send({ + name: 123, + description: "description", + }) + .expect(400); + }); + + test("should store the new feature flag in the database", async () => { + await agent + .post("/api/v2/feature-flags") + .send({ + name: "name", + description: "description", + }) + .expect(200); + const result = await db.query( + `SELECT + name, + description, + globally_enabled::boolean as "globallyEnabled" + FROM + feature_flag + WHERE + name = 'name'` + ); + expect(result.rows.length).toBe(1); + expect(result.rows[0]).toEqual({ + name: "name", + description: "description", + globallyEnabled: false, + }); + }); + + test("should respond with 500 if the database call fails", async () => { + jest.spyOn(db, "sql").mockImplementation(() => { + throw new Error("SQL error"); + }); + await agent + .post("/api/v2/feature-flags") + .send({ + name: "name", + description: "description", + }) + .expect(500); + }); + + test("should log the error if the database call fails", async () => { + const testError = new Error("SQL error"); + jest.spyOn(db, "sql").mockRejectedValueOnce(testError); + await agent + .post("/api/v2/feature-flags") + .send({ + name: "name", + description: "description", + }) + .expect(500); + expect(logger.error).toHaveBeenCalled(); + }); +}); diff --git a/packages/api/src/feature_flag/controller.ts b/packages/api/src/feature_flag/controller.ts index a1fcfbb..009e7e3 100644 --- a/packages/api/src/feature_flag/controller.ts +++ b/packages/api/src/feature_flag/controller.ts @@ -3,7 +3,12 @@ import type { Request, Response, NextFunction } from "express"; import { logger } from "@stela/logger"; import { featureService } from "./service"; -import { extractUserIsAdminFromAuthToken } from "../middleware"; +import { createFeatureService } from "./service/create"; +import { + extractUserIsAdminFromAuthToken, + verifyAdminAuthentication, +} from "../middleware"; +import { validateCreateFeatureFlagRequest } from "./validators"; import { validateIsAdminFromAuthentication } from "../validators/shared"; import { isValidationError } from "../validators/validator_util"; @@ -27,3 +32,23 @@ featureController.get( } } ); + +featureController.post( + "/", + verifyAdminAuthentication, + async (req: Request, res: Response, next: NextFunction) => { + try { + validateCreateFeatureFlagRequest(req.body); + const insertedFeatureFlag = await createFeatureService.createFeatureFlag( + req.body + ); + res.status(200).send({ data: insertedFeatureFlag }); + } catch (err) { + if (isValidationError(err)) { + res.status(400).json({ error: err.message }); + return; + } + next(err); + } + } +); diff --git a/packages/api/src/feature_flag/models.ts b/packages/api/src/feature_flag/models.ts index 3bdf565..08234ba 100644 --- a/packages/api/src/feature_flag/models.ts +++ b/packages/api/src/feature_flag/models.ts @@ -11,6 +11,13 @@ export interface FeatureFlagNameRow { name: string; } +export interface CreateFeatureFlagRequest { + emailFromAuthToken: string; + adminSubjectFromAuthToken: string; + name: string; + description: string; +} + export interface FeatureFlagRequest { admin: boolean; } diff --git a/packages/api/src/feature_flag/queries/create_feature_flag.sql b/packages/api/src/feature_flag/queries/create_feature_flag.sql new file mode 100644 index 0000000..0dcf0d8 --- /dev/null +++ b/packages/api/src/feature_flag/queries/create_feature_flag.sql @@ -0,0 +1,15 @@ +INSERT INTO feature_flag ( + name, + description, + globally_enabled +) VALUES ( + :name, + :description, + :globally_enabled +) RETURNING +id, +name, +description, +globally_enabled AS "globallyEnabled", +created_at AS "createdAt", +updated_at AS "updatedAt"; diff --git a/packages/api/src/feature_flag/service/create.ts b/packages/api/src/feature_flag/service/create.ts new file mode 100644 index 0000000..42582fd --- /dev/null +++ b/packages/api/src/feature_flag/service/create.ts @@ -0,0 +1,41 @@ +import { logger } from "@stela/logger"; +import createError from "http-errors"; +import { TinyPgError } from "tinypg"; +import type { CreateFeatureFlagRequest, FeatureFlagRow } from "../models"; +import { db } from "../../database"; + +const duplicateFeatureFlagError = + 'duplicate key value violates unique constraint "feature_flag_name_unique"'; + +export const createFeatureFlag = async ( + featureFlagData: CreateFeatureFlagRequest +): Promise => { + const result = await db + .sql("feature_flag.queries.create_feature_flag", { + name: featureFlagData.name, + description: featureFlagData.description, + globally_enabled: false, + }) + .catch((err) => { + logger.error(err); + if ( + err instanceof TinyPgError && + err.message === duplicateFeatureFlagError + ) { + throw new createError.BadRequest("Feature flag already exists"); + } + throw new createError.InternalServerError( + "Failed to create feature flag" + ); + }); + + if (result.rows[0] === undefined) { + throw new createError.InternalServerError("Failed to create feature flag"); + } + + return result.rows[0]; +}; + +export const createFeatureService = { + createFeatureFlag, +}; diff --git a/packages/api/src/feature_flag/validators.test.ts b/packages/api/src/feature_flag/validators.test.ts new file mode 100644 index 0000000..439910f --- /dev/null +++ b/packages/api/src/feature_flag/validators.test.ts @@ -0,0 +1,78 @@ +import { validateCreateFeatureFlagRequest } from "./validators"; + +describe("validateCreateFeatureFlagRequest", () => { + test("should find no errors in a valid request", () => { + let error = null; + try { + validateCreateFeatureFlagRequest({ + emailFromAuthToken: "user@example.com", + adminSubjectFromAuthToken: "5c3473b6-cf2e-4c55-a80e-8db51d1bc5fd", + name: "test-feature-flag", + description: "feature flag description", + }); + } catch (err) { + error = err; + } finally { + expect(error).toBeNull(); + } + }); + test("should not raise an error when optional fields are missing", () => { + let error = null; + try { + validateCreateFeatureFlagRequest({ + emailFromAuthToken: "user@example.com", + adminSubjectFromAuthToken: "5c3473b6-cf2e-4c55-a80e-8db51d1bc5fd", + name: "test-feature-flag", + }); + } catch (err) { + error = err; + } finally { + expect(error).toBeNull(); + } + }); + test("should raise an error if name is missing", () => { + let error = null; + try { + validateCreateFeatureFlagRequest({ + emailFromAuthToken: "user@example.com", + adminSubjectFromAuthToken: "5c3473b6-cf2e-4c55-a80e-8db51d1bc5fd", + description: "feature flag description", + }); + } catch (err) { + error = err; + } finally { + expect(error).not.toBeNull(); + } + }); + test("should raise an error if name is wrong type", () => { + let error = null; + try { + validateCreateFeatureFlagRequest({ + emailFromAuthToken: "user@example.com", + adminSubjectFromAuthToken: "5c3473b6-cf2e-4c55-a80e-8db51d1bc5fd", + name: 1, + description: "feature flag description", + }); + } catch (err) { + error = err; + } finally { + expect(error).not.toBeNull(); + } + }); + + test("should raise an error if description is wrong type", () => { + let error = null; + try { + validateCreateFeatureFlagRequest({ + emailFromAuthToken: "user@example.com", + adminSubjectFromAuthToken: "5c3473b6-cf2e-4c55-a80e-8db51d1bc5fd", + name: "name", + description: 123, + }); + } catch (err) { + error = err; + } finally { + expect(error).not.toBeNull(); + } + }); +}); diff --git a/packages/api/src/feature_flag/validators.ts b/packages/api/src/feature_flag/validators.ts new file mode 100644 index 0000000..9a3fff1 --- /dev/null +++ b/packages/api/src/feature_flag/validators.ts @@ -0,0 +1,19 @@ +import Joi from "joi"; +import type { CreateFeatureFlagRequest } from "./models"; +import { fieldsFromAdminAuthentication } from "../validators"; + +export function validateCreateFeatureFlagRequest( + data: unknown +): asserts data is CreateFeatureFlagRequest { + const validation = Joi.object() + .keys({ + ...fieldsFromAdminAuthentication, + name: Joi.string().required(), + description: Joi.string(), + }) + .validate(data); + + if (validation.error) { + throw validation.error; + } +}