From d1a154780226653512c3dffe9a8daa277ea36268 Mon Sep 17 00:00:00 2001 From: Sonia Zorba Date: Wed, 6 Nov 2024 10:51:10 +0100 Subject: [PATCH 1/5] Distinguished handling of 401 Unauthorized and 403 Forbidden responses --- src/app.ts | 41 ++-- src/authorizer.ts | 200 ++++++++---------- src/data.ts | 47 ++++ src/path.ts | 29 +++ src/user.ts | 52 +++++ test/app.spec.ts | 83 ++++++++ test/authorizer/authorizer-mocks.ts | 22 -- test/authorizer/none-authorizer.spec.ts | 51 ----- .../user-folders-authorizer.spec.ts | 50 +++-- .../viewer-paths-authorizer.spec.ts | 45 ++-- test/mock.ts | 36 ++++ test/path.spec.ts | 29 +++ 12 files changed, 436 insertions(+), 249 deletions(-) create mode 100644 src/data.ts create mode 100644 src/path.ts create mode 100644 src/user.ts create mode 100644 test/app.spec.ts delete mode 100644 test/authorizer/authorizer-mocks.ts delete mode 100644 test/authorizer/none-authorizer.spec.ts create mode 100644 test/mock.ts create mode 100644 test/path.spec.ts diff --git a/src/app.ts b/src/app.ts index 2e90399..0dcfaf1 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1,8 +1,8 @@ -import express from 'express'; -import * as fs from 'fs'; -import { getLogger } from './logger.js'; -import { getConfig } from './config.js'; -import { getAuthorizer } from './authorizer.js'; +import express from "express"; +import { getLogger } from "./logger.js"; +import { getConfig } from "./config.js"; +import { serveZarrData } from "./data.js"; +import { getAuthorizer } from "./authorizer.js"; const config = getConfig(); const logger = getLogger(); @@ -20,26 +20,7 @@ const authorizer = getAuthorizer(); // Endpoint serving zarr files app.use(`${config.basePath}data`, async function (req, res) { - try { - const authorizedPath = await authorizer.getAuthorizedPath(req); - if (!authorizedPath) { - logger.info("Forbidden request: %s", req.path.normalize()); - return res.status(403).send('Forbidden').end(); - } - if (!fs.existsSync(authorizedPath)) { - logger.info("File not found: %s", authorizedPath); - return res.status(404).send('Not Found').end(); - } - if (fs.lstatSync(authorizedPath).isDirectory()) { - logger.info("Path is directory: %s", authorizedPath); - return res.status(400).send('Is directory').end(); - } - const stream = fs.createReadStream(authorizedPath); - stream.pipe(res); - } catch (err) { - logger.error('Error reading file', err); - return res.status(500).send('Internal Server Error').end(); - } + await serveZarrData(authorizer, req, res); }); // Serving Vizarr static files @@ -47,12 +28,16 @@ app.use(`${config.basePath}`, express.static(config.vizarrStaticFilesPath)); // Start server const server = app.listen(config.port, () => { - logger.info('fractal-vizarr-viewer is listening at http://localhost:%d%s', config.port, config.basePath) + logger.info( + "fractal-vizarr-viewer is listening at http://localhost:%d%s", + config.port, + config.basePath + ); }); -for (const signal of ['SIGINT', 'SIGTERM', 'SIGQUIT']) { +for (const signal of ["SIGINT", "SIGTERM", "SIGQUIT"]) { process.on(signal, (signal) => { - logger.info('Process received a %s signal', signal); + logger.info("Process received a %s signal", signal); server.close(); }); } diff --git a/src/authorizer.ts b/src/authorizer.ts index f434210..3bcc39e 100644 --- a/src/authorizer.ts +++ b/src/authorizer.ts @@ -1,9 +1,11 @@ -import * as path from 'path'; -import type { Request } from 'express'; -import { caching } from 'cache-manager'; -import { getConfig } from './config.js'; +import * as path from "path"; +import type { Request } from "express"; +import { caching } from "cache-manager"; +import { getConfig } from "./config.js"; import { getLogger } from "./logger.js"; -import { User, UserSettings } from "./types"; +import { UserSettings } from "./types"; +import { getUserFromCookie } from "./user.js"; +import { isSubfolder } from "./path.js"; const config = getConfig(); const logger = getLogger(); @@ -11,13 +13,11 @@ const logger = getLogger(); // cache TTL in milliseconds const ttl = config.cacheExpirationTime * 1000; -const cookiesCache = await caching('memory', { ttl }); -const settingsCache = await caching('memory', { ttl }); -const viewerPathsCache = await caching('memory', { ttl }); +const settingsCache = await caching("memory", { ttl }); +const viewerPathsCache = await caching("memory", { ttl }); // Track the cookies for which we are retrieving the user info from fractal-server // Used to avoid querying the cache while the fetch call is in progress -let loadingCookies: string[] = []; let loadingSettings: string[] = []; let loadingViewerPaths: string[] = []; @@ -26,115 +26,61 @@ let loadingViewerPaths: string[] = []; */ export function getAuthorizer() { switch (config.authorizationScheme) { - case 'fractal-server-viewer-paths': + case "fractal-server-viewer-paths": return new ViewerPathsAuthorizer(); - case 'user-folders': + case "user-folders": return new UserFoldersAuthorizer(); - case 'none': - logger.warn('Authorization scheme is set to "none": everybody will be able to access the file. Do not use in production!'); + case "none": + logger.warn( + 'Authorization scheme is set to "none": everybody will be able to access the file. Do not use in production!' + ); return new NoneAuthorizer(); default: - logger.error('Unsupported authorization scheme %s', config.authorizationScheme); + logger.error( + "Unsupported authorization scheme %s", + config.authorizationScheme + ); process.exit(1); } } -abstract class BaseAuthorizer { - +export interface Authorizer { /** - * Returns the requested file path if authorized, undefined otherwise. + * Returns true if the request comes from a valid user, false otherwise. */ - async getAuthorizedPath(req: Request): Promise { - const completePath = this.getValidPath(req); - if (!completePath) { - return; - } - const user = await this.getUserFromCookie(req); - const authorized = await this.isUserAuthorized(completePath, user, req.get('Cookie')); - if (!authorized) { - return undefined; - } - logger.trace("Path to load: %s", completePath); - return completePath; - } - - getValidPath(req: Request): string | undefined { - const requestPath = decodeURIComponent(req.path).normalize(); - if (!config.zarrDataBasePath) { - return requestPath; - } - // Ensure that the selected path is a subfolder of the base data folder - if (this.isSubfolder(config.zarrDataBasePath, requestPath)) { - return requestPath; - } - return undefined; - } + isUserValid(req: Request): Promise; /** - * Ensures that a path to check is a subfolder of a given parent folder. + * Returns true if the user is authorized to read the file path passed as first argument, false otherwise. */ - isSubfolder(parentFolder: string, pathToCheck: string): boolean { - const result = !path.relative(parentFolder, pathToCheck).includes('..'); - if (!result) { - logger.warn('Path "%s" is not a subfolder of "%s"', pathToCheck, parentFolder); - } - return result; - } - - abstract isUserAuthorized(completePath: string, user: User | undefined, cookie: string | undefined): Promise; + isUserAuthorized(completePath: string, req: Request): Promise; +} - async getUserFromCookie(req: Request): Promise { - const cookie = req.get('Cookie'); - if (!cookie) { - logger.debug("Missing cookie header"); - return undefined; - } - while (loadingCookies.includes(cookie)) { - // a fetch call for this cookie is in progress; wait for its completion - await new Promise(r => setTimeout(r)); - } - loadingCookies.push(cookie); - let user: User | undefined = undefined; - try { - const value: string | undefined = await cookiesCache.get(cookie); - if (value) { - user = JSON.parse(value); - } else { - logger.trace("Retrieving user from cookie"); - const response = await fetch(`${config.fractalServerUrl}/auth/current-user/`, { - headers: { - 'Cookie': cookie - } - }); - if (response.ok) { - user = await response.json() as User; - logger.trace("Retrieved user %s", user.email); - cookiesCache.set(cookie, JSON.stringify(user)); - } else { - logger.debug("Fractal server replied with %d while retrieving user from cookie", response.status); - } - } - } finally { - loadingCookies = loadingCookies.filter(c => c !== cookie); - } - return user; +export class NoneAuthorizer implements Authorizer { + async isUserValid(): Promise { + return true; } -} -export class NoneAuthorizer extends BaseAuthorizer { async isUserAuthorized(): Promise { return true; } } -export class UserFoldersAuthorizer extends BaseAuthorizer { - async isUserAuthorized(completePath: string, user: User | undefined, cookie: string | undefined): Promise { +export class UserFoldersAuthorizer implements Authorizer { + async isUserValid(req: Request): Promise { + const user = await getUserFromCookie(req.get("Cookie")); + return !!user; + } + + async isUserAuthorized(completePath: string, req: Request): Promise { + const cookie = req.get("Cookie"); + const user = await getUserFromCookie(cookie); if (!user || !cookie) { return false; } while (loadingSettings.includes(cookie)) { // a fetch call for this cookie is in progress; wait for its completion - await new Promise(r => setTimeout(r)); + await new Promise((r) => setTimeout(r)); } loadingSettings.push(cookie); let settings: UserSettings | undefined = undefined; @@ -144,22 +90,28 @@ export class UserFoldersAuthorizer extends BaseAuthorizer { settings = JSON.parse(value) as UserSettings; } else { logger.trace("Retrieving settings from cookie"); - const response = await fetch(`${config.fractalServerUrl}/auth/current-user/settings/`, { - headers: { - 'Cookie': cookie + const response = await fetch( + `${config.fractalServerUrl}/auth/current-user/settings/`, + { + headers: { + Cookie: cookie, + }, } - }); + ); if (response.ok) { - settings = await response.json() as UserSettings; + settings = (await response.json()) as UserSettings; logger.trace("Retrieved settings for user %s", user.email); settingsCache.set(cookie, JSON.stringify(settings)); } else { - logger.debug("Fractal server replied with %d while retrieving settings from cookie", response.status); + logger.debug( + "Fractal server replied with %d while retrieving settings from cookie", + response.status + ); return false; } } } finally { - loadingSettings = loadingSettings.filter(c => c !== cookie); + loadingSettings = loadingSettings.filter((c) => c !== cookie); } const username = settings.slurm_user; if (!username) { @@ -167,38 +119,58 @@ export class UserFoldersAuthorizer extends BaseAuthorizer { return false; } const userPath = path.join(config.zarrDataBasePath!, username); - if (!this.isSubfolder(config.zarrDataBasePath!, userPath)) { + if (!isSubfolder(config.zarrDataBasePath!, userPath)) { return false; } return completePath.startsWith(userPath); } } -export class ViewerPathsAuthorizer extends BaseAuthorizer { - async isUserAuthorized(completePath: string, user: User | undefined, cookie: string | undefined): Promise { +export class ViewerPathsAuthorizer implements Authorizer { + async isUserValid(req: Request): Promise { + const user = await getUserFromCookie(req.get("Cookie")); + return !!user; + } + + async isUserAuthorized(completePath: string, req: Request): Promise { + const cookie = req.get("Cookie"); + const user = await getUserFromCookie(cookie); if (!user || !cookie) { return false; } while (loadingViewerPaths.includes(cookie)) { // a fetch call for this cookie is in progress; wait for its completion - await new Promise(r => setTimeout(r)); + await new Promise((r) => setTimeout(r)); } loadingViewerPaths.push(cookie); try { - let viewerPaths: string[] | undefined = await viewerPathsCache.get(cookie); + let viewerPaths: string[] | undefined = await viewerPathsCache.get( + cookie + ); if (viewerPaths === undefined) { - logger.trace('Retrieving viewer paths for user %s', user.email); - const response = await fetch(`${config.fractalServerUrl}/auth/current-user/viewer-paths/`, { - headers: { - 'Cookie': cookie + logger.trace("Retrieving viewer paths for user %s", user.email); + const response = await fetch( + `${config.fractalServerUrl}/auth/current-user/viewer-paths/`, + { + headers: { + Cookie: cookie, + }, } - }); + ); if (response.ok) { - viewerPaths = await response.json() as string[]; - logger.trace('Retrieved %d viewer paths for user %s', viewerPaths.length, user.email); + viewerPaths = (await response.json()) as string[]; + logger.trace( + "Retrieved %d viewer paths for user %s", + viewerPaths.length, + user.email + ); viewerPathsCache.set(cookie, viewerPaths); } else { - logger.debug('Fractal server replied with %d while retrieving viewer paths for user %s', response.status, user.email); + logger.debug( + "Fractal server replied with %d while retrieving viewer paths for user %s", + response.status, + user.email + ); return false; } } @@ -207,10 +179,10 @@ export class ViewerPathsAuthorizer extends BaseAuthorizer { return true; } } - logger.trace('Unauthorized path %s', completePath); + logger.trace("Unauthorized path %s", completePath); return false; } finally { - loadingViewerPaths = loadingViewerPaths.filter(c => c !== cookie); + loadingViewerPaths = loadingViewerPaths.filter((c) => c !== cookie); } } } diff --git a/src/data.ts b/src/data.ts new file mode 100644 index 0000000..13782e0 --- /dev/null +++ b/src/data.ts @@ -0,0 +1,47 @@ +import * as fs from "fs"; +import type { Request, Response } from "express"; +import { getValidPath } from "./path.js"; +import { getConfig } from "./config.js"; +import { getLogger } from "./logger.js"; +import { Authorizer } from "./authorizer.js"; + +const config = getConfig(); +const logger = getLogger(); + +export async function serveZarrData( + authorizer: Authorizer, + req: Request, + res: Response +) { + try { + const completePath = getValidPath(req, config); + if (!completePath) { + logger.info("Invalid path: %s", req.path.normalize()); + return res.status(404).send("Not Found").end(); + } + const validUser = await authorizer.isUserValid(req); + if (!validUser) { + logger.info("Unauthorized request: %s", req.path.normalize()); + return res.status(401).send("Unauthorized").end(); + } + const authorized = await authorizer.isUserAuthorized(completePath, req); + if (!authorized) { + logger.info("Forbidden request: %s", req.path.normalize()); + return res.status(403).send("Forbidden").end(); + } + if (!fs.existsSync(completePath)) { + logger.info("File not found: %s", completePath); + return res.status(404).send("Not Found").end(); + } + if (fs.lstatSync(completePath).isDirectory()) { + logger.info("Path is directory: %s", completePath); + return res.status(400).send("Is directory").end(); + } + logger.trace("Path to load: %s", completePath); + const stream = fs.createReadStream(completePath); + stream.pipe(res); + } catch (err) { + logger.error("Error reading file", err); + return res.status(500).send("Internal Server Error").end(); + } +} diff --git a/src/path.ts b/src/path.ts new file mode 100644 index 0000000..2bb795f --- /dev/null +++ b/src/path.ts @@ -0,0 +1,29 @@ +import * as path from 'path'; +import type { Request } from 'express'; +import { getLogger } from './logger.js'; +import { Config } from './types'; + +const logger = getLogger(); + +export function getValidPath(req: Request, config: Config): string | undefined { + const requestPath = decodeURIComponent(req.path).normalize(); + if (!config.zarrDataBasePath) { + return requestPath; + } + // Ensure that the selected path is a subfolder of the base data folder + if (isSubfolder(config.zarrDataBasePath, requestPath)) { + return requestPath; + } + return undefined; +} + +/** + * Ensures that a path to check is a subfolder of a given parent folder. + */ +export function isSubfolder(parentFolder: string, pathToCheck: string): boolean { + const result = !path.relative(parentFolder, pathToCheck).includes('..'); + if (!result) { + logger.warn('Path "%s" is not a subfolder of "%s"', pathToCheck, parentFolder); + } + return result; +} diff --git a/src/user.ts b/src/user.ts new file mode 100644 index 0000000..789bf39 --- /dev/null +++ b/src/user.ts @@ -0,0 +1,52 @@ +import { caching } from 'cache-manager'; +import { getConfig } from "./config.js"; +import { getLogger } from "./logger.js"; +import { User } from './types'; + +const config = getConfig(); +const logger = getLogger(); + +// cache TTL in milliseconds +const ttl = config.cacheExpirationTime * 1000; + +const cookiesCache = await caching('memory', { ttl }); + +// Track the cookies for which we are retrieving the user info from fractal-server +// Used to avoid querying the cache while the fetch call is in progress +let loadingCookies: string[] = []; + +export async function getUserFromCookie(cookie: string | undefined): Promise { + if (!cookie) { + logger.debug("Missing cookie header"); + return undefined; + } + while (loadingCookies.includes(cookie)) { + // a fetch call for this cookie is in progress; wait for its completion + await new Promise(r => setTimeout(r)); + } + loadingCookies.push(cookie); + let user: User | undefined = undefined; + try { + const value: string | undefined = await cookiesCache.get(cookie); + if (value) { + user = JSON.parse(value); + } else { + logger.trace("Retrieving user from cookie"); + const response = await fetch(`${config.fractalServerUrl}/auth/current-user/`, { + headers: { + 'Cookie': cookie + } + }); + if (response.ok) { + user = await response.json() as User; + logger.trace("Retrieved user %s", user.email); + cookiesCache.set(cookie, JSON.stringify(user)); + } else { + logger.debug("Fractal server replied with %d while retrieving user from cookie", response.status); + } + } + } finally { + loadingCookies = loadingCookies.filter(c => c !== cookie); + } + return user; +} diff --git a/test/app.spec.ts b/test/app.spec.ts new file mode 100644 index 0000000..eccad22 --- /dev/null +++ b/test/app.spec.ts @@ -0,0 +1,83 @@ +import { describe, it, expect, vi, beforeAll, afterAll } from "vitest"; +import { getMockedResponse, getMockedRequest, mockConfig } from "./mock"; +import fs from "fs"; +import os from "os"; +import path from "path"; + +vi.mock("../src/config.js", () => { + return mockConfig({ + authorizationScheme: "user-folders", + zarrDataBasePath: fs.mkdtempSync( + path.join(os.tmpdir(), "fractal-vizarr-app-test") + ), + }); +}); + +import { serveZarrData } from "../src/data"; +import { Authorizer } from "../src/authorizer"; +import { getConfig } from "../src/config"; + +describe("Serving vizarr data", () => { + const config = getConfig(); + const tmpDir = config.zarrDataBasePath!; + + beforeAll(() => { + // Create test files + fs.mkdirSync(path.join(tmpDir, "directory")); + }); + + afterAll(() => { + fs.rmSync(tmpDir, { recursive: true }); + }); + + it("Invalid path request", async () => { + const request = getMockedRequest(`${tmpDir}/../invalid/path`); + const response = getMockedResponse(); + const authorizer = mockAuthorizer(true, true); + await serveZarrData(authorizer, request, response); + expect(response.status).toHaveBeenCalledWith(404); + }); + + it("Unauthorized request", async () => { + const request = getMockedRequest(`${tmpDir}/test1`); + const response = getMockedResponse(); + const authorizer = mockAuthorizer(false, true); + await serveZarrData(authorizer, request, response); + expect(response.status).toHaveBeenCalledWith(401); + }); + + it("Forbidden request", async () => { + const request = getMockedRequest(`${tmpDir}/test1`); + const response = getMockedResponse(); + const authorizer = mockAuthorizer(true, false); + await serveZarrData(authorizer, request, response); + expect(response.status).toHaveBeenCalledWith(403); + }); + + it("File not found", async () => { + const request = getMockedRequest(`${tmpDir}/test2`); + const response = getMockedResponse(); + const authorizer = mockAuthorizer(true, true); + await serveZarrData(authorizer, request, response); + expect(response.status).toHaveBeenCalledWith(404); + }); + + it("File is directory", async () => { + const request = getMockedRequest(`${tmpDir}/directory`); + const response = getMockedResponse(); + const authorizer = mockAuthorizer(true, true); + await serveZarrData(authorizer, request, response); + expect(response.status).toHaveBeenCalledWith(400); + }); +}); + +function mockAuthorizer(valid: boolean, authorized: boolean): Authorizer { + return { + async isUserValid() { + return valid; + }, + async isUserAuthorized() { + return authorized; + }, + }; +} diff --git a/test/authorizer/authorizer-mocks.ts b/test/authorizer/authorizer-mocks.ts deleted file mode 100644 index 88e6245..0000000 --- a/test/authorizer/authorizer-mocks.ts +++ /dev/null @@ -1,22 +0,0 @@ -import type { Request } from "express"; -import { Config } from "../../src/types"; - -export function mockConfig(config: Partial) { - const getConfig = () => ({ - fractalServerUrl: "http://localhost:8000", - zarrDataBasePath: "/path/to/zarr/data", - authorizationScheme: config.authorizationScheme, - }); - return { - getConfig, - }; -} - -export function getMockedRequest(path: string, cookie: string | undefined) { - return { - path, - get: () => { - return cookie; - }, - } as unknown as Request; -} diff --git a/test/authorizer/none-authorizer.spec.ts b/test/authorizer/none-authorizer.spec.ts deleted file mode 100644 index c1c13fc..0000000 --- a/test/authorizer/none-authorizer.spec.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { describe, it, beforeEach, expect, vi } from "vitest"; -import { getMockedRequest, mockConfig } from "./authorizer-mocks.js"; - -vi.mock("../../src/config.js", () => { - return mockConfig({ - authorizationScheme: "none", - }); -}); - -// Mocking fetch -const fetch = vi.fn(); -global.fetch = fetch; - -import { getAuthorizer } from "../../src/authorizer.js"; -const authorizer = getAuthorizer(); - -describe("None authorizer", () => { - beforeEach(() => { - fetch.mockClear(); - }); - - it("Registered user with valid absolute path", async () => { - fetch.mockResolvedValueOnce({ - ok: true, - status: 200, - json: () => - new Promise((resolve) => resolve({ email: "admin@example.com" })), - }); - const request = getMockedRequest( - "/path/to/zarr/data/foo/bar", - "cookie-user-1" - ); - const path = await authorizer.getAuthorizedPath(request); - expect(path).eq("/path/to/zarr/data/foo/bar"); - }); - - it("Allowed user with invalid path", async () => { - const request = getMockedRequest("../foo/bar", "cookie-user-1"); - const path = await authorizer.getAuthorizedPath(request); - expect(path).eq(undefined); - }); - - it("Path with URL encoded characters", async () => { - const request = getMockedRequest( - "/path/to/zarr/data/foo/bar%23baz", - undefined - ); - const path = await authorizer.getAuthorizedPath(request); - expect(path).eq("/path/to/zarr/data/foo/bar#baz"); - }); -}); diff --git a/test/authorizer/user-folders-authorizer.spec.ts b/test/authorizer/user-folders-authorizer.spec.ts index 06f9997..aa5305b 100644 --- a/test/authorizer/user-folders-authorizer.spec.ts +++ b/test/authorizer/user-folders-authorizer.spec.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from "vitest"; -import { getMockedRequest, mockConfig } from "./authorizer-mocks.js"; +import { getMockedRequest, mockConfig } from "../mock"; vi.mock("../../src/config.js", () => { return mockConfig({ @@ -32,8 +32,13 @@ describe("User folders authorizer", () => { "/path/to/zarr/data/admin/foo/bar", "cookie-user-1" ); - const path = await authorizer.getAuthorizedPath(request); - expect(path).eq("/path/to/zarr/data/admin/foo/bar"); + const validUser = await authorizer.isUserValid(request); + const authorizedUser = await authorizer.isUserAuthorized( + "/path/to/zarr/data/admin/foo/bar", + request + ); + expect(validUser).toBeTruthy(); + expect(authorizedUser).toBeTruthy(); }); it("Registered user with path of another user", async () => { @@ -53,23 +58,34 @@ describe("User folders authorizer", () => { "/path/to/zarr/data/admin/foo/bar", "cookie-user-2" ); - const path = await authorizer.getAuthorizedPath(request); - expect(path).eq(undefined); - }); - - it("Registered user with invalid path", async () => { - const request = getMockedRequest("../foo/bar", "cookie-user-1"); - const path = await authorizer.getAuthorizedPath(request); - expect(path).eq(undefined); + const validUser = await authorizer.isUserValid(request); + const authorizedUser = await authorizer.isUserAuthorized( + "/path/to/zarr/data/admin/foo/bar", + request + ); + expect(validUser).toBeTruthy(); + expect(authorizedUser).toBeFalsy(); }); it("/auth/current-user/settings/ returns error", async () => { - fetch.mockResolvedValueOnce({ - ok: false, - status: 500, - }); + fetch + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: () => + new Promise((resolve) => resolve({ email: "user3@example.com" })), + }) + .mockResolvedValueOnce({ + ok: false, + status: 500, + }); const request = getMockedRequest("/user2/foo/bar", "cookie-user-3"); - const path = await authorizer.getAuthorizedPath(request); - expect(path).eq(undefined); + const validUser = await authorizer.isUserValid(request); + const authorizedUser = await authorizer.isUserAuthorized( + "/user2/foo/bar", + request + ); + expect(validUser).toBeTruthy(); + expect(authorizedUser).toBeFalsy(); }); }); diff --git a/test/authorizer/viewer-paths-authorizer.spec.ts b/test/authorizer/viewer-paths-authorizer.spec.ts index 357caaf..219fdec 100644 --- a/test/authorizer/viewer-paths-authorizer.spec.ts +++ b/test/authorizer/viewer-paths-authorizer.spec.ts @@ -1,5 +1,5 @@ import { describe, it, beforeEach, expect, vi } from "vitest"; -import { getMockedRequest, mockConfig } from "./authorizer-mocks.js"; +import { getMockedRequest, mockConfig } from "../mock"; vi.mock("../../src/config.js", () => { return mockConfig({ @@ -39,29 +39,35 @@ describe("Viewer paths authorizer", () => { "/path/to/zarr/data/foo/xxx", "cookie-user-1" ); - const path = await authorizer.getAuthorizedPath(request); - expect(path).eq("/path/to/zarr/data/foo/xxx"); + const validUser = await authorizer.isUserValid(request); + const authorizedUser = await authorizer.isUserAuthorized( + "/path/to/zarr/data/foo/xxx", + request + ); + expect(validUser).toBeTruthy(); + expect(authorizedUser).toBeTruthy(); }); it("Allowed user with forbidden path", async () => { const request = getMockedRequest("/path/to/forbidden", "cookie-user-1"); - const path = await authorizer.getAuthorizedPath(request); - expect(path).eq(undefined); - }); - - it("Allowed user with forbidden relative path", async () => { - const request = getMockedRequest( - "/path/to/zarr/data/foo/xxx/../../forbidden", - "cookie-user-1" + const validUser = await authorizer.isUserValid(request); + const authorizedUser = await authorizer.isUserAuthorized( + "/path/to/forbidden", + request ); - const path = await authorizer.getAuthorizedPath(request); - expect(path).eq(undefined); + expect(validUser).toBeTruthy(); + expect(authorizedUser).toBeFalsy(); }); it("Anonymous user with valid path", async () => { const request = getMockedRequest("/path/to/zarr/data/foo/xxx", undefined); - const path = await authorizer.getAuthorizedPath(request); - expect(path).eq(undefined); + const validUser = await authorizer.isUserValid(request); + const authorizedUser = await authorizer.isUserAuthorized( + "/path/to/zarr/data/foo/xxx", + request + ); + expect(validUser).toBeFalsy(); + expect(authorizedUser).toBeFalsy(); }); it("/auth/current-user/viewer-paths/ returns error", async () => { @@ -81,7 +87,12 @@ describe("Viewer paths authorizer", () => { "/path/to/zarr/data/foo/xxx", "cookie-user-3" ); - const path = await authorizer.getAuthorizedPath(request); - expect(path).eq(undefined); + const validUser = await authorizer.isUserValid(request); + const authorizedUser = await authorizer.isUserAuthorized( + "/path/to/zarr/data/foo/xxx", + request + ); + expect(validUser).toBeTruthy(); + expect(authorizedUser).toBeFalsy(); }); }); diff --git a/test/mock.ts b/test/mock.ts new file mode 100644 index 0000000..bd4efe8 --- /dev/null +++ b/test/mock.ts @@ -0,0 +1,36 @@ +import type { Request, Response } from "express"; +import { Config } from "../src/types"; +import { vi } from "vitest"; + +export function mockConfig(config: Partial) { + const getConfig = () => + ({ + basePath: "/vizarr/", + fractalServerUrl: "http://localhost:8000", + zarrDataBasePath: "/path/to/zarr/data", + ...config, + } as Config); + return { + getConfig, + }; +} + +export function getMockedResponse() { + return { + status: vi.fn().mockReturnThis(), + send: vi.fn().mockReturnThis(), + end: vi.fn(), + } as unknown as Response; +} + +export function getMockedRequest( + path: string, + cookie: string | undefined = undefined +) { + return { + path, + get: () => { + return cookie; + }, + } as unknown as Request; +} diff --git a/test/path.spec.ts b/test/path.spec.ts new file mode 100644 index 0000000..72e01cd --- /dev/null +++ b/test/path.spec.ts @@ -0,0 +1,29 @@ +import { describe, it, expect } from "vitest"; +import { getMockedRequest, mockConfig } from "./mock"; +import { getValidPath } from "../src/path"; + +describe("Path utilities", () => { + it("valid path with URL encoded characters - no zarrDataBasePath", async () => { + const { getConfig } = mockConfig({ zarrDataBasePath: null }); + const config = getConfig(); + const request = getMockedRequest("/path/to/bar%23baz"); + const path = getValidPath(request, config); + expect(path).toEqual("/path/to/bar#baz"); + }); + + it("valid path with zarrDataBasePath", async () => { + const { getConfig } = mockConfig({ zarrDataBasePath: "/base/zarr/path" }); + const config = getConfig(); + const request = getMockedRequest("/base/zarr/path/valid"); + const path = getValidPath(request, config); + expect(path).toEqual("/base/zarr/path/valid"); + }); + + it("invalid path with zarrDataBasePath", async () => { + const { getConfig } = mockConfig({ zarrDataBasePath: "/base/zarr/path" }); + const config = getConfig(); + const request = getMockedRequest("/base/zarr/path/../invalid"); + const path = getValidPath(request, config); + expect(path).toEqual(undefined); + }); +}); From 7827bf993f4dcceee7322e87420c8e2eed78b93a Mon Sep 17 00:00:00 2001 From: Sonia Zorba Date: Wed, 6 Nov 2024 11:23:42 +0100 Subject: [PATCH 2/5] Included user_settings.project_dir in list of allowed paths --- src/authorizer.ts | 89 +++++++++++-------- src/types.ts | 1 + .../viewer-paths-authorizer.spec.ts | 47 +++++++++- 3 files changed, 99 insertions(+), 38 deletions(-) diff --git a/src/authorizer.ts b/src/authorizer.ts index 3bcc39e..9cd4c91 100644 --- a/src/authorizer.ts +++ b/src/authorizer.ts @@ -3,7 +3,7 @@ import type { Request } from "express"; import { caching } from "cache-manager"; import { getConfig } from "./config.js"; import { getLogger } from "./logger.js"; -import { UserSettings } from "./types"; +import { User, UserSettings } from "./types"; import { getUserFromCookie } from "./user.js"; import { isSubfolder } from "./path.js"; @@ -78,40 +78,9 @@ export class UserFoldersAuthorizer implements Authorizer { if (!user || !cookie) { return false; } - while (loadingSettings.includes(cookie)) { - // a fetch call for this cookie is in progress; wait for its completion - await new Promise((r) => setTimeout(r)); - } - loadingSettings.push(cookie); - let settings: UserSettings | undefined = undefined; - try { - const value: string | undefined = await settingsCache.get(cookie); - if (value) { - settings = JSON.parse(value) as UserSettings; - } else { - logger.trace("Retrieving settings from cookie"); - const response = await fetch( - `${config.fractalServerUrl}/auth/current-user/settings/`, - { - headers: { - Cookie: cookie, - }, - } - ); - if (response.ok) { - settings = (await response.json()) as UserSettings; - logger.trace("Retrieved settings for user %s", user.email); - settingsCache.set(cookie, JSON.stringify(settings)); - } else { - logger.debug( - "Fractal server replied with %d while retrieving settings from cookie", - response.status - ); - return false; - } - } - } finally { - loadingSettings = loadingSettings.filter((c) => c !== cookie); + const settings = await getUserSettings(user, cookie); + if (!settings) { + return false; } const username = settings.slurm_user; if (!username) { @@ -138,6 +107,7 @@ export class ViewerPathsAuthorizer implements Authorizer { if (!user || !cookie) { return false; } + const settings = await getUserSettings(user, cookie); while (loadingViewerPaths.includes(cookie)) { // a fetch call for this cookie is in progress; wait for its completion await new Promise((r) => setTimeout(r)); @@ -174,8 +144,12 @@ export class ViewerPathsAuthorizer implements Authorizer { return false; } } - for (const viewerPath of viewerPaths) { - if (path.resolve(completePath).startsWith(viewerPath)) { + const allowedPaths = + settings && settings.project_dir + ? [settings.project_dir, ...viewerPaths] + : viewerPaths; + for (const allowedPath of allowedPaths) { + if (path.resolve(completePath).startsWith(allowedPath)) { return true; } } @@ -186,3 +160,44 @@ export class ViewerPathsAuthorizer implements Authorizer { } } } + +async function getUserSettings( + user: User, + cookie: string +): Promise { + while (loadingSettings.includes(cookie)) { + // a fetch call for this cookie is in progress; wait for its completion + await new Promise((r) => setTimeout(r)); + } + loadingSettings.push(cookie); + try { + const value: string | undefined = await settingsCache.get(cookie); + if (value) { + return JSON.parse(value) as UserSettings; + } else { + logger.trace("Retrieving settings from cookie"); + const response = await fetch( + `${config.fractalServerUrl}/auth/current-user/settings/`, + { + headers: { + Cookie: cookie, + }, + } + ); + if (response.ok) { + const settings = (await response.json()) as UserSettings; + logger.trace("Retrieved settings for user %s", user.email); + settingsCache.set(cookie, JSON.stringify(settings)); + return settings; + } else { + logger.debug( + "Fractal server replied with %d while retrieving settings from cookie", + response.status + ); + return undefined; + } + } + } finally { + loadingSettings = loadingSettings.filter((c) => c !== cookie); + } +} diff --git a/src/types.ts b/src/types.ts index 1632bfb..389c91c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -22,5 +22,6 @@ export type User = { export type UserSettings = { slurm_user: string | null cache_dir: string | null + project_dir: string | null slurm_accounts: string[] } diff --git a/test/authorizer/viewer-paths-authorizer.spec.ts b/test/authorizer/viewer-paths-authorizer.spec.ts index 219fdec..73ad286 100644 --- a/test/authorizer/viewer-paths-authorizer.spec.ts +++ b/test/authorizer/viewer-paths-authorizer.spec.ts @@ -19,7 +19,7 @@ describe("Viewer paths authorizer", () => { fetch.mockClear(); }); - it("Allowed user with valid path", async () => { + it("Allowed user with valid path in zarr directory list", async () => { fetch .mockResolvedValueOnce({ ok: true, @@ -27,6 +27,14 @@ describe("Viewer paths authorizer", () => { json: () => new Promise((resolve) => resolve({ email: "admin@example.com" })), }) + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: () => + new Promise((resolve) => + resolve({ project_dir: "/path/to/project" }) + ), + }) .mockResolvedValueOnce({ ok: true, status: 200, @@ -48,6 +56,38 @@ describe("Viewer paths authorizer", () => { expect(authorizedUser).toBeTruthy(); }); + it("Allowed user with valid path in zarr project dir", async () => { + fetch + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: () => + new Promise((resolve) => resolve({ email: "user2@example.com" })), + }) + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: () => + new Promise((resolve) => + resolve({ project_dir: "/path/to/project" }) + ), + }) + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: () => + new Promise((resolve) => resolve(["/path/to/zarr/data/bar"])), + }); + const request = getMockedRequest("/path/to/project/xxx", "cookie-user-2"); + const validUser = await authorizer.isUserValid(request); + const authorizedUser = await authorizer.isUserAuthorized( + "/path/to/project/xxx", + request + ); + expect(validUser).toBeTruthy(); + expect(authorizedUser).toBeTruthy(); + }); + it("Allowed user with forbidden path", async () => { const request = getMockedRequest("/path/to/forbidden", "cookie-user-1"); const validUser = await authorizer.isUserValid(request); @@ -78,6 +118,11 @@ describe("Viewer paths authorizer", () => { json: () => new Promise((resolve) => resolve({ email: "user3@example.com" })), }) + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: () => new Promise((resolve) => resolve({ project_dir: null })), + }) .mockResolvedValueOnce({ ok: false, status: 400, From aeff7ba3de3893c5e45dfc483397f80e993cd09a Mon Sep 17 00:00:00 2001 From: Sonia Zorba Date: Wed, 6 Nov 2024 12:29:11 +0100 Subject: [PATCH 3/5] Supported AUTHORIZATION_SCHEME="testing-basic-auth" --- README.md | 4 +- src/authorizer.ts | 26 ++++++++ src/config.ts | 69 +++++++++++++------- src/data.ts | 8 +++ src/types.ts | 48 ++++++++------ test/authorizer/testing-basic-auth.spec.ts | 74 ++++++++++++++++++++++ 6 files changed, 183 insertions(+), 46 deletions(-) create mode 100644 test/authorizer/testing-basic-auth.spec.ts diff --git a/README.md b/README.md index 4c7d58f..3da1ba3 100644 --- a/README.md +++ b/README.md @@ -61,8 +61,9 @@ To start the application installed in this way see the section [Run fractal-viza * `VIZARR_STATIC_FILES_PATH`: path to the files generated running `npm run build` in vizarr source folder; * `BASE_PATH`: base path of fractal-vizarr-viewer application; * `AUTHORIZATION_SCHEME`: defines how the service verifies user authorization. The following options are available: - * `fractal-server-viewer-paths`: the paths that can be accessed by each user are retrieved calling fractal-server API; + * `fractal-server-viewer-paths`: the paths that can be accessed by each user are retrieved calling fractal-server API. * `user-folders`: each registered user can only access their own folder, which corresponds to a directory under `ZARR_DATA_BASE_PATH` named as their `slurm_user` field. + * `testing-basic-auth`: enables Basic Authentication for testing purposes. The credentials are specified through two additional environment variables: `TESTING_USERNAME` and `TESTING_PASSWORD`. This option should not be used in production environments. * `none`: no authorization checks are performed, allowing access to all users, including anonymous ones. This option is useful for demonstrations and testing but should not be used in production environments. * `CACHE_EXPIRATION_TIME`: cookie cache TTL in seconds; when user info is retrieved from a cookie calling the current user endpoint on fractal-server the information is cached for the specified amount of seconds, to reduce the number of calls to fractal-server; * `LOG_LEVEL_CONSOLE`: the log level of logs that will be written to the console; the default value is `info`; @@ -210,7 +211,6 @@ The following command can be used to start the docker image for testing: ```sh docker run --network host \ -v /path/to/zarr-files:/zarr-files \ - -e ZARR_DATA_BASE_PATH=/zarr-files \ -e FRACTAL_SERVER_URL=http://localhost:8000 \ -e AUTHORIZATION_SCHEME=fractal-server-viewer-paths \ fractal-vizarr-viewer diff --git a/src/authorizer.ts b/src/authorizer.ts index 9cd4c91..ede33c0 100644 --- a/src/authorizer.ts +++ b/src/authorizer.ts @@ -35,6 +35,11 @@ export function getAuthorizer() { 'Authorization scheme is set to "none": everybody will be able to access the file. Do not use in production!' ); return new NoneAuthorizer(); + case "testing-basic-auth": + logger.warn( + 'Authorization scheme is set to "testing-basic-auth". Do not use in production!' + ); + return new TestingBasicAuthAuthorizer(); default: logger.error( "Unsupported authorization scheme %s", @@ -201,3 +206,24 @@ async function getUserSettings( loadingSettings = loadingSettings.filter((c) => c !== cookie); } } + +export class TestingBasicAuthAuthorizer implements Authorizer { + async isUserValid(req: Request): Promise { + const authHeader = req.get("Authorization"); + return !!authHeader; + } + + async isUserAuthorized(_: string, req: Request): Promise { + const authHeader = req.get("Authorization")!; + const [scheme, credentials] = authHeader.split(" "); + if (scheme !== "Basic" || !credentials) { + return false; + } + const [username, password] = Buffer.from(credentials, "base64") + .toString() + .split(":"); + return ( + username === config.testingUsername && password === config.testingPassword + ); + } +} diff --git a/src/config.ts b/src/config.ts index 99f7b4e..ca43008 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1,6 +1,6 @@ -import * as dotenv from 'dotenv'; +import * as dotenv from "dotenv"; import { getLogger } from "./logger.js"; -import { AuthorizationScheme, Config } from './types'; +import { AuthorizationScheme, Config } from "./types"; // Loading environment variables dotenv.config(); @@ -10,7 +10,10 @@ const logger = getLogger(); function getRequiredEnv(envName: string) { const value = process.env[envName]; if (!value) { - logger.error('Missing required environment variable %s. Check the configuration.', envName); + logger.error( + "Missing required environment variable %s. Check the configuration.", + envName + ); process.exit(1); } return value; @@ -20,43 +23,61 @@ function getRequiredEnv(envName: string) { * @returns the service configuration */ function loadConfig(): Config { - const port = process.env.PORT ? parseInt(process.env.PORT) : 3000; - const fractalServerUrl = getRequiredEnv('FRACTAL_SERVER_URL'); - const vizarrStaticFilesPath = getRequiredEnv('VIZARR_STATIC_FILES_PATH'); + const fractalServerUrl = getRequiredEnv("FRACTAL_SERVER_URL"); + const vizarrStaticFilesPath = getRequiredEnv("VIZARR_STATIC_FILES_PATH"); - const validAuthorizationSchemes = ['fractal-server-viewer-paths', 'user-folders', 'none']; - const authorizationScheme = getRequiredEnv('AUTHORIZATION_SCHEME'); + const validAuthorizationSchemes = [ + "fractal-server-viewer-paths", + "user-folders", + "testing-basic-auth", + "none", + ]; + const authorizationScheme = getRequiredEnv("AUTHORIZATION_SCHEME"); if (!validAuthorizationSchemes.includes(authorizationScheme)) { - logger.error('Invalid authorization scheme "%s", allowed values: %s', authorizationScheme, - validAuthorizationSchemes.map(v => `"${v}"`).join(', ')); + logger.error( + 'Invalid authorization scheme "%s", allowed values: %s', + authorizationScheme, + validAuthorizationSchemes.map((v) => `"${v}"`).join(", ") + ); process.exit(1); } let zarrDataBasePath: string | null = null; - if (authorizationScheme !== 'fractal-server-viewer-paths') { - zarrDataBasePath = getRequiredEnv('ZARR_DATA_BASE_PATH'); + if (authorizationScheme !== "fractal-server-viewer-paths") { + zarrDataBasePath = getRequiredEnv("ZARR_DATA_BASE_PATH"); } else if (process.env.ZARR_DATA_BASE_PATH) { - logger.error(`ZARR_DATA_BASE_PATH will be ignored because AUTHORIZATION_SCHEME is set to fractal-server-viewer-paths`); + logger.error( + `ZARR_DATA_BASE_PATH will be ignored because AUTHORIZATION_SCHEME is set to fractal-server-viewer-paths` + ); process.exit(1); } + let testingUsername: string | null = null; + let testingPassword: string | null = null; + if (authorizationScheme === "testing-basic-auth") { + testingUsername = getRequiredEnv("TESTING_USERNAME"); + testingPassword = getRequiredEnv("TESTING_PASSWORD"); + } + // Cookie cache TTL in seconds - const cacheExpirationTime = process.env.CACHE_EXPIRATION_TIME ? parseInt(process.env.CACHE_EXPIRATION_TIME) : 60; + const cacheExpirationTime = process.env.CACHE_EXPIRATION_TIME + ? parseInt(process.env.CACHE_EXPIRATION_TIME) + : 60; - let basePath = process.env.BASE_PATH || '/vizarr'; - if (!basePath.endsWith('/')) { - basePath += '/'; + let basePath = process.env.BASE_PATH || "/vizarr"; + if (!basePath.endsWith("/")) { + basePath += "/"; } - logger.debug('FRACTAL_SERVER_URL: %s', fractalServerUrl); - logger.debug('BASE_PATH: %s', basePath); + logger.debug("FRACTAL_SERVER_URL: %s", fractalServerUrl); + logger.debug("BASE_PATH: %s", basePath); if (zarrDataBasePath) { - logger.debug('ZARR_DATA_BASE_PATH: %s', zarrDataBasePath); + logger.debug("ZARR_DATA_BASE_PATH: %s", zarrDataBasePath); } - logger.debug('VIZARR_STATIC_FILES_PATH: %s', vizarrStaticFilesPath); - logger.debug('AUTHORIZATION_SCHEME: %s', authorizationScheme); - logger.debug('CACHE_EXPIRATION_TIME: %d', cacheExpirationTime); + logger.debug("VIZARR_STATIC_FILES_PATH: %s", vizarrStaticFilesPath); + logger.debug("AUTHORIZATION_SCHEME: %s", authorizationScheme); + logger.debug("CACHE_EXPIRATION_TIME: %d", cacheExpirationTime); return { port, @@ -66,6 +87,8 @@ function loadConfig(): Config { vizarrStaticFilesPath, authorizationScheme: authorizationScheme as AuthorizationScheme, cacheExpirationTime, + testingUsername, + testingPassword, }; } diff --git a/src/data.ts b/src/data.ts index 13782e0..0295daa 100644 --- a/src/data.ts +++ b/src/data.ts @@ -22,11 +22,13 @@ export async function serveZarrData( const validUser = await authorizer.isUserValid(req); if (!validUser) { logger.info("Unauthorized request: %s", req.path.normalize()); + addBasicAuthChallengeIfNeeded(res); return res.status(401).send("Unauthorized").end(); } const authorized = await authorizer.isUserAuthorized(completePath, req); if (!authorized) { logger.info("Forbidden request: %s", req.path.normalize()); + addBasicAuthChallengeIfNeeded(res); return res.status(403).send("Forbidden").end(); } if (!fs.existsSync(completePath)) { @@ -45,3 +47,9 @@ export async function serveZarrData( return res.status(500).send("Internal Server Error").end(); } } + +function addBasicAuthChallengeIfNeeded(res: Response) { + if (config.authorizationScheme === "testing-basic-auth") { + res.set("WWW-Authenticate", 'Basic realm="Testing credentials"'); + } +} diff --git a/src/types.ts b/src/types.ts index 389c91c..34cb235 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,27 +1,33 @@ -export type AuthorizationScheme = 'fractal-server-viewer-paths' | 'user-folders' | 'none'; +export type AuthorizationScheme = + | "fractal-server-viewer-paths" + | "user-folders" + | "testing-basic-auth" + | "none"; export type Config = { - port: number - fractalServerUrl: string - basePath: string - zarrDataBasePath: string | null - vizarrStaticFilesPath: string - authorizationScheme: AuthorizationScheme - cacheExpirationTime: number -} + port: number; + fractalServerUrl: string; + basePath: string; + zarrDataBasePath: string | null; + vizarrStaticFilesPath: string; + authorizationScheme: AuthorizationScheme; + cacheExpirationTime: number; + testingUsername: string | null; + testingPassword: string | null; +}; export type User = { - id: number - email: string - is_active: boolean - is_superuser: boolean - is_verified: boolean - username: string | null -} + id: number; + email: string; + is_active: boolean; + is_superuser: boolean; + is_verified: boolean; + username: string | null; +}; export type UserSettings = { - slurm_user: string | null - cache_dir: string | null - project_dir: string | null - slurm_accounts: string[] -} + slurm_user: string | null; + cache_dir: string | null; + project_dir: string | null; + slurm_accounts: string[]; +}; diff --git a/test/authorizer/testing-basic-auth.spec.ts b/test/authorizer/testing-basic-auth.spec.ts new file mode 100644 index 0000000..09210a5 --- /dev/null +++ b/test/authorizer/testing-basic-auth.spec.ts @@ -0,0 +1,74 @@ +import { describe, it, expect, vi } from "vitest"; +import { mockConfig } from "../mock"; +import type { Request } from "express"; + +vi.mock("../../src/config.js", () => { + return mockConfig({ + authorizationScheme: "testing-basic-auth", + testingUsername: "test", + testingPassword: "password", + }); +}); + +// Mocking fetch +const fetch = vi.fn(); +global.fetch = fetch; + +import { getAuthorizer } from "../../src/authorizer.js"; +const authorizer = getAuthorizer(); + +describe("Testing basic auth authorizer", () => { + it("Missing authorization header", async () => { + const request = mockAuthHeader(undefined); + const validUser = await authorizer.isUserValid(request); + expect(validUser).toBeFalsy(); + }); + + it("Invalid authorization header", async () => { + const request = mockAuthHeader("foo"); + const validUser = await authorizer.isUserValid(request); + const authorizedUser = await authorizer.isUserAuthorized("", request); + expect(validUser).toBeTruthy(); + expect(authorizedUser).toBeFalsy(); + }); + + it("Non Basic authorization header", async () => { + const request = mockAuthHeader("Bearer token"); + const validUser = await authorizer.isUserValid(request); + const authorizedUser = await authorizer.isUserAuthorized("", request); + expect(validUser).toBeTruthy(); + expect(authorizedUser).toBeFalsy(); + }); + + it("Basic authorization header with bad password format", async () => { + const request = mockAuthHeader("Basic not-base-64"); + const validUser = await authorizer.isUserValid(request); + const authorizedUser = await authorizer.isUserAuthorized("", request); + expect(validUser).toBeTruthy(); + expect(authorizedUser).toBeFalsy(); + }); + + it("Basic authorization header with invalid password", async () => { + const request = mockAuthHeader("Basic Zm9vOmJhcg=="); + const validUser = await authorizer.isUserValid(request); + const authorizedUser = await authorizer.isUserAuthorized("", request); + expect(validUser).toBeTruthy(); + expect(authorizedUser).toBeFalsy(); + }); + + it("Basic authorization header with valid password", async () => { + const request = mockAuthHeader("Basic dGVzdDpwYXNzd29yZA=="); + const validUser = await authorizer.isUserValid(request); + const authorizedUser = await authorizer.isUserAuthorized("", request); + expect(validUser).toBeTruthy(); + expect(authorizedUser).toBeTruthy(); + }); +}); + +function mockAuthHeader(header: string | undefined) { + return { + get: () => { + return header; + }, + } as unknown as Request; +} From 6fe735a274edb7bd89eb448456f999ca78bf0a15 Mon Sep 17 00:00:00 2001 From: Sonia Zorba Date: Wed, 6 Nov 2024 13:44:23 +0100 Subject: [PATCH 4/5] Updated CHANGELOG.md --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9385e4..35bd56b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ Note: Numbers like (#123) point to closed Pull Requests on the fractal-vizarr-viewer repository. +# Unreleased + +* Distinguished handling of 401 Unauthorized and 403 Forbidden responses (\#47); +* Included `user_settings.project_dir` in list of allowed paths (\#47); +* Supported `AUTHORIZATION_SCHEME="testing-basic-auth"` (\#47); + # 0.2.1 * Updated Vizarr git commit references and removed vizarr.patch (\#42); From 53c41d385623ca933cfd67b84ccd5da4c9123c43 Mon Sep 17 00:00:00 2001 From: Sonia Zorba Date: Wed, 6 Nov 2024 15:36:55 +0100 Subject: [PATCH 5/5] Updated Docker setup in README.md [skip ci] --- README.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 3da1ba3..3870004 100644 --- a/README.md +++ b/README.md @@ -200,20 +200,21 @@ Then go back to fractal-vizarr-viewer folder and run `npm run start` to start th ## Docker setup -Build the docker image: +The following script can be used to build and start a docker image for testing: ```sh -docker build . -t fractal-vizarr-viewer -``` +#!/bin/sh -The following command can be used to start the docker image for testing: +COMMIT_HASH=$(git rev-parse HEAD) +IMAGE_NAME="fractal-vizarr-viewer-$COMMIT_HASH" + +docker build . -t "$IMAGE_NAME" -```sh docker run --network host \ - -v /path/to/zarr-files:/zarr-files \ + -v /tmp/zarr-files:/zarr-files \ -e FRACTAL_SERVER_URL=http://localhost:8000 \ -e AUTHORIZATION_SCHEME=fractal-server-viewer-paths \ - fractal-vizarr-viewer + "$IMAGE_NAME" ``` For production replace the `--network host` option with a proper published port `-p 3000:3000` and set `FRACTAL_SERVER_URL` as an URL using a public domain.