Skip to content

Commit

Permalink
Prevent perpetual use of expired auth signature; remove superfluous A…
Browse files Browse the repository at this point in the history
…PI functions (#567)
  • Loading branch information
derekpierre authored Aug 27, 2024
2 parents 43786ce + 236f296 commit a2f8e6b
Show file tree
Hide file tree
Showing 13 changed files with 172 additions and 127 deletions.
13 changes: 0 additions & 13 deletions examples/taco/nodejs/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
encrypt,
fromBytes,
initialize,
isAuthorized,
toBytes,
toHexString,
} from '@nucypher/taco';
Expand Down Expand Up @@ -81,18 +80,6 @@ const encryptToBytes = async (messageString: string) => {
encryptorSigner,
);

// Note: Not actually needed but used by CI for checking contract compatibility.
// Calling it after the encryption because we need material from messageKit.
const isEncryptorAuthenticated = await isAuthorized(
provider,
domain,
messageKit,
ritualId,
);
if (!isEncryptorAuthenticated) {
throw new Error('Not authorized');
}

return messageKit.toBytes();
};

Expand Down
6 changes: 3 additions & 3 deletions packages/pre/test/acceptance/alice-grants.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
bytesEqual,
fakePorterUri,
fakeProvider,
fakeSigner,
fakeUrsulas,
fromBytes,
mockGetUrsulas,
Expand Down Expand Up @@ -70,9 +69,10 @@ describe('story: alice shares message with bob through policy', () => {
startDate,
endDate,
};
const provider = fakeProvider();
policy = await alice.grant(
fakeProvider(),
fakeSigner(),
provider,
provider.getSigner(),
domains.DEVNET,
fakePorterUri,
policyParams,
Expand Down
3 changes: 1 addition & 2 deletions packages/pre/test/acceptance/delay-enact.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import {
bytesEqual,
fakePorterUri,
fakeProvider,
fakeSigner,
fakeUrsulas,
mockGetUrsulas,
} from '@nucypher/test-utils';
Expand Down Expand Up @@ -61,7 +60,7 @@ describe('story: alice creates a policy but someone else enacts it', () => {

const enacted = await preEnactedPolicy.enact(
provider,
fakeSigner(),
provider.getSigner(),
domains.DEVNET,
);
expect(enacted.txHash).toBeDefined();
Expand Down
21 changes: 20 additions & 1 deletion packages/taco-auth/src/providers/eip4361/eip4361.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,13 @@ export class EIP4361AuthProvider {
// If we have a signature in localStorage, return it
const maybeSignature = this.storage.getAuthSignature(storageKey);
if (maybeSignature) {
return maybeSignature;
// check whether older than node freshness requirement
if (this.isMessageExpired(maybeSignature.typedData)) {
// clear signature so that it will be recreated and stored
this.storage.clear(storageKey);
} else {
return maybeSignature;
}
}

// If at this point we didn't return, we need to create a new message
Expand All @@ -61,6 +67,19 @@ export class EIP4361AuthProvider {
return authMessage;
}

private isMessageExpired(message: string): boolean {
const siweMessage = new SiweMessage(message);
if (!siweMessage.issuedAt) {
// don't expect to ever happen; but just in case
return false;
}

const twoHourWindow = new Date(siweMessage.issuedAt);
twoHourWindow.setHours(twoHourWindow.getHours() + 2);
const now = new Date();
return twoHourWindow < now;
}

private async createSIWEAuthMessage(): Promise<AuthSignature> {
const address = await this.signer.getAddress();
const { domain, uri } = this.providerParams;
Expand Down
14 changes: 14 additions & 0 deletions packages/taco-auth/src/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ interface IStorage {
getItem(key: string): string | null;

setItem(key: string, value: string): void;

removeItem(key: string): void;
}

class BrowserStorage implements IStorage {
Expand All @@ -14,6 +16,10 @@ class BrowserStorage implements IStorage {
public setItem(key: string, value: string): void {
localStorage.setItem(key, value);
}

public removeItem(key: string): void {
localStorage.removeItem(key);
}
}

class NodeStorage implements IStorage {
Expand All @@ -26,6 +32,10 @@ class NodeStorage implements IStorage {
public setItem(key: string, value: string): void {
this.storage[key] = value;
}

public removeItem(key: string) {
delete this.storage[key];
}
}

export class LocalStorage {
Expand All @@ -50,4 +60,8 @@ export class LocalStorage {
const asJson = JSON.stringify(authSignature);
this.storage.setItem(key, asJson);
}

public clear(key: string): void {
this.storage.removeItem(key);
}
}
88 changes: 84 additions & 4 deletions packages/taco-auth/test/auth-provider.test.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
import {
bobSecretKeyBytes,
fakeProvider,
fakeSigner,
TEST_SIWE_PARAMS,
} from '@nucypher/test-utils';
import { SiweMessage } from 'siwe';
import { describe, expect, it } from 'vitest';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

import { EIP4361AuthProvider } from '../src/providers';
import {
EIP4361AuthProvider,
SingleSignOnEIP4361AuthProvider,
} from '../src/providers';
import { EIP4361TypedDataSchema } from '../src/providers/eip4361/common';

describe('auth provider', () => {
const provider = fakeProvider(bobSecretKeyBytes);
const signer = fakeSigner(bobSecretKeyBytes);
const signer = provider.getSigner();
const eip4361Provider = new EIP4361AuthProvider(
provider,
signer,
Expand Down Expand Up @@ -52,3 +54,81 @@ describe('auth provider', () => {
).toThrow();
});
});

describe('auth provider caching', () => {
beforeEach(() => {
// tell vitest we use mocked time
vi.useFakeTimers();
});

afterEach(() => {
// restoring date after each test run
vi.useRealTimers();
});

const provider = fakeProvider(bobSecretKeyBytes);
const signer = provider.getSigner();
const eip4361Provider = new EIP4361AuthProvider(
provider,
signer,
TEST_SIWE_PARAMS,
);

it('caches auth signature, but regenerates when expired', async () => {
const createAuthSignatureSpy = vi.spyOn(
eip4361Provider,
'createSIWEAuthMessage',
);

const typedSignature = await eip4361Provider.getOrCreateAuthSignature();
expect(createAuthSignatureSpy).toHaveBeenCalledTimes(1);

const typedSignatureSecondCall =
await eip4361Provider.getOrCreateAuthSignature();
// auth signature not expired, so spy is not called a 2nd time
expect(createAuthSignatureSpy).toHaveBeenCalledTimes(1);
expect(typedSignatureSecondCall).toEqual(typedSignature);

// time travel to 2h:1m in the future; auth signature is now expired
const now = new Date();
now.setHours(now.getHours() + 2, now.getMinutes() + 1);
vi.setSystemTime(now);

const typedSignatureThirdCall =
await eip4361Provider.getOrCreateAuthSignature();
// auth signature is now expired, so spy is called a 2nd time
expect(createAuthSignatureSpy).toHaveBeenCalledTimes(2);
expect(typedSignatureThirdCall).not.toEqual(typedSignature);
});
});

describe('single sign-on auth provider', async () => {
const provider = fakeProvider(bobSecretKeyBytes);
const signer = provider.getSigner();

const eip4361Provider = new EIP4361AuthProvider(
provider,
signer,
TEST_SIWE_PARAMS,
);
const originalTypedSignature =
await eip4361Provider.getOrCreateAuthSignature();

it('use existing SIWE message', async () => {
const originalSiweMessage = originalTypedSignature.typedData;
const originalSiweSignature = originalTypedSignature.signature;

const singleSignOnProvider =
await SingleSignOnEIP4361AuthProvider.fromExistingSiweInfo(
originalSiweMessage,
originalSiweSignature,
);

const typedSignature =
await singleSignOnProvider.getOrCreateAuthSignature();
expect(typedSignature.typedData).toEqual(originalSiweMessage);
expect(typedSignature.signature).toEqual(originalSiweSignature);
expect(typedSignature.address).toEqual(await signer.getAddress());
expect(typedSignature.scheme).toEqual('EIP4361');
});
});
4 changes: 2 additions & 2 deletions packages/taco/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ export {
} from '@nucypher/shared';

export * as conditions from './conditions';
// TODO(#324): Expose registerEncrypters from taco API
export { decrypt, encrypt, encryptWithPublicKey, isAuthorized } from './taco';

export { decrypt, encrypt, encryptWithPublicKey } from './taco';

// TODO: Remove this re-export once `@nucypher/taco-auth` is mature and published
export {
Expand Down
45 changes: 0 additions & 45 deletions packages/taco/src/taco.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,10 @@ import {
ThresholdMessageKit,
} from '@nucypher/nucypher-core';
import {
ChecksumAddress,
DkgCoordinatorAgent,
Domain,
fromHexString,
getPorterUris,
GlobalAllowListAgent,
PorterClient,
toBytes,
} from '@nucypher/shared';
Expand Down Expand Up @@ -160,46 +158,3 @@ export const decrypt = async (
context,
);
};

/**
* Checks if the encryption from the provided messageKit is authorized for the specified ritual.
*
* @export
* @param {ethers.providers.Provider} provider - Instance of ethers provider which is used to interact with
* your selected network.
* @param {Domain} domain - The domain which was used to encrypt the network. Must match the `ritualId`.
* @param {ThresholdMessageKit} messageKit - The encrypted message kit to be checked.
* @param {number} ritualId - The ID of the DKG Ritual under which the messageKit was supposedly encrypted.
*
* @returns {Promise<boolean>} Returns a Promise that resolves with the authorization status.
* True if authorized, false otherwise
*/
export const isAuthorized = async (
provider: ethers.providers.Provider,
domain: Domain,
messageKit: ThresholdMessageKit,
ritualId: number,
): Promise<boolean> =>
DkgCoordinatorAgent.isEncryptionAuthorized(
provider,
domain,
ritualId,
messageKit,
);

// TODO is this still valid and actually needed? should we remove this?
export const registerEncrypters = async (
provider: ethers.providers.Provider,
signer: ethers.Signer,
domain: Domain,
ritualId: number,
encrypters: ChecksumAddress[],
): Promise<void> => {
await GlobalAllowListAgent.registerEncrypters(
provider,
signer,
domain,
ritualId,
encrypters,
);
};
14 changes: 5 additions & 9 deletions packages/taco/test/conditions/context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,7 @@ import {
USER_ADDRESS_PARAM_DEFAULT,
USER_ADDRESS_PARAM_EXTERNAL_EIP4361,
} from '@nucypher/taco-auth';
import {
fakeAuthProviders,
fakeProvider,
fakeSigner,
} from '@nucypher/test-utils';
import { fakeAuthProviders, fakeProvider } from '@nucypher/test-utils';
import { ethers } from 'ethers';
import { beforeAll, describe, expect, it, vi } from 'vitest';

Expand Down Expand Up @@ -308,8 +304,8 @@ describe('context', () => {

// TODO: Move to a separate file
describe('No authentication provider', () => {
let provider: ethers.providers.Provider;
let signer: ethers.Signer;
let provider: ethers.providers.Web3Provider;
let signer: ethers.providers.JsonRpcSigner;
let authProviders: Record<string, AuthProvider>;

async function testEIP4361AuthSignature(
Expand Down Expand Up @@ -337,8 +333,8 @@ describe('No authentication provider', () => {
beforeAll(async () => {
await initialize();
provider = fakeProvider();
signer = fakeSigner();
authProviders = await fakeAuthProviders();
signer = provider.getSigner();
authProviders = await fakeAuthProviders(signer);
});

it('throws an error if there is no auth provider', () => {
Expand Down
5 changes: 2 additions & 3 deletions packages/taco/test/taco.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
fakeDkgFlow,
fakePorterUri,
fakeProvider,
fakeSigner,
fakeTDecFlow,
mockGetRitualIdFromPublicKey,
mockTacoDecrypt,
Expand Down Expand Up @@ -48,7 +47,7 @@ describe('taco', () => {
const mockedDkg = fakeDkgFlow(FerveoVariant.precomputed, 0, 4, 4);
const mockedDkgRitual = fakeDkgRitual(mockedDkg);
const provider = fakeProvider(aliceSecretKeyBytes);
const signer = fakeSigner(aliceSecretKeyBytes);
const signer = provider.getSigner();
const getFinalizedRitualSpy = mockGetActiveRitual(mockedDkgRitual);

const messageKit = await taco.encrypt(
Expand Down Expand Up @@ -110,7 +109,7 @@ describe('taco', () => {
const mockedDkg = fakeDkgFlow(FerveoVariant.precomputed, 0, 4, 4);
const mockedDkgRitual = fakeDkgRitual(mockedDkg);
const provider = fakeProvider(aliceSecretKeyBytes);
const signer = fakeSigner(aliceSecretKeyBytes);
const signer = provider.getSigner();
const getFinalizedRitualSpy = mockGetActiveRitual(mockedDkgRitual);

const customParamKey = ':nftId';
Expand Down
Loading

0 comments on commit a2f8e6b

Please sign in to comment.