refactor: Clean up internal storage

Each IDP class using storage now has a different storage. This way those classes don't have to worry about clashing keys anymore. All internal storage is now in the /.internal/ container, thereby making it easier to take the location of the internal data into account: only 1 path needs to be blocked and a regex router handling internal data differently only has to match 1 path as well.
CommunitySolidServer · Sep 8, 2021 · 1e1edd5 · 1e1edd5
1 parent 60fc273
commit 1e1edd5
Show file tree

Hide file tree

Showing 14 changed files with 90 additions and 78 deletions.
diff --git a/config/identity/handler/account-store/default.json b/config/identity/handler/account-store/default.json
@@ -5,10 +5,9 @@
       "comment": "The storage adapter that persists usernames, passwords, etc.",
       "@id": "urn:solid-server:auth:password:AccountStore",
       "@type": "BaseAccountStore",
-      "args_storageName": "/idp/email-password-db",
-      "args_saltRounds": 10,
-      "args_storage": {
-        "@id": "urn:solid-server:default:IdpStorage"
+      "saltRounds": 10,
+      "storage": {
+        "@id": "urn:solid-server:default:AccountStorage"
       }
     }
   ]

diff --git a/config/identity/handler/adapter-factory/webid.json b/config/identity/handler/adapter-factory/webid.json
@@ -7,8 +7,7 @@
       "@type": "WebIdAdapterFactory",
       "source": {
         "@type": "ExpiringAdapterFactory",
-        "args_storageName": "/idp/oidc",
-        "args_storage": { "@id": "urn:solid-server:default:ExpiringIdpStorage" }
+        "storage": { "@id": "urn:solid-server:default:IdpAdapterStorage" }
       },
       "converter": { "@id": "urn:solid-server:default:RepresentationConverter" }
     }

diff --git a/config/identity/handler/default.json b/config/identity/handler/default.json
@@ -4,7 +4,6 @@
     "files-scs:config/identity/handler/account-store/default.json",
     "files-scs:config/identity/handler/adapter-factory/webid.json",
     "files-scs:config/identity/handler/interaction/routes.json",
-    "files-scs:config/identity/handler/key-value/storage.json",
     "files-scs:config/identity/handler/provider-factory/identity.json"
   ],
   "@graph": [

diff --git a/config/identity/handler/key-value/storage.json b/config/identity/handler/key-value/storage.json
diff --git a/config/identity/handler/provider-factory/identity.json b/config/identity/handler/provider-factory/identity.json
@@ -11,7 +11,7 @@
       "args_adapterFactory": { "@id": "urn:solid-server:default:IdpAdapterFactory" },
       "args_baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" },
       "args_idpPath": "/idp",
-      "args_storage": { "@id": "urn:solid-server:default:IdpStorage" },
+      "args_storage": { "@id": "urn:solid-server:default:IdpKeyStorage" },
       "args_errorHandler": { "@id": "urn:solid-server:default:ErrorHandler" },
       "args_responseWriter": { "@id": "urn:solid-server:default:ResponseWriter" },
       "config": {

diff --git a/config/identity/ownership/token.json b/config/identity/ownership/token.json
@@ -6,7 +6,19 @@
       "@id": "urn:solid-server:auth:password:OwnershipValidator",
       "@type": "TokenOwnershipValidator",
       "converter": { "@id": "urn:solid-server:default:RepresentationConverter" },
-      "storage": { "@id": "urn:solid-server:default:ExpiringIdpStorage" }
+      "storage": { "@id": "urn:solid-server:default:ExpiringTokenStorage" }
+    },
+
+    {
+      "comment": "Stores expiring data. This class has a `finalize` function that needs to be called after stopping the server.",
+      "@id": "urn:solid-server:default:ExpiringTokenStorage",
+      "@type": "WrappedExpiringStorage",
+      "source": { "@id": "urn:solid-server:default:IdpTokenStorage" }
+    },
+    {
+      "comment": "Makes sure the expiring storage cleanup timer is stopped when the application needs to stop.",
+      "@id": "urn:solid-server:default:Finalizer",
+      "ParallelFinalizer:_finalizers": [ { "@id": "urn:solid-server:default:ExpiringTokenStorage" } ]
     }
   ]
 }
