From fb24c552595f79839cc788bcc338ad33267c02c0 Mon Sep 17 00:00:00 2001 From: Sagi Levi Date: Wed, 10 Mar 2021 16:12:14 +0200 Subject: [PATCH 01/19] Add the option to set a custom_id for users with admin right only --- src/routes/url.ts | 11 +++-- .../storage/drivers/inMemory/index.ts | 8 +++- .../storage/drivers/relational/index.ts | 11 +++-- .../migrations/20210310115401_customId.ts | 44 +++++++++++++++++++ src/services/storage/index.ts | 4 +- src/services/storage/types/url.ts | 5 ++- 6 files changed, 70 insertions(+), 13 deletions(-) create mode 100644 src/services/storage/drivers/relational/migrations/20210310115401_customId.ts diff --git a/src/routes/url.ts b/src/routes/url.ts index e19acde..09b98c4 100644 --- a/src/routes/url.ts +++ b/src/routes/url.ts @@ -3,9 +3,8 @@ import { Route } from '../types/routes.js' import { NotFoundError } from '../errors/notFound.js' import { validateUrl } from '../services/urlValidator.js' import { UrlRequestData } from '../services/storage/types/url' - /* Save URL to store and return the new shortened url */ -const saveUrl: Route<{ Body: { url: string } }> = { +const saveUrl: Route<{ Body: { url: string, id?: string } }> = { method: 'POST', url: '/url', schema: { @@ -14,13 +13,19 @@ const saveUrl: Route<{ Body: { url: string } }> = { required: ['url'], properties: { url: { type: 'string' }, + id: { type: 'string' } }, }, }, async handler(request) { await validateUrl(request.body.url) - const urlRequestData = { url: request.body.url, ip: request.ip } as UrlRequestData + let urlRequestData = { url: request.body.url, ip: request.ip } as UrlRequestData + // need to make sure that has admin rights + if (await this.auth.isAuthorized(request) && request.body.id) { + urlRequestData = { ...urlRequestData, id: request.body.id } as UrlRequestData + } + const url = await this.storage.url.save(urlRequestData) return `${this.config.baseRedirectUrl}${url.id}` diff --git a/src/services/storage/drivers/inMemory/index.ts b/src/services/storage/drivers/inMemory/index.ts index 3ee34c8..d243666 100644 --- a/src/services/storage/drivers/inMemory/index.ts +++ b/src/services/storage/drivers/inMemory/index.ts @@ -1,4 +1,5 @@ import cryptoRandomString from 'crypto-random-string' +import { GeneralError } from '../../../../errors/generalError.js' import { NotFoundError } from '../../../../errors/notFound.js' import { InMemoryStorageConfig } from '../../types/config.js' import type { StorageDriver } from '../../types/index.js' @@ -10,7 +11,7 @@ export class InMemoryStorage implements StorageDriver { urlInformation: new Map(), } url = new (class InMemoryUrlStorage { - constructor(public storage: InMemoryStorage) {} + constructor(public storage: InMemoryStorage) { } public uuid() { let id @@ -64,8 +65,11 @@ export class InMemoryStorage implements StorageDriver { } public async save(requestData: UrlRequestData): Promise { + if (requestData.id && this.storage.data.urls.has(requestData.id)) { + throw new GeneralError('The specific id you chose is already in use') + } const storedUrl = { - id: this.uuid(), + id: requestData.id || this.uuid(), url: requestData.url, createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(), diff --git a/src/services/storage/drivers/relational/index.ts b/src/services/storage/drivers/relational/index.ts index 2f8ab12..c629028 100644 --- a/src/services/storage/drivers/relational/index.ts +++ b/src/services/storage/drivers/relational/index.ts @@ -63,7 +63,7 @@ export class RelationalStorage implements StorageDriver { await this.db.schema.dropSchemaIfExists(this.config.appName) } url = new (class RelationalUrlStorage { - constructor(public storage: RelationalStorage) {} + constructor(public storage: RelationalStorage) { } public async get(id: string, options = { withInfo: false }): Promise { let storedUrl, urlInfo @@ -90,7 +90,8 @@ export class RelationalStorage implements StorageDriver { //All Info associated with that Urls also get deleted, automatically. //https://stackoverflow.com/questions/53859207/deleting-data-from-associated-tables-using-knex-js public async delete(id: string): Promise { - await this.storage.db.table('urls').where('id', id).delete() + await this.storage.db.table('urls').where('id', id).delete(); + return; } public async deleteOverdue(timespanMs: number): Promise { @@ -112,7 +113,9 @@ export class RelationalStorage implements StorageDriver { } public async save(urlBody: UrlRequestData): Promise { - const url = urlBody.url + const { url, id } = urlBody; + const urlTableEntry = { url, id: id || "" } + const urlInfo: Partial = { ip: urlBody.ip, urlVisitCount: 0, @@ -121,7 +124,7 @@ export class RelationalStorage implements StorageDriver { } as UrlWithInformation if (!url) throw NotFoundError() - const [storedUrl] = await this.storage.db.table('urls').insert({ url }).returning('*') + const [storedUrl] = await this.storage.db.table('urls').insert(urlTableEntry).returning('*') await this.storage.db .table('url_information') .insert({ urlId: storedUrl.id, ...urlInfo }) diff --git a/src/services/storage/drivers/relational/migrations/20210310115401_customId.ts b/src/services/storage/drivers/relational/migrations/20210310115401_customId.ts new file mode 100644 index 0000000..0a57dbd --- /dev/null +++ b/src/services/storage/drivers/relational/migrations/20210310115401_customId.ts @@ -0,0 +1,44 @@ +import * as Knex from "knex"; + + +export async function up(knex: Knex): Promise { + await knex.raw(` + DROP TRIGGER IF EXISTS on_new_url ON urls; + CREATE OR REPLACE FUNCTION on_url_before_insert() + RETURNS trigger + LANGUAGE 'plpgsql' + AS $BODY$ + BEGIN + IF (NEW.id = '') IS NOT FALSE THEN + NEW.id = stringify_bigint(pseudo_encrypt(NEW.serial)); + END IF; + RETURN NEW; + END; + $BODY$; + + CREATE TRIGGER on_new_url + BEFORE INSERT ON urls + FOR EACH ROW + EXECUTE PROCEDURE on_url_before_insert();`) +} + + +export async function down(knex: Knex): Promise { + await knex.raw(` + DROP TRIGGER IF EXISTS on_new_url ON urls; + CREATE OR REPLACE FUNCTION on_url_before_insert() + RETURNS trigger + LANGUAGE 'plpgsql' + AS $BODY$ + BEGIN + NEW.id = stringify_bigint(pseudo_encrypt(NEW.serial)); + RETURN NEW; + END; + $BODY$; + + CREATE TRIGGER on_new_url + BEFORE INSERT ON urls + FOR EACH ROW + EXECUTE PROCEDURE on_url_before_insert();`) +} + diff --git a/src/services/storage/index.ts b/src/services/storage/index.ts index 137e1cb..796f27c 100644 --- a/src/services/storage/index.ts +++ b/src/services/storage/index.ts @@ -41,7 +41,7 @@ export class Storage implements StorageDriver { } url = new (class UrlStorage { - constructor(public storage: Storage) {} + constructor(public storage: Storage) { } get driver() { return this.storage._driver @@ -88,7 +88,7 @@ export class Storage implements StorageDriver { public async save(body: UrlRequestData): Promise { try { - logger.debug(`Start Storage.url.save with url: ${body.url}, ip: ${body.ip}`) + logger.debug(`Start Storage.url.save with url: ${body.url}, ip: ${body.ip}${body.id && `, id: ${body.id}`}`) return await this.driver.url.save(body) } catch (err) { logger.error(`Storage.url.save failed: ${err}`) diff --git a/src/services/storage/types/url.ts b/src/services/storage/types/url.ts index a06df4d..632cfd8 100644 --- a/src/services/storage/types/url.ts +++ b/src/services/storage/types/url.ts @@ -22,8 +22,9 @@ export interface UrlStorageDriver { } export interface UrlRequestData { - url: string - ip: string + url: string; + id?: string; + ip: string; } export interface UrlWithInformation extends StoredUrl { From 8712965577dbb1c0a672008b5240d20fbbad1b59 Mon Sep 17 00:00:00 2001 From: lesagi Date: Sat, 13 Mar 2021 22:29:53 +0200 Subject: [PATCH 02/19] Update src/services/storage/drivers/relational/index.ts destructuring function argument Co-authored-by: Snir Shechter --- src/services/storage/drivers/relational/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/storage/drivers/relational/index.ts b/src/services/storage/drivers/relational/index.ts index cf94576..96689b1 100644 --- a/src/services/storage/drivers/relational/index.ts +++ b/src/services/storage/drivers/relational/index.ts @@ -112,7 +112,7 @@ export class RelationalStorage implements StorageDriver { return storedUrl } - public async save(urlBody: UrlRequestData): Promise { + public async save({ url, id = '', ip }: UrlRequestData): Promise { const { url, id } = urlBody const urlTableEntry = { url, id: id || '' } From f3cca3707fcf4533ff98fe4f6d6eedc4f17e3424 Mon Sep 17 00:00:00 2001 From: lesagi Date: Sat, 13 Mar 2021 22:35:11 +0200 Subject: [PATCH 03/19] Update src/routes/url.ts Co-authored-by: Snir Shechter --- src/routes/url.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/routes/url.ts b/src/routes/url.ts index 9d707db..b3367dd 100644 --- a/src/routes/url.ts +++ b/src/routes/url.ts @@ -21,7 +21,7 @@ const saveUrl: Route<{ Body: { url: string; id?: string } }> = { await validateUrl(request.body.url) let urlRequestData = { url: request.body.url, ip: request.ip } as UrlRequestData - // need to make sure that has admin rights + // Custom ids require admin rights if ((await this.auth.isAuthorized(request)) && request.body.id) { urlRequestData = { ...urlRequestData, id: request.body.id } as UrlRequestData } From 10839e3e233e9a7f494e7fa27e47b89f9873100f Mon Sep 17 00:00:00 2001 From: lesagi Date: Sat, 13 Mar 2021 22:44:12 +0200 Subject: [PATCH 04/19] Update src/services/storage/drivers/relational/index.ts Co-authored-by: Snir Shechter --- src/services/storage/drivers/relational/index.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/services/storage/drivers/relational/index.ts b/src/services/storage/drivers/relational/index.ts index 96689b1..18f9656 100644 --- a/src/services/storage/drivers/relational/index.ts +++ b/src/services/storage/drivers/relational/index.ts @@ -113,11 +113,8 @@ export class RelationalStorage implements StorageDriver { } public async save({ url, id = '', ip }: UrlRequestData): Promise { - const { url, id } = urlBody - const urlTableEntry = { url, id: id || '' } - const urlInfo: UrlInformation = { - ip: urlBody.ip, + ip, urlVisitCount: 0, infoVisitCount: 0, lastUsed: new Date().toISOString(), From a7f4980c7b117d14b25bde4996b99518634310b8 Mon Sep 17 00:00:00 2001 From: lesagi Date: Sat, 13 Mar 2021 22:44:38 +0200 Subject: [PATCH 05/19] Update src/services/storage/drivers/relational/index.ts Co-authored-by: Snir Shechter --- src/services/storage/drivers/relational/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/storage/drivers/relational/index.ts b/src/services/storage/drivers/relational/index.ts index 18f9656..388a408 100644 --- a/src/services/storage/drivers/relational/index.ts +++ b/src/services/storage/drivers/relational/index.ts @@ -121,7 +121,7 @@ export class RelationalStorage implements StorageDriver { } if (!url) throw NotFoundError() - const [storedUrl] = await this.storage.db.table('urls').insert(urlTableEntry).returning('*') + const [storedUrl] = await this.storage.db.table('urls').insert({ url, id }).returning('*') await this.storage.db .table('url_information') .insert({ urlId: storedUrl.id, ...urlInfo }) From 5bf4b3c7ed1404fd43888d80bbecffebd59d7be4 Mon Sep 17 00:00:00 2001 From: Sagi Levi Date: Sun, 14 Mar 2021 00:54:19 +0200 Subject: [PATCH 06/19] Unauthorized Error on Custom ID throwing an error when non-privileged user trying to create a custom id --- src/routes/url.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/routes/url.ts b/src/routes/url.ts index b3367dd..9d02904 100644 --- a/src/routes/url.ts +++ b/src/routes/url.ts @@ -3,6 +3,7 @@ import { Route } from '../types/routes.js' import { NotFoundError } from '../errors/notFound.js' import { validateUrl } from '../services/urlValidator.js' import { UrlRequestData } from '../services/storage/types/url' +import { UnauthorizedError } from '../errors/unauthorized.js' /* Save URL to store and return the new shortened url */ const saveUrl: Route<{ Body: { url: string; id?: string } }> = { method: 'POST', @@ -22,8 +23,13 @@ const saveUrl: Route<{ Body: { url: string; id?: string } }> = { let urlRequestData = { url: request.body.url, ip: request.ip } as UrlRequestData // Custom ids require admin rights - if ((await this.auth.isAuthorized(request)) && request.body.id) { - urlRequestData = { ...urlRequestData, id: request.body.id } as UrlRequestData + if (request.body.id) { + if (await this.auth.isAuthorized(request)) { + urlRequestData = { ...urlRequestData, id: request.body.id } as UrlRequestData + } else { + throw UnauthorizedError("user without admin priviliges trying to create a url with custom id") + } + } const url = await this.storage.url.save(urlRequestData) From ff593724a507cdf40509aac2b6080c5641fd17d4 Mon Sep 17 00:00:00 2001 From: Sagi Levi Date: Mon, 15 Mar 2021 15:37:33 +0200 Subject: [PATCH 07/19] Use postgres Error number to identify id duplicates --- src/services/storage/drivers/relational/index.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/services/storage/drivers/relational/index.ts b/src/services/storage/drivers/relational/index.ts index 388a408..bbfed0e 100644 --- a/src/services/storage/drivers/relational/index.ts +++ b/src/services/storage/drivers/relational/index.ts @@ -9,6 +9,7 @@ import camelcaseKeys from 'camelcase-keys' import { snakeCase } from 'snake-case' import type { StoredUrl, UrlWithInformation, UrlRequestData, UrlInformation } from '../../types/url.js' import { RelationalStorageConfig } from '../../types/config.js' +import { GeneralError } from '../../../../errors/generalError.js' const __dirname = dirname(fileURLToPath(import.meta.url)) @@ -63,7 +64,7 @@ export class RelationalStorage implements StorageDriver { await this.db.schema.dropSchemaIfExists(this.config.appName) } url = new (class RelationalUrlStorage { - constructor(public storage: RelationalStorage) {} + constructor(public storage: RelationalStorage) { } public async get(id: string, options = { withInfo: false }): Promise { let storedUrl, urlInfo @@ -121,7 +122,17 @@ export class RelationalStorage implements StorageDriver { } if (!url) throw NotFoundError() - const [storedUrl] = await this.storage.db.table('urls').insert({ url, id }).returning('*') + + let storedUrl: StoredUrl; + try { + [storedUrl] = await this.storage.db.table('urls').insert({ url, id }).returning('*') + } catch (e) { + if (e.code === "23505") + throw new GeneralError("The specific id you chose is already in use") + else + throw new GeneralError() + } + await this.storage.db .table('url_information') .insert({ urlId: storedUrl.id, ...urlInfo }) From ceb52c30f05f0a471c3ba2a6ad1031dd4bc65cf9 Mon Sep 17 00:00:00 2001 From: Sagi Levi Date: Sat, 20 Mar 2021 12:14:43 +0200 Subject: [PATCH 08/19] Check if custom_id already exists before saving --- src/services/storage/drivers/relational/index.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/services/storage/drivers/relational/index.ts b/src/services/storage/drivers/relational/index.ts index bbfed0e..6091d7a 100644 --- a/src/services/storage/drivers/relational/index.ts +++ b/src/services/storage/drivers/relational/index.ts @@ -123,16 +123,12 @@ export class RelationalStorage implements StorageDriver { if (!url) throw NotFoundError() - let storedUrl: StoredUrl; - try { - [storedUrl] = await this.storage.db.table('urls').insert({ url, id }).returning('*') - } catch (e) { - if (e.code === "23505") - throw new GeneralError("The specific id you chose is already in use") - else - throw new GeneralError() + if (id && await this.storage.db.table('urls').select('*').where('id', id).first()) { + throw new GeneralError("The specific id you chose is already in use") } + const [storedUrl] = await this.storage.db.table('urls').insert({ url, id }).returning('*') + await this.storage.db .table('url_information') .insert({ urlId: storedUrl.id, ...urlInfo }) From f5d58ab9f8c1f4e9788aa46185eaeddf45d69354 Mon Sep 17 00:00:00 2001 From: Sagi Levi Date: Tue, 23 Mar 2021 17:12:25 +0200 Subject: [PATCH 09/19] Add 'deletedAt' property to StoredUrl type --- src/services/storage/types/url.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/services/storage/types/url.ts b/src/services/storage/types/url.ts index 31c9ea0..b167e4b 100644 --- a/src/services/storage/types/url.ts +++ b/src/services/storage/types/url.ts @@ -3,6 +3,7 @@ export interface StoredUrl { url: string createdAt: string updatedAt: string + deletedAt?: string } export interface UrlStorageDriver { From eb1c327a57fb43899e205acaa292d62dbf0ee71a Mon Sep 17 00:00:00 2001 From: Sagi Levi Date: Tue, 23 Mar 2021 17:13:24 +0200 Subject: [PATCH 10/19] Replace hard delete with soft delete for InMemory Driver --- src/services/storage/drivers/inMemory/index.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/services/storage/drivers/inMemory/index.ts b/src/services/storage/drivers/inMemory/index.ts index ad95eaa..b2e3b3e 100644 --- a/src/services/storage/drivers/inMemory/index.ts +++ b/src/services/storage/drivers/inMemory/index.ts @@ -11,7 +11,7 @@ export class InMemoryStorage implements StorageDriver { urlInformation: new Map(), } url = new (class InMemoryUrlStorage { - constructor(public storage: InMemoryStorage) {} + constructor(public storage: InMemoryStorage) { } public uuid() { let id @@ -35,10 +35,20 @@ export class InMemoryStorage implements StorageDriver { } public async delete(id: string): Promise { - if (typeof this.storage.data.urls.get(id) === 'undefined') throw new NotFoundError() + const storedUrl = this.storage.data.urls.get(id); + if (typeof storedUrl === 'undefined') throw new NotFoundError() - this.storage.data.urls.delete(id) + const { url, createdAt, updatedAt } = storedUrl; + const newStoredUrl: StoredUrl = { + id, + url, + createdAt, + updatedAt, + deletedAt: new Date().toUTCString() + } + this.storage.data.urls.set(id, newStoredUrl); } + public async deleteOverdue(timespanMs: number): Promise { const deleteBefore = new Date().getTime() - timespanMs let deletedCount = 0 From a1f5d2f7dce9974c71cdc788ce9f4d8d43a9c3b6 Mon Sep 17 00:00:00 2001 From: Sagi Levi Date: Tue, 23 Mar 2021 17:17:02 +0200 Subject: [PATCH 11/19] Store ISOString in the 'deletedAt' field instead of UTCString --- src/services/storage/drivers/inMemory/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/storage/drivers/inMemory/index.ts b/src/services/storage/drivers/inMemory/index.ts index b2e3b3e..a24f209 100644 --- a/src/services/storage/drivers/inMemory/index.ts +++ b/src/services/storage/drivers/inMemory/index.ts @@ -44,7 +44,7 @@ export class InMemoryStorage implements StorageDriver { url, createdAt, updatedAt, - deletedAt: new Date().toUTCString() + deletedAt: new Date().toISOString() } this.storage.data.urls.set(id, newStoredUrl); } From 274e13e8555e82e5e42f977dd9fbe8ffca094ddc Mon Sep 17 00:00:00 2001 From: Sagi Levi Date: Tue, 23 Mar 2021 17:42:41 +0200 Subject: [PATCH 12/19] Prevent unauthorized user to fetch soft deleted url --- src/routes/url.ts | 4 ++-- src/services/storage/drivers/inMemory/index.ts | 5 +++-- src/services/storage/index.ts | 2 +- src/services/storage/types/url.ts | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/routes/url.ts b/src/routes/url.ts index 9d02904..95f7f9a 100644 --- a/src/routes/url.ts +++ b/src/routes/url.ts @@ -57,8 +57,8 @@ const retrieveUrl: Route<{ Params: { id: string } }> = { async handler(request) { if (request.validationError) throw new NotFoundError() - const withInfo = await this.auth.isAuthorized(request) - const storedUrl = await this.storage.url.get(request.params.id, { withInfo }) + const isAuthorized = await this.auth.isAuthorized(request) + const storedUrl = await this.storage.url.get(request.params.id, { withInfo: isAuthorized, isAuthorized }) if (typeof storedUrl === 'undefined') throw new NotFoundError() try { diff --git a/src/services/storage/drivers/inMemory/index.ts b/src/services/storage/drivers/inMemory/index.ts index a24f209..51c89be 100644 --- a/src/services/storage/drivers/inMemory/index.ts +++ b/src/services/storage/drivers/inMemory/index.ts @@ -1,6 +1,7 @@ import cryptoRandomString from 'crypto-random-string' import { GeneralError } from '../../../../errors/generalError.js' import { NotFoundError } from '../../../../errors/notFound.js' +import { UnauthorizedError } from '../../../../errors/unauthorized.js' import { InMemoryStorageConfig } from '../../types/config.js' import type { StorageDriver } from '../../types/index.js' import type { StoredUrl, UrlWithInformation, UrlRequestData, UrlInformation } from '../../types/url.js' @@ -22,10 +23,10 @@ export class InMemoryStorage implements StorageDriver { return id } - public async get(id: string, options = { withInfo: false }): Promise { + public async get(id: string, options = { withInfo: false, isAuthorized: false }): Promise { const storedUrl = this.storage.data.urls.get(id) if (typeof storedUrl === 'undefined') throw new NotFoundError() - + if (storedUrl.deletedAt && !options.isAuthorized) throw new UnauthorizedError(); if (!options.withInfo) { return storedUrl } diff --git a/src/services/storage/index.ts b/src/services/storage/index.ts index f82e287..79e1e55 100644 --- a/src/services/storage/index.ts +++ b/src/services/storage/index.ts @@ -56,7 +56,7 @@ export class Storage implements StorageDriver { get driver() { return this.storage._driver } - public async get(id: string, options = { withInfo: false }): Promise { + public async get(id: string, options = { withInfo: false, isAuthorized: false }): Promise { try { logger.debug(`Running Storage.url.get with ${id}`) return await this.driver.url.get(id, options) diff --git a/src/services/storage/types/url.ts b/src/services/storage/types/url.ts index b167e4b..c960f3c 100644 --- a/src/services/storage/types/url.ts +++ b/src/services/storage/types/url.ts @@ -7,7 +7,7 @@ export interface StoredUrl { } export interface UrlStorageDriver { - get(id: string, options: { withInfo: boolean }): Promise + get(id: string, options: { withInfo: boolean, isAuthorized: boolean }): Promise save(url: UrlRequestData): Promise From 2f0c51dfe526b4eb6108630b63208be1fc911280 Mon Sep 17 00:00:00 2001 From: Sagi Levi Date: Tue, 23 Mar 2021 18:01:41 +0200 Subject: [PATCH 13/19] Make the hard delete optional along with soft delete --- .../storage/drivers/inMemory/index.ts | 23 +++++++++++-------- src/services/storage/index.ts | 4 ++-- src/services/storage/types/url.ts | 2 +- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/services/storage/drivers/inMemory/index.ts b/src/services/storage/drivers/inMemory/index.ts index 51c89be..d6c1fd5 100644 --- a/src/services/storage/drivers/inMemory/index.ts +++ b/src/services/storage/drivers/inMemory/index.ts @@ -35,19 +35,24 @@ export class InMemoryStorage implements StorageDriver { return { ...storedUrl, ...urlInfo } } - public async delete(id: string): Promise { + public async delete(id: string, options: { hardDelete: false }): Promise { const storedUrl = this.storage.data.urls.get(id); if (typeof storedUrl === 'undefined') throw new NotFoundError() - const { url, createdAt, updatedAt } = storedUrl; - const newStoredUrl: StoredUrl = { - id, - url, - createdAt, - updatedAt, - deletedAt: new Date().toISOString() + if (options.hardDelete) { + this.storage.data.urls.delete(id); + } + else { + const { url, createdAt, updatedAt } = storedUrl; + const newStoredUrl: StoredUrl = { + id, + url, + createdAt, + updatedAt, + deletedAt: new Date().toISOString() + } + this.storage.data.urls.set(id, newStoredUrl); } - this.storage.data.urls.set(id, newStoredUrl); } public async deleteOverdue(timespanMs: number): Promise { diff --git a/src/services/storage/index.ts b/src/services/storage/index.ts index 79e1e55..82cf9d2 100644 --- a/src/services/storage/index.ts +++ b/src/services/storage/index.ts @@ -66,10 +66,10 @@ export class Storage implements StorageDriver { } } - public async delete(id: string): Promise { + public async delete(id: string, options: { hardDelete: false }): Promise { try { logger.debug(`Running Storage.url.delete with ${id}`) - return await this.driver.url.delete(id) + return await this.driver.url.delete(id, options) } catch (err) { logger.error(`Storage.url.delete failed: ${err}`) throw new GeneralError('Could not delete url') diff --git a/src/services/storage/types/url.ts b/src/services/storage/types/url.ts index c960f3c..831443a 100644 --- a/src/services/storage/types/url.ts +++ b/src/services/storage/types/url.ts @@ -13,7 +13,7 @@ export interface UrlStorageDriver { edit(id: string, url: string): Promise - delete(id: string): Promise + delete(id: string, options: { hardDelete: boolean }): Promise deleteOverdue(timespanMs: number): Promise From e3707fa8331aa32575349f213f46c54b3b545e5f Mon Sep 17 00:00:00 2001 From: Sagi Levi Date: Tue, 23 Mar 2021 18:11:31 +0200 Subject: [PATCH 14/19] Add the option for soft delete for Relational DB --- src/services/storage/drivers/relational/index.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/services/storage/drivers/relational/index.ts b/src/services/storage/drivers/relational/index.ts index f21017c..01b9970 100644 --- a/src/services/storage/drivers/relational/index.ts +++ b/src/services/storage/drivers/relational/index.ts @@ -93,9 +93,14 @@ export class RelationalStorage implements StorageDriver { //All Info associated with that Urls also get deleted, automatically. //https://stackoverflow.com/questions/53859207/deleting-data-from-associated-tables-using-knex-js - public async delete(id: string): Promise { - await this.storage.db.table('urls').where('id', id).delete() - return + public async delete(id: string, options: { hardDelete: false }): Promise { + if (!await this.storage.db.table('urls').where('id', id)) throw new NotFoundError(); + + if (options.hardDelete) { + await this.storage.db.table('urls').where('id', id).delete() + } else { + await this.storage.db.table('urls').update({ deletedAt: new Date().toISOString() }).where('id', id) + } } public async deleteOverdue(timespanMs: number): Promise { From 1fafdc8b21ab0ae654c4896b7daa9f6cfe80622b Mon Sep 17 00:00:00 2001 From: Sagi Levi Date: Tue, 23 Mar 2021 18:19:50 +0200 Subject: [PATCH 15/19] Relational: Prevent unauthorized user to fetch soft deleted url --- src/services/storage/drivers/relational/index.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/services/storage/drivers/relational/index.ts b/src/services/storage/drivers/relational/index.ts index 01b9970..163b63e 100644 --- a/src/services/storage/drivers/relational/index.ts +++ b/src/services/storage/drivers/relational/index.ts @@ -10,6 +10,7 @@ import { snakeCase } from 'snake-case' import type { StoredUrl, UrlWithInformation, UrlRequestData, UrlInformation } from '../../types/url.js' import { RelationalStorageConfig } from '../../types/config.js' import { GeneralError } from '../../../../errors/generalError.js' +import { UnauthorizedError } from '../../../../errors/unauthorized.js' const __dirname = dirname(fileURLToPath(import.meta.url)) @@ -69,15 +70,15 @@ export class RelationalStorage implements StorageDriver { url = new (class RelationalUrlStorage { constructor(public storage: RelationalStorage) { } - public async get(id: string, options = { withInfo: false }): Promise { + public async get(id: string, options = { withInfo: false, isAuthorized: false }): Promise { let storedUrl, urlInfo + storedUrl = await this.storage.db + .table('urls') + .select('*') + .where('id', id) + .first() + if (storedUrl?.deletedAt && !options.isAuthorized) throw new UnauthorizedError(); if (!options.withInfo) { - storedUrl = await this.storage.db - .table('urls') - .select('*') - .where('id', id) - .first() - delete storedUrl?.serial } else { urlInfo = await this.storage.db From 0039269f564f44fe067cdfde215d060014ab5adc Mon Sep 17 00:00:00 2001 From: Sagi Levi Date: Thu, 1 Apr 2021 14:47:00 +0300 Subject: [PATCH 16/19] Update Soft Delete - Add a migration for the new 'deletedAt' Column - Define a default SOFT delete method for deleteOverdue - Change the 'options' parameter passed to the drivers' functions --- src/routes/url.ts | 2 +- .../storage/drivers/inMemory/index.ts | 43 ++++++++++--------- .../storage/drivers/relational/index.ts | 11 +++-- .../migrations/20210401143602_softDelete.ts | 17 ++++++++ src/services/storage/index.ts | 11 +++-- src/services/storage/types/url.ts | 4 +- 6 files changed, 54 insertions(+), 34 deletions(-) create mode 100644 src/services/storage/drivers/relational/migrations/20210401143602_softDelete.ts diff --git a/src/routes/url.ts b/src/routes/url.ts index 95f7f9a..095b6ff 100644 --- a/src/routes/url.ts +++ b/src/routes/url.ts @@ -58,7 +58,7 @@ const retrieveUrl: Route<{ Params: { id: string } }> = { if (request.validationError) throw new NotFoundError() const isAuthorized = await this.auth.isAuthorized(request) - const storedUrl = await this.storage.url.get(request.params.id, { withInfo: isAuthorized, isAuthorized }) + const storedUrl = await this.storage.url.get(request.params.id, { withInfo: isAuthorized, includeDeleted: isAuthorized }) if (typeof storedUrl === 'undefined') throw new NotFoundError() try { diff --git a/src/services/storage/drivers/inMemory/index.ts b/src/services/storage/drivers/inMemory/index.ts index d6c1fd5..5561ee4 100644 --- a/src/services/storage/drivers/inMemory/index.ts +++ b/src/services/storage/drivers/inMemory/index.ts @@ -1,11 +1,9 @@ import cryptoRandomString from 'crypto-random-string' import { GeneralError } from '../../../../errors/generalError.js' import { NotFoundError } from '../../../../errors/notFound.js' -import { UnauthorizedError } from '../../../../errors/unauthorized.js' import { InMemoryStorageConfig } from '../../types/config.js' import type { StorageDriver } from '../../types/index.js' import type { StoredUrl, UrlWithInformation, UrlRequestData, UrlInformation } from '../../types/url.js' - export class InMemoryStorage implements StorageDriver { data: { urls: Map; urlInformation: Map } = { urls: new Map(), @@ -23,10 +21,10 @@ export class InMemoryStorage implements StorageDriver { return id } - public async get(id: string, options = { withInfo: false, isAuthorized: false }): Promise { + public async get(id: string, options = { withInfo: false, includeDeleted: true }): Promise { const storedUrl = this.storage.data.urls.get(id) - if (typeof storedUrl === 'undefined') throw new NotFoundError() - if (storedUrl.deletedAt && !options.isAuthorized) throw new UnauthorizedError(); + if (typeof storedUrl === 'undefined' || storedUrl.deletedAt) throw new NotFoundError() + if (!options.withInfo) { return storedUrl } @@ -35,23 +33,26 @@ export class InMemoryStorage implements StorageDriver { return { ...storedUrl, ...urlInfo } } - public async delete(id: string, options: { hardDelete: false }): Promise { + public async softDelete(storedUrl: StoredUrl): Promise { + const { id, url, createdAt, updatedAt } = storedUrl; + const newStoredUrl: StoredUrl = { + id, + url, + createdAt, + updatedAt, + deletedAt: new Date().toISOString() + } + this.storage.data.urls.set(id, newStoredUrl); + } + + public async delete(id: string, options: { softDelete: boolean }): Promise { const storedUrl = this.storage.data.urls.get(id); if (typeof storedUrl === 'undefined') throw new NotFoundError() - if (options.hardDelete) { + if (!options.softDelete) { this.storage.data.urls.delete(id); - } - else { - const { url, createdAt, updatedAt } = storedUrl; - const newStoredUrl: StoredUrl = { - id, - url, - createdAt, - updatedAt, - deletedAt: new Date().toISOString() - } - this.storage.data.urls.set(id, newStoredUrl); + } else { + this.softDelete(storedUrl); } } @@ -60,9 +61,9 @@ export class InMemoryStorage implements StorageDriver { let deletedCount = 0 this.storage.data.urls.forEach((storedUrl) => { - const updatedAt = new Date(storedUrl.updatedAt).getTime() - if (updatedAt <= deleteBefore) { - this.storage.data.urls.delete(storedUrl.id) + const updatedAtDate = new Date(storedUrl.updatedAt).getTime() + if (!storedUrl.deletedAt && updatedAtDate <= deleteBefore) { + this.softDelete(storedUrl); deletedCount++ } }) diff --git a/src/services/storage/drivers/relational/index.ts b/src/services/storage/drivers/relational/index.ts index 163b63e..94d4a77 100644 --- a/src/services/storage/drivers/relational/index.ts +++ b/src/services/storage/drivers/relational/index.ts @@ -10,7 +10,6 @@ import { snakeCase } from 'snake-case' import type { StoredUrl, UrlWithInformation, UrlRequestData, UrlInformation } from '../../types/url.js' import { RelationalStorageConfig } from '../../types/config.js' import { GeneralError } from '../../../../errors/generalError.js' -import { UnauthorizedError } from '../../../../errors/unauthorized.js' const __dirname = dirname(fileURLToPath(import.meta.url)) @@ -70,14 +69,14 @@ export class RelationalStorage implements StorageDriver { url = new (class RelationalUrlStorage { constructor(public storage: RelationalStorage) { } - public async get(id: string, options = { withInfo: false, isAuthorized: false }): Promise { + public async get(id: string, options = { withInfo: false, includeDeleted: true }): Promise { let storedUrl, urlInfo storedUrl = await this.storage.db .table('urls') .select('*') .where('id', id) .first() - if (storedUrl?.deletedAt && !options.isAuthorized) throw new UnauthorizedError(); + if (storedUrl?.deletedAt) throw new NotFoundError(); if (!options.withInfo) { delete storedUrl?.serial } else { @@ -94,10 +93,10 @@ export class RelationalStorage implements StorageDriver { //All Info associated with that Urls also get deleted, automatically. //https://stackoverflow.com/questions/53859207/deleting-data-from-associated-tables-using-knex-js - public async delete(id: string, options: { hardDelete: false }): Promise { + public async delete(id: string, options: { softDelete: true }): Promise { if (!await this.storage.db.table('urls').where('id', id)) throw new NotFoundError(); - if (options.hardDelete) { + if (options.softDelete) { await this.storage.db.table('urls').where('id', id).delete() } else { await this.storage.db.table('urls').update({ deletedAt: new Date().toISOString() }).where('id', id) @@ -106,7 +105,7 @@ export class RelationalStorage implements StorageDriver { public async deleteOverdue(timespanMs: number): Promise { const deleteBefore = new Date(new Date().getTime() - timespanMs) - return await this.storage.db.table('urls').where('updatedAt', '<', deleteBefore).delete() + return await this.storage.db.table('urls').update({ deletedAt: new Date().toISOString() }).where('updatedAt', '<', deleteBefore); } public async edit(id: string, url: string): Promise { diff --git a/src/services/storage/drivers/relational/migrations/20210401143602_softDelete.ts b/src/services/storage/drivers/relational/migrations/20210401143602_softDelete.ts new file mode 100644 index 0000000..954973f --- /dev/null +++ b/src/services/storage/drivers/relational/migrations/20210401143602_softDelete.ts @@ -0,0 +1,17 @@ +import * as Knex from "knex"; + + +export async function up(knex: Knex): Promise { + return await knex.schema.table('urls', table => { + table.string('deletedAt'); + }) + +} + + +export async function down(knex: Knex): Promise { + return knex.schema.table('urls', table => { + table.dropColumn('deletedAt'); + }) +} + diff --git a/src/services/storage/index.ts b/src/services/storage/index.ts index 82cf9d2..d91f4b0 100644 --- a/src/services/storage/index.ts +++ b/src/services/storage/index.ts @@ -35,9 +35,12 @@ export class Storage implements StorageDriver { logger.debug(`Running Storage.initialize`) // Waits for 1 minute (6 * 10,000ms) before failing await runWithRetries(this._driver.initialize.bind(this._driver), { retries: 6, retryTime: 10 * 1000 }) - await this.url.deleteOverdue(this.config.lifetimeMs) + + const { lifetimeMs } = this.config; + await this.url.deleteOverdue(lifetimeMs) + this._intervalToken = setInterval( - () => this.url.deleteOverdue(this.config.lifetimeMs), + () => this.url.deleteOverdue(lifetimeMs), this.config.cleanupIntervalMs, ) } catch (err) { @@ -56,7 +59,7 @@ export class Storage implements StorageDriver { get driver() { return this.storage._driver } - public async get(id: string, options = { withInfo: false, isAuthorized: false }): Promise { + public async get(id: string, options = { withInfo: false, includeDeleted: true }): Promise { try { logger.debug(`Running Storage.url.get with ${id}`) return await this.driver.url.get(id, options) @@ -66,7 +69,7 @@ export class Storage implements StorageDriver { } } - public async delete(id: string, options: { hardDelete: false }): Promise { + public async delete(id: string, options: { softDelete: boolean }): Promise { try { logger.debug(`Running Storage.url.delete with ${id}`) return await this.driver.url.delete(id, options) diff --git a/src/services/storage/types/url.ts b/src/services/storage/types/url.ts index 831443a..12c67e0 100644 --- a/src/services/storage/types/url.ts +++ b/src/services/storage/types/url.ts @@ -7,13 +7,13 @@ export interface StoredUrl { } export interface UrlStorageDriver { - get(id: string, options: { withInfo: boolean, isAuthorized: boolean }): Promise + get(id: string, options: { withInfo: boolean, includeDeleted: boolean }): Promise save(url: UrlRequestData): Promise edit(id: string, url: string): Promise - delete(id: string, options: { hardDelete: boolean }): Promise + delete(id: string, options: { softDelete: boolean }): Promise deleteOverdue(timespanMs: number): Promise From 0350ad872c235e10be32021b1a12b1ff9ae35ad8 Mon Sep 17 00:00:00 2001 From: Sagi Levi Date: Thu, 1 Apr 2021 17:07:36 +0300 Subject: [PATCH 17/19] Remove unecessary import --- src/services/auth/drivers/bearerToken/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/services/auth/drivers/bearerToken/index.ts b/src/services/auth/drivers/bearerToken/index.ts index 7da9d0d..b1607cb 100644 --- a/src/services/auth/drivers/bearerToken/index.ts +++ b/src/services/auth/drivers/bearerToken/index.ts @@ -2,7 +2,6 @@ import { AuthDriver } from '../../types' import { BearerTokenDriverConfig } from './types' import { FastifyRequest } from 'fastify' import { UnauthorizedError } from '../../../../errors/unauthorized.js' -import { logger } from '../../../logger/logger.js' export class BearerTokenAuth implements AuthDriver { private token: string From d9195ffe82f9217764a456de3ba633ccd91e29ee Mon Sep 17 00:00:00 2001 From: Sagi Levi Date: Thu, 1 Apr 2021 17:08:03 +0300 Subject: [PATCH 18/19] Refactor softDelete function location in InMemoryStorage --- .../storage/drivers/inMemory/index.ts | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/services/storage/drivers/inMemory/index.ts b/src/services/storage/drivers/inMemory/index.ts index 5561ee4..bb2ae8b 100644 --- a/src/services/storage/drivers/inMemory/index.ts +++ b/src/services/storage/drivers/inMemory/index.ts @@ -9,6 +9,17 @@ export class InMemoryStorage implements StorageDriver { urls: new Map(), urlInformation: new Map(), } + private async softDelete(storedUrl: StoredUrl): Promise { + const { id, url, createdAt, updatedAt } = storedUrl; + const newStoredUrl: StoredUrl = { + id, + url, + createdAt, + updatedAt, + deletedAt: new Date().toISOString() + } + this.data.urls.set(id, newStoredUrl); + } url = new (class InMemoryUrlStorage { constructor(public storage: InMemoryStorage) { } @@ -33,17 +44,7 @@ export class InMemoryStorage implements StorageDriver { return { ...storedUrl, ...urlInfo } } - public async softDelete(storedUrl: StoredUrl): Promise { - const { id, url, createdAt, updatedAt } = storedUrl; - const newStoredUrl: StoredUrl = { - id, - url, - createdAt, - updatedAt, - deletedAt: new Date().toISOString() - } - this.storage.data.urls.set(id, newStoredUrl); - } + public async delete(id: string, options: { softDelete: boolean }): Promise { const storedUrl = this.storage.data.urls.get(id); @@ -52,7 +53,7 @@ export class InMemoryStorage implements StorageDriver { if (!options.softDelete) { this.storage.data.urls.delete(id); } else { - this.softDelete(storedUrl); + this.storage.softDelete(storedUrl); } } @@ -131,4 +132,5 @@ export class InMemoryStorage implements StorageDriver { // eslint-disable-next-line constructor(public config: InMemoryStorageConfig) { } + } From df3172f60dc68c05bc3905040b4981b2374460ce Mon Sep 17 00:00:00 2001 From: Sagi Levi Date: Fri, 2 Apr 2021 14:47:02 +0300 Subject: [PATCH 19/19] Add a call to storage.softDelete in Overdue --- src/services/storage/drivers/inMemory/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/storage/drivers/inMemory/index.ts b/src/services/storage/drivers/inMemory/index.ts index bb2ae8b..aa3c473 100644 --- a/src/services/storage/drivers/inMemory/index.ts +++ b/src/services/storage/drivers/inMemory/index.ts @@ -64,7 +64,7 @@ export class InMemoryStorage implements StorageDriver { this.storage.data.urls.forEach((storedUrl) => { const updatedAtDate = new Date(storedUrl.updatedAt).getTime() if (!storedUrl.deletedAt && updatedAtDate <= deleteBefore) { - this.softDelete(storedUrl); + this.storage.softDelete(storedUrl); deletedCount++ } })