Skip to content

Commit

Permalink
add projectId scoping to streams/sessions/webhook (#2103)
Browse files Browse the repository at this point in the history
* stream/session: add projectId scoping

* webhook: add projectId scoping

* api/middleware: Create helper functions to check access to resource

* user resource access helper

* address comments

* fix common

* fix tests

* signing keys

* check resource access

* remove project id when not needed

* address comments

* fix

* fix tests

* fix

* address comment

* better tests

* regex for test-single

---------

Co-authored-by: gioelecerati <gioele@cerati.org>
  • Loading branch information
emranemran and gioelecerati committed May 10, 2024
1 parent 145fa59 commit 21dbe39
Show file tree
Hide file tree
Showing 13 changed files with 345 additions and 184 deletions.
1 change: 1 addition & 0 deletions packages/api/package.json
Expand Up @@ -33,6 +33,7 @@
"go-livepeer:broadcaster": "bin/livepeer -broadcaster -datadir ./bin/broadcaster -orchAddr 127.0.0.1:3086 -rtmpAddr 0.0.0.0:3035 -httpAddr :3085 -cliAddr :3075 -v 6 -authWebhookUrl http://127.0.0.1:3004/api/stream/hook -orchWebhookUrl http://127.0.0.1:3004/api/orchestrator",
"go-livepeer:orchestrator": "bin/livepeer -orchestrator -datadir ./bin/orchestrator -transcoder -serviceAddr 127.0.0.1:3086 -cliAddr :3076 -v 6",
"test": "POSTGRES_CONNECT_TIMEOUT=120000 jest -i --silent \"${PWD}/src\"",
"test-single": "POSTGRES_CONNECT_TIMEOUT=120000 jest -i --silent \"${PWD}/src/controllers/+$filename.+\"",
"test:dev": "jest \"${PWD}/src\" -i --silent --watch",
"test:build": "parcel build --no-autoinstall --no-minify --bundle-node-modules -t browser --out-dir ../dist-worker ../src/worker.js",
"coverage": "yarn run test --coverage",
Expand Down
14 changes: 9 additions & 5 deletions packages/api/src/controllers/session.ts
@@ -1,8 +1,8 @@
import { Router, Request } from "express";
import sql from "sql-template-strings";

import { authorizer } from "../middleware";
import { User } from "../schema/types";
import { authorizer, hasAccessToResource } from "../middleware";
import { User, Project } from "../schema/types";
import { db } from "../store";
import { DBSession } from "../store/session-table";
import { pathJoin } from "../controllers/helpers";
Expand Down Expand Up @@ -36,6 +36,7 @@ const fieldsMap: FieldsMap = {
userId: `session.data->>'userId'`,
"user.email": { val: `users.data->>'email'`, type: "full-text" },
parentId: `session.data->>'parentId'`,
projectId: `session.data->>'projectId'`,
record: { val: `session.data->'record'`, type: "boolean" },
sourceSegments: `session.data->'sourceSegments'`,
transcodedSegments: {
Expand Down Expand Up @@ -65,10 +66,14 @@ app.get("/", authorizer({}), async (req, res, next) => {
limit = undefined;
}

const query = parseFilters(fieldsMap, filters);

if (!req.user.admin) {
userId = req.user.id;
query.push(
sql`coalesce(session.data->>'projectId', '') = ${req.project?.id || ""}`
);
}
const query = parseFilters(fieldsMap, filters);
if (!all || all === "false" || !req.user.admin) {
query.push(sql`(session.data->>'deleted')::boolean IS false`);
}
Expand Down Expand Up @@ -146,8 +151,7 @@ app.get("/:id", authorizer({}), async (req, res) => {
let session = await db.session.get(req.params.id);
if (
!session ||
((session.userId !== req.user.id || session.deleted) &&
!req.user.admin &&
(!hasAccessToResource(req, session) &&
!LVPR_SDK_EMAILS.includes(req.user.email))
) {
// do not reveal that session exists
Expand Down
54 changes: 53 additions & 1 deletion packages/api/src/controllers/signing-key.test.ts
Expand Up @@ -2,13 +2,20 @@ import serverPromise, { TestServer } from "../test-server";
import {
TestClient,
clearDatabase,
createApiToken,
setupUsers,
verifyJwt,
} from "../test-helpers";
import { SigningKey, SigningKeyResponsePayload, User } from "../schema/types";
import {
ApiToken,
SigningKey,
SigningKeyResponsePayload,
User,
} from "../schema/types";
import { WithID } from "../store/types";
import jwt, { JsonWebTokenError, JwtPayload } from "jsonwebtoken";
import { db } from "../store";
import { createProject } from "../test-helpers";

// includes auth file tests

Expand Down Expand Up @@ -48,6 +55,8 @@ describe("controllers/signing-key", () => {
let samplePrivateKey: string;
let decodedPublicKey: string;
let otherPublicKey: string;
let projectId: string;
let apiKeyWithProject: WithID<ApiToken>;

beforeEach(async () => {
({ client, adminUser, adminToken, nonAdminUser, nonAdminToken } =
Expand All @@ -69,10 +78,27 @@ describe("controllers/signing-key", () => {
otherSigningKey.publicKey,
"base64"
).toString();
// create a new project
client.jwtAuth = nonAdminToken;
let project = await createProject(client);
expect(project).toBeDefined();
projectId = project.id;
apiKeyWithProject = await createApiToken({
client: client,
projectId: project.id,
jwtAuthToken: nonAdminToken,
});
expect(apiKeyWithProject).toMatchObject({
id: expect.any(String),
projectId: projectId,
});
projectId = project.id;
});

it("should create a signing key and display the private key only on creation", async () => {
const preCreationTime = Date.now();
client.jwtAuth = "";
client.apiKey = apiKeyWithProject.id;
let res = await client.post("/access-control/signing-key");
expect(res.status).toBe(201);
const created = (await res.json()) as SigningKeyResponsePayload;
Expand All @@ -81,6 +107,7 @@ describe("controllers/signing-key", () => {
privateKey: expect.any(String),
publicKey: expect.any(String),
createdAt: expect.any(Number),
projectId: projectId,
});
expect(created.createdAt).toBeGreaterThanOrEqual(preCreationTime);
res = await client.get(`/access-control/signing-key/${created.id}`);
Expand All @@ -91,11 +118,26 @@ describe("controllers/signing-key", () => {
});

it("should list all user signing keys", async () => {
client.jwtAuth = nonAdminToken;
client.apiKey = null;
let sigKeyWithoutProject = await client.post(
"/access-control/signing-key"
);
expect(sigKeyWithoutProject.status).toBe(201);
client.jwtAuth = "";
client.apiKey = apiKeyWithProject.id;
let sigkey = await client.post("/access-control/signing-key");
expect(sigkey.status).toBe(201);
const res = await client.get(`/access-control/signing-key`);
expect(res.status).toBe(200);
const output = await res.json();
expect(output).toHaveLength(1);
expect(output[0].projectId).toBe(projectId);
});

it("should create a JWT using the private key and verify it with the public key", async () => {
client.jwtAuth = "";
client.apiKey = apiKeyWithProject.id;
const expiration = Math.floor(Date.now() / 1000) + 1000;
const payload: JwtPayload = {
sub: "b0dcxvwml48mxt2s",
Expand All @@ -120,6 +162,11 @@ describe("controllers/signing-key", () => {
});

it("should allow disable and enable the signing key & change the name", async () => {
client.jwtAuth = "";
client.apiKey = apiKeyWithProject.id;
let sigkey = await client.post("/access-control/signing-key");
expect(sigkey.status).toBe(201);
let signingKey = await sigkey.json();
let res = await client.patch(
`/access-control/signing-key/${signingKey.id}`,
{
Expand All @@ -144,6 +191,11 @@ describe("controllers/signing-key", () => {
});

it("should delete the signing key", async () => {
client.jwtAuth = "";
client.apiKey = apiKeyWithProject.id;
let sigkey = await client.post("/access-control/signing-key");
expect(sigkey.status).toBe(201);
let signingKey = await sigkey.json();
let res = await client.delete(
`/access-control/signing-key/${signingKey.id}`
);
Expand Down
31 changes: 15 additions & 16 deletions packages/api/src/controllers/signing-key.ts
Expand Up @@ -25,6 +25,7 @@ const fieldsMap: FieldsMap = {
name: { val: `signing_key.data->>'name'`, type: "full-text" },
deleted: { val: `signing_key.data->'deleted'`, type: "boolean" },
createdAt: { val: `signing_key.data->'createdAt'`, type: "int" },
projectId: `signing_key.data->>'projectId'`,
userId: `signing_key.data->>'userId'`,
};

Expand Down Expand Up @@ -71,6 +72,12 @@ signingKeyApp.get("/", authorizer({}), async (req, res) => {
query.push(sql`signing_key.data->>'deleted' IS NULL`);
}

query.push(
sql`coalesce(signing_key.data->>'projectId', '') = ${
req.project?.id || ""
}`
);

let fields =
" signing_key.id as id, signing_key.data as data, users.id as usersId, users.data as usersdata";
if (count) {
Expand Down Expand Up @@ -106,6 +113,10 @@ signingKeyApp.get("/", authorizer({}), async (req, res) => {
query.push(sql`signing_key.data->>'userId' = ${req.user.id}`);
query.push(sql`signing_key.data->>'deleted' IS NULL`);

query.push(
sql`coalesce(signing_key.data->>'projectId', '') = ${req.project?.id || ""}`
);

let fields = " signing_key.id as id, signing_key.data as data";
if (count) {
fields = fields + ", count(*) OVER() AS count";
Expand Down Expand Up @@ -137,16 +148,7 @@ signingKeyApp.get("/", authorizer({}), async (req, res) => {
signingKeyApp.get("/:id", authorizer({}), async (req, res) => {
const signingKey = await db.signingKey.get(req.params.id);

if (
!signingKey ||
signingKey.deleted ||
(req.user.admin !== true && req.user.id !== signingKey.userId)
) {
res.status(404);
return res.json({
errors: ["not found"],
});
}
req.checkResourceAccess(signingKey);

res.json(signingKey);
});
Expand Down Expand Up @@ -188,6 +190,7 @@ signingKeyApp.post(
userId: req.user.id,
createdAt: Date.now(),
publicKey: b64PublicKey,
projectId: req.project?.id ?? "",
};

await db.signingKey.create(doc);
Expand All @@ -205,9 +208,7 @@ signingKeyApp.post(
signingKeyApp.delete("/:id", authorizer({}), async (req, res) => {
const { id } = req.params;
const signingKey = await db.signingKey.get(id);
if (!signingKey || signingKey.deleted) {
throw new NotFoundError(`signing key not found`);
}
req.checkResourceAccess(signingKey);
if (!req.user.admin && req.user.id !== signingKey.userId) {
throw new ForbiddenError(`users may only delete their own signing keys`);
}
Expand All @@ -223,9 +224,7 @@ signingKeyApp.patch(
async (req, res) => {
const { id } = req.params;
const signingKey = await db.signingKey.get(id);
if (!signingKey || signingKey.deleted) {
return res.status(404).json({ errors: ["not found"] });
}
req.checkResourceAccess(signingKey);
if (!req.user.admin && req.user.id !== signingKey.userId) {
return res.status(403).json({
errors: ["users may change only their own signing key"],
Expand Down
88 changes: 88 additions & 0 deletions packages/api/src/controllers/stream.test.ts
Expand Up @@ -19,6 +19,8 @@ import {
clearDatabase,
setupUsers,
startAuxTestServer,
createProject,
createApiToken,
} from "../test-helpers";
import serverPromise, { TestServer } from "../test-server";
import { semaphore, sleep } from "../util";
Expand Down Expand Up @@ -113,6 +115,7 @@ describe("controllers/stream", () => {
let nonAdminUser: User;
let nonAdminToken: string;
let nonAdminApiKey: string;
let projectId: string;

beforeEach(async () => {
await server.store.create(mockStore);
Expand All @@ -127,6 +130,9 @@ describe("controllers/stream", () => {
nonAdminApiKey,
} = await setupUsers(server, mockAdminUser, mockNonAdminUser));
client.jwtAuth = adminToken;

projectId = await createProject(client);
expect(projectId).toBeDefined();
});

describe("basic CRUD with JWT authorization", () => {
Expand All @@ -136,6 +142,7 @@ describe("controllers/stream", () => {
const document = {
id: uuid(),
kind: "stream",
projectId: i > 7 ? projectId : undefined,
};
await server.store.create(document);
const res = await client.get(`/stream/${document.id}`);
Expand All @@ -151,6 +158,31 @@ describe("controllers/stream", () => {
id: uuid(),
kind: "stream",
deleted: i > 3 ? true : undefined,
projectId: i > 2 ? projectId : undefined,
} as DBStream;
await server.store.create(document);
const res = await client.get(`/stream/${document.id}`);
const stream = await res.json();
expect(stream).toEqual(server.db.stream.addDefaultFields(document));
}

const res = await client.get("/stream");
expect(res.status).toBe(200);
const streams = await res.json();
expect(streams.length).toEqual(4);
const resAll = await client.get("/stream?all=1");
expect(resAll.status).toBe(200);
const streamsAll = await resAll.json();
expect(streamsAll.length).toEqual(5);
});

it("should get all streams with admin authorization and specific projectId in query param", async () => {
for (let i = 0; i < 5; i += 1) {
const document = {
id: uuid(),
kind: "stream",
deleted: i > 3 ? true : undefined,
projectId: i > 2 ? projectId : undefined,
} as DBStream;
await server.store.create(document);
const res = await client.get(`/stream/${document.id}`);
Expand Down Expand Up @@ -1393,7 +1425,9 @@ describe("controllers/stream", () => {
});

describe("stream endpoint with api key", () => {
let newApiKey;
beforeEach(async () => {
// create streams without a projectId
for (let i = 0; i < 5; i += 1) {
const document = {
id: uuid(),
Expand All @@ -1404,7 +1438,45 @@ describe("controllers/stream", () => {
const res = await client.get(`/stream/${document.id}`);
expect(res.status).toBe(200);
}

// create a new project
client.jwtAuth = nonAdminToken;
let project = await createProject(client);
expect(project).toBeDefined();

// then create a new api-key under that project
newApiKey = await createApiToken({
client: client,
projectId: project.id,
jwtAuthToken: nonAdminToken,
});
expect(newApiKey).toMatchObject({
id: expect.any(String),
projectId: project.id,
});

client.jwtAuth = "";
client.apiKey = newApiKey.id;

// create streams with a projectId
for (let i = 0; i < 5; i += 1) {
const document = {
id: uuid(),
kind: "stream",
userId: nonAdminUser.id,
projectId: project.id,
};
const resCreate = await client.post("/stream", {
...postMockStream,
name: "videorec+samplePlaybackId",
});
expect(resCreate.status).toBe(201);
const createdStream = await resCreate.json();
const res = await client.get(`/stream/${createdStream.id}`);
expect(res.status).toBe(200);
let stream = await res.json();
expect(stream.projectId).toEqual(project.id);
}
});

it("should get own streams", async () => {
Expand All @@ -1414,6 +1486,22 @@ describe("controllers/stream", () => {
const streams = await res.json();
expect(streams.length).toEqual(3);
expect(streams[0].userId).toEqual(nonAdminUser.id);

client.apiKey = newApiKey.id;
let res2 = await client.get(`/stream/user/${nonAdminUser.id}`);
expect(res2.status).toBe(200);
const streams2 = await res2.json();
expect(streams2.length).toEqual(5);
expect(streams2[0].userId).toEqual(nonAdminUser.id);
});

it("should get streams owned by project when using api-key for project", async () => {
client.apiKey = newApiKey.id;
let res = await client.get(`/stream/`);
expect(res.status).toBe(200);
const streams = await res.json();
expect(streams.length).toEqual(5);
expect(streams[0].userId).toEqual(nonAdminUser.id);
});

it("should delete stream", async () => {
Expand Down

0 comments on commit 21dbe39

Please sign in to comment.