diff --git a/config/storage/key-value/memory.json b/config/storage/key-value/memory.json
@@ -10,8 +10,23 @@
       "@type": "MemoryMapStorage"
     },
     {
-      "comment": "Storage used by the IDP component.",
-      "@id": "urn:solid-server:default:IdpStorage",
+      "comment": "Storage used by the IDP adapter.",
+      "@id": "urn:solid-server:default:IdpAdapterStorage",
+      "@type": "MemoryMapStorage"
+    },
+    {
+      "comment": "Storage used for the IDP keys.",
+      "@id": "urn:solid-server:default:IdpKeyStorage",
+      "@type": "MemoryMapStorage"
+    },
+    {
+      "comment": "Storage used for IDP ownership tokens.",
+      "@id": "urn:solid-server:default:IdpTokenStorage",
+      "@type": "MemoryMapStorage"
+    },
+    {
+      "comment": "Storage used for account management.",
+      "@id": "urn:solid-server:default:AccountStorage",
       "@type": "MemoryMapStorage"
     }
   ]

diff --git a/config/storage/key-value/resource-store.json b/config/storage/key-value/resource-store.json
@@ -13,26 +13,46 @@
       "@type": "JsonResourceStorage",
       "source": { "@id": "urn:solid-server:default:ResourceStore_Backend" },
       "baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" },
-      "container": "/locks/"
+      "container": "/.internal/locks/"
     },
     {
-      "comment": "Storage used by the IDP component.",
-      "@id": "urn:solid-server:default:IdpStorage",
+      "comment": "Storage used by the IDP adapter.",
+      "@id": "urn:solid-server:default:IdpAdapterStorage",
       "@type": "JsonResourceStorage",
       "source": { "@id": "urn:solid-server:default:ResourceStore" },
       "baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" },
