etag for keys (#1753)

* etag for keys

* Tests added

* add etag to reponse header on update

* check alias key etag

* safe

Co-authored-by: Yoav Karako <yoav@soluto.com>

e2e/integration/spec/authoring-api/key-etag.spec.js

@@ -0,0 +1,78 @@
+const { expect } = require('chai');
+const client = require('../../utils/client');
+const { pollUntil } = require('../../utils/utils');
+const { createManifestForJPadKey } = require('../../utils/manifest');
+
+const keyPath = '@tests/integration/new_key_with_etag';
+const author = { name: 'ellie', email: 'ellie@lou.com' };
+const authorQuery = { 'author.name': author.name, 'author.email': author.email };
+
+describe.only('Key etag', () => {
+  let etag;
+  it('Create new key', async () => {
+    await client
+      .put(`/api/v2/keys/${keyPath}`)
+      .query(authorQuery)
+      .send({
+        manifest: createManifestForJPadKey(keyPath),
+        implementation: JSON.stringify({
+          partitions: [],
+          defaultValue: 'test',
+          valueType: 'string',
+          rules: [],
+        }),
+      })
+      .expect(200);
+
+    await pollUntil(
+      () => client.get(`/api/v2/values/${keyPath}`),
+      (res) => expect(res.body).to.eql('test'),
+    );
+  });
+
+  it('GET key with Etag', async () => {
+    await client.get(`/api/v2/keys/${keyPath}`).then((res) => {
+      etag = res.headers.etag;
+    });
+    expect(etag).to.be.not.undefined;
+  });
+
+  it('Update key with right Etag', async () => {
+    await client
+      .put(`/api/v2/keys/${keyPath}`)
+      .query(authorQuery)
+      .send({
+        manifest: createManifestForJPadKey(keyPath),
+        implementation: JSON.stringify({
+          partitions: [],
+          defaultValue: 'test2',
+          valueType: 'string',
+          rules: [],
+        }),
+      })
+      .set('If-Match', etag)
+      .expect(200);
+
+    await pollUntil(
+      () => client.get(`/api/v2/values/${keyPath}`),
+      (res) => expect(res.body).to.eql('test2'),
+    );
+  });
+
+  it('Update key with wrong Etag', async () => {
+    await client
+      .put(`/api/v2/keys/${keyPath}`)
+      .query(authorQuery)
+      .send({
+        manifest: createManifestForJPadKey(keyPath),
+        implementation: JSON.stringify({
+          partitions: [],
+          defaultValue: 'test3',
+          valueType: 'string',
+          rules: [],
+        }),
+      })
+      .set('If-Match', etag)
+      .expect(412);
+  });
+});

services/authoring/src/repositories/keys-repository.ts

@@ -1,4 +1,5 @@
 import path = require('path');
+import hash from 'object-hash';
 import Transactor from '../utils/transactor';
 import GitRepository from './git-repository';
 
@@ -40,8 +41,9 @@ function getPathForManifest(keyName) {
 }
 
 function getPathForSourceFile(manifest) {
-  return `implementations/${manifest.implementation.format}/${manifest.key_path}.${manifest.implementation.extension ||
-    manifest.implementation.format}`;
+  return `implementations/${manifest.implementation.format}/${manifest.key_path}.${
+    manifest.implementation.extension || manifest.implementation.format
+  }`;
 }
 
 function getKeyFromPath(keyPath) {
@@ -89,25 +91,25 @@ export default class KeysRepository {
   constructor(private _gitTransactionManager: Transactor<GitRepository>) {}
 
   getAllKeys() {
-    return this._gitTransactionManager.with(async gitRepo => {
+    return this._gitTransactionManager.with(async (gitRepo) => {
       const keyFiles = await gitRepo.listFiles('manifests');
 
       return keyFiles.map(getKeyFromPath);
     });
   }
 
   getAllManifests(prefix = '') {
-    return this._gitTransactionManager.with(async gitRepo => {
+    return this._gitTransactionManager.with(async (gitRepo) => {
       const normalizedPrefix = `${path.normalize(`manifests/${prefix}/.`)}`.replace(/\\/g, '/');
       const files = await gitRepo.listFiles(normalizedPrefix);
-      const manifestFiles = files.map(keyPath => `${normalizedPrefix}/${keyPath}`);
-      const manifests = await Promise.all(manifestFiles.map(pathForManifest => gitRepo.readFile(pathForManifest)));
+      const manifestFiles = files.map((keyPath) => `${normalizedPrefix}/${keyPath}`);
+      const manifests = await Promise.all(manifestFiles.map((pathForManifest) => gitRepo.readFile(pathForManifest)));
       return manifests.map(<any>JSON.parse);
     });
   }
 
   getKeyDetails(keyPath, { revision }: any = {}) {
-    return this._gitTransactionManager.with(async gitRepo => {
+    return this._gitTransactionManager.with(async (gitRepo) => {
       const manifest = await getManifestFile(keyPath, gitRepo, revision);
       return {
         manifest,
@@ -120,28 +122,28 @@ export default class KeysRepository {
   }
 
   getKeyManifest(keyPath, { revision }: any = {}) {
-    return this._gitTransactionManager.with(async gitRepo => {
+    return this._gitTransactionManager.with(async (gitRepo) => {
       const pathForManifest = getPathForManifest(keyPath);
       return JSON.parse(await gitRepo.readFile(pathForManifest, { revision }));
     });
   }
 
   getKeyRevisionHistory(keyPath, config) {
-    return this._gitTransactionManager.with(async gitRepo => {
+    return this._gitTransactionManager.with(async (gitRepo) => {
       const manifest = await getManifestFile(keyPath, gitRepo);
       return getRevisionHistory(manifest, gitRepo, config);
     });
   }
 
   updateKey(keyPath, manifest, fileImplementation, author) {
-    return this._gitTransactionManager.write(async gitRepo => {
+    return this._gitTransactionManager.write(async (gitRepo) => {
       await updateKey(gitRepo, keyPath, manifest, fileImplementation);
       return await gitRepo.commitAndPush(`Editor - updating ${keyPath}`, author);
     });
   }
 
   updateBulkKeys(files, author, commitMessage = 'Bulk update through API') {
-    return this._gitTransactionManager.write(async gitRepo => {
+    return this._gitTransactionManager.write(async (gitRepo) => {
       for (const file of files) {
         const content = await file.read();
         await gitRepo.updateFile(file.name, content);
@@ -151,7 +153,7 @@ export default class KeysRepository {
   }
 
   deleteKeys(keys: string[], author) {
-    return this._gitTransactionManager.write(async gitRepo => {
+    return this._gitTransactionManager.write(async (gitRepo) => {
       for (const keyPath of keys) {
         const manifest = await getManifestFile(keyPath, gitRepo);
         await gitRepo.deleteFile(getPathForManifest(keyPath));
@@ -164,6 +166,16 @@ export default class KeysRepository {
   }
 
   getRevision() {
-    return this._gitTransactionManager.with(gitRepo => gitRepo.getLastCommit());
+    return this._gitTransactionManager.with((gitRepo) => gitRepo.getLastCommit());
+  }
+
+  async getKeyETag(keyPath: string): Promise<string> {
+    const keyDetails = await this.getKeyDetails(keyPath);
+    return hash(keyDetails);
+  }
+
+  async validateKeyETag(keyPath: string, etag: string): Promise<boolean> {
+    const currentETag = await this.getKeyETag(keyPath);
+    return etag === currentETag;
   }
 }

services/authoring/src/routes/keys.ts

@@ -38,6 +38,7 @@ export class KeysController {
   @Path('/key')
   async getKey(@QueryParam('keyPath') keyPath: string, @QueryParam('revision') revision?: string): Promise<any> {
     try {
+      await this._setKeyETagHeader(keyPath);
       return await this.keysRepository.getKeyDetails(keyPath, { revision });
     } catch (err) {
       logger.error({ err, keyPath }, 'Error retrieving key');
@@ -55,14 +56,18 @@ export class KeysController {
     @QueryParam('author.email') email: string,
     newKeyModel: KeyUpdateModel,
   ): Promise<string> {
+    if (!(await this._handleKeyETagValidation(keyPath, newKeyModel))) return 'Conflict';
+
     const { implementation } = newKeyModel;
     let { manifest } = newKeyModel;
+
     manifest = Object.assign({ key_path: keyPath }, manifest);
     const oid = await this.keysRepository.updateKey(keyPath, manifest, implementation, {
       name,
       email,
     });
     addOid(this.context.response, oid);
+    await this._setKeyETagHeader(keyPath);
 
     return 'OK';
   }
@@ -76,6 +81,8 @@ export class KeysController {
     @QueryParam('author.email') email: string,
     additionalKeys?: string[],
   ): Promise<string> {
+    if (!(await this._handleKeyETagValidation(keyPath))) return 'Conflict';
+
     let keysToDelete = [keyPath];
     if (additionalKeys && Array.isArray(additionalKeys)) {
       keysToDelete = keysToDelete.concat(additionalKeys);
@@ -120,4 +127,24 @@ export class KeysController {
   async getDependents(@QueryParam('keyPath') keyPath: string): Promise<any> {
     return await searchIndex.dependents(keyPath);
   }
+
+  private async _setKeyETagHeader(keyPath: string): Promise<void> {
+    const etag = await this.keysRepository.getKeyETag(keyPath);
+    this.context.response.setHeader('ETag', etag);
+  }
+
+  private async _handleKeyETagValidation(keyPath: string, newKeyModel?: KeyUpdateModel): Promise<boolean> {
+    const etag = this.context.request.header('If-Match');
+    if (!etag) return true;
+
+    const validationKeypath =
+      newKeyModel?.manifest?.implementation?.type === 'alias' ? newKeyModel.manifest.implementation.key : keyPath;
+
+    if (!(await this.keysRepository.validateKeyETag(validationKeypath, etag))) {
+      this.context.response.sendStatus(412);
+      return false;
+    }
+
+    return true;
+  }
 }