-      "container": "/idp/data/"
+      "container": "/.internal/idp/adapter/"
+    },
+    {
+      "comment": "Storage used for the IDP keys.",
+      "@id": "urn:solid-server:default:IdpKeyStorage",
+      "@type": "JsonResourceStorage",
+      "source": { "@id": "urn:solid-server:default:ResourceStore" },
+      "baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" },
+      "container": "/.internal/idp/keys/"
+    },
+    {
+      "comment": "Storage used for IDP ownership tokens.",
+      "@id": "urn:solid-server:default:IdpTokenStorage",
+      "@type": "JsonResourceStorage",
+      "source": { "@id": "urn:solid-server:default:ResourceStore" },
+      "baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" },
+      "container": "/.internal/idp/tokens/"
+    },
+    {
+      "comment": "Storage used for account management.",
+      "@id": "urn:solid-server:default:AccountStorage",
+      "@type": "JsonResourceStorage",
+      "source": { "@id": "urn:solid-server:default:ResourceStore" },
+      "baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" },
+      "container": "/.internal/accounts/"
     },
     {
       "comment": "Block external access to the storage containers to avoid exposing internal data.",
       "@id": "urn:solid-server:default:PathBasedAuthorizer",
       "PathBasedAuthorizer:_paths": [
         {
-          "PathBasedAuthorizer:_paths_key": "^/locks(/.*)?$",
-          "PathBasedAuthorizer:_paths_value": { "@type": "DenyAllAuthorizer" }
-        },
-        {
-          "PathBasedAuthorizer:_paths_key": "^/idp/data(/.*)?$",
+          "PathBasedAuthorizer:_paths_key": "^/.internal(/.*)?$",
           "PathBasedAuthorizer:_paths_value": { "@type": "DenyAllAuthorizer" }
         }
       ]

diff --git a/src/identity/configuration/IdentityProviderFactory.ts b/src/identity/configuration/IdentityProviderFactory.ts
@@ -47,6 +47,9 @@ export interface IdentityProviderFactoryArgs {
   responseWriter: ResponseWriter;
 }
 
+const JWKS_KEY = 'jwks';
+const COOKIES_KEY = 'cookie-secret';
+
 /**
  * Creates an OIDC Provider based on the provided configuration and parameters.
  * The provider will be cached and returned on subsequent calls.
@@ -138,8 +141,7 @@ export class IdentityProviderFactory implements ProviderFactory {
    */
   private async generateJwks(): Promise<{ keys: JWK[] }> {
     // Check to see if the keys are already saved
-    const key = `${this.idpPath}/jwks`;
-    const jwks = await this.storage.get(key) as { keys: JWK[] } | undefined;
+    const jwks = await this.storage.get(JWKS_KEY) as { keys: JWK[] } | undefined;
     if (jwks) {
       return jwks;
     }
@@ -152,7 +154,7 @@ export class IdentityProviderFactory implements ProviderFactory {
     // which is why we convert it into a plain object here.
     // Potentially this can be changed at a later point in time to `{ keys: [ jwk ]}`.
     const newJwks = { keys: [{ ...jwk }]};
-    await this.storage.set(key, newJwks);
+    await this.storage.set(JWKS_KEY, newJwks);
     return newJwks;
   }
 
@@ -162,14 +164,13 @@ export class IdentityProviderFactory implements ProviderFactory {
    */
   private async generateCookieKeys(): Promise<string[]> {
     // Check to see if the keys are already saved
-    const key = `${this.idpPath}/cookie-secret`;
-    const cookieSecret = await this.storage.get(key);
+    const cookieSecret = await this.storage.get(COOKIES_KEY);
     if (Array.isArray(cookieSecret)) {
       return cookieSecret;
     }
     // If they are not, generate and save them
     const newCookieSecret = [ randomBytes(64).toString('hex') ];
-    await this.storage.set(key, newCookieSecret);
+    await this.storage.set(COOKIES_KEY, newCookieSecret);
     return newCookieSecret;
   }
 

diff --git a/src/identity/interaction/email-password/storage/BaseAccountStore.ts b/src/identity/interaction/email-password/storage/BaseAccountStore.ts
@@ -25,39 +25,31 @@ export interface ForgotPasswordPayload {
 
 export type EmailPasswordData = AccountPayload | ForgotPasswordPayload;
 
-export interface BaseAccountStoreArgs {
-  storageName: string;
-  storage: KeyValueStorage<string, EmailPasswordData>;
-  saltRounds: number;
-}
-
 /**
  * A EmailPasswordStore that uses a KeyValueStorage
  * to persist its information.
  */
 export class BaseAccountStore implements AccountStore {
-  private readonly storageName: string;
   private readonly storage: KeyValueStorage<string, EmailPasswordData>;
   private readonly saltRounds: number;
 
-  public constructor(args: BaseAccountStoreArgs) {
-    this.storageName = args.storageName;
-    this.storage = args.storage;
-    this.saltRounds = args.saltRounds;
+  public constructor(storage: KeyValueStorage<string, EmailPasswordData>, saltRounds: number) {
+    this.storage = storage;
+    this.saltRounds = saltRounds;
   }
 
   /**
    * Generates a ResourceIdentifier to store data for the given email.
    */
   private getAccountResourceIdentifier(email: string): string {
-    return `${this.storageName}/account/${encodeURIComponent(email)}`;
+    return `account/${encodeURIComponent(email)}`;
   }
 
   /**
    * Generates a ResourceIdentifier to store data for the given recordId.
    */
   private getForgotPasswordRecordResourceIdentifier(recordId: string): string {
-    return `${this.storageName}/forgot-password-resource-identifier/${encodeURIComponent(recordId)}`;
+    return `forgot-password-resource-identifier/${encodeURIComponent(recordId)}`;
   }
 
   /**

diff --git a/src/identity/storage/ExpiringAdapterFactory.ts b/src/identity/storage/ExpiringAdapterFactory.ts
@@ -3,42 +3,35 @@ import { getLoggerFor } from '../../logging/LogUtil';
 import type { ExpiringStorage } from '../../storage/keyvalue/ExpiringStorage';
 import type { AdapterFactory } from './AdapterFactory';
 
-export interface ExpiringAdapterArgs {
-  storageName: string;
-  storage: ExpiringStorage<string, unknown>;
-}
-
 /**
  * An IDP storage adapter that uses an ExpiringStorage
  * to persist data.
  */
 export class ExpiringAdapter implements Adapter {
   protected readonly logger = getLoggerFor(this);
 
-  private readonly storageName: string;
   private readonly name: string;
   private readonly storage: ExpiringStorage<string, unknown>;
 
-  public constructor(name: string, args: ExpiringAdapterArgs) {
+  public constructor(name: string, storage: ExpiringStorage<string, unknown>) {
     this.name = name;
-    this.storageName = args.storageName;
-    this.storage = args.storage;
+    this.storage = storage;
   }
 
   private grantKeyFor(id: string): string {
-    return `${this.storageName}/grant/${encodeURIComponent(id)}`;
+    return `grant/${encodeURIComponent(id)}`;
   }
 
   private userCodeKeyFor(userCode: string): string {
-    return `${this.storageName}/user_code/${encodeURIComponent(userCode)}`;
+    return `user_code/${encodeURIComponent(userCode)}`;
   }
 
   private uidKeyFor(uid: string): string {
-    return `${this.storageName}/uid/${encodeURIComponent(uid)}`;
+    return `uid/${encodeURIComponent(uid)}`;
   }
 
   private keyFor(id: string): string {
-    return `${this.storageName}/${this.name}/${encodeURIComponent(id)}`;
+    return `${this.name}/${encodeURIComponent(id)}`;
   }
 
   public async upsert(id: string, payload: AdapterPayload, expiresIn?: number): Promise<void> {
@@ -117,13 +110,13 @@ export class ExpiringAdapter implements Adapter {
  * The factory for a ExpiringStorageAdapter
  */
 export class ExpiringAdapterFactory implements AdapterFactory {
-  private readonly args: ExpiringAdapterArgs;
+  private readonly storage: ExpiringStorage<string, unknown>;
 
-  public constructor(args: ExpiringAdapterArgs) {
-    this.args = args;
+  public constructor(storage: ExpiringStorage<string, unknown>) {
+    this.storage = storage;
   }
 
   public createStorageAdapter(name: string): ExpiringAdapter {
-    return new ExpiringAdapter(name, this.args);
+    return new ExpiringAdapter(name, this.storage);
   }
 }
diff --git a/test/unit/identity/configuration/IdentityProviderFactory.test.ts b/test/unit/identity/configuration/IdentityProviderFactory.test.ts
@@ -158,7 +158,7 @@ describe('An IdentityProviderFactory', (): void => {
     expect(result1.config.jwks).toEqual(result2.config.jwks);
     expect(storage.get).toHaveBeenCalledTimes(4);
     expect(storage.set).toHaveBeenCalledTimes(2);
-    expect(storage.set).toHaveBeenCalledWith('/idp/jwks', result1.config.jwks);
-    expect(storage.set).toHaveBeenCalledWith('/idp/cookie-secret', result1.config.cookies?.keys);
+    expect(storage.set).toHaveBeenCalledWith('jwks', result1.config.jwks);
+    expect(storage.set).toHaveBeenCalledWith('cookie-secret', result1.config.cookies?.keys);
   });
 });
diff --git a/test/unit/identity/interaction/email-password/storage/BaseAccountStore.test.ts b/test/unit/identity/interaction/email-password/storage/BaseAccountStore.test.ts
@@ -5,7 +5,6 @@ import { BaseAccountStore } from '../../../../../../src/identity/interaction/ema
 import type { KeyValueStorage } from '../../../../../../src/storage/keyvalue/KeyValueStorage';
 
 describe('A BaseAccountStore', (): void => {
-  const storageName = '/mail/storage';
   let storage: KeyValueStorage<string, EmailPasswordData>;
   const saltRounds = 11;
   let store: BaseAccountStore;
@@ -21,7 +20,7 @@ describe('A BaseAccountStore', (): void => {
       delete: jest.fn((id: string): any => map.delete(id)),
     } as any;
 
-    store = new BaseAccountStore({ storageName, storage, saltRounds });
+    store = new BaseAccountStore(storage, saltRounds);
   });
 
   it('can create accounts.', async(): Promise<void> => {

diff --git a/test/unit/identity/storage/ExpiringAdapterFactory.test.ts b/test/unit/identity/storage/ExpiringAdapterFactory.test.ts
@@ -7,7 +7,6 @@ import type { ExpiringStorage } from '../../../../src/storage/keyvalue/ExpiringS
 jest.useFakeTimers();
 
 describe('An ExpiringAdapterFactory', (): void => {
-  const storageName = '/storage';
   const name = 'nnaammee';
   const id = 'http://alice.test.com/card#me';
   const grantId = 'grant123456';
@@ -27,7 +26,7 @@ describe('An ExpiringAdapterFactory', (): void => {
       delete: jest.fn().mockImplementation((key: string): any => map.delete(key)),
     } as any;
 
-    factory = new ExpiringAdapterFactory({ storageName, storage });
+    factory = new ExpiringAdapterFactory(storage);
     adapter = factory.createStorageAdapter(name);
   });