Skip to content

Commit

Permalink
feat(eip4361): allow for reusing siwe messages
Browse files Browse the repository at this point in the history
  • Loading branch information
@piotr-roslaniec
piotr-roslaniec committed Jun 12, 2024
1 parent c8100d8 commit a472dec
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 47 deletions.
5 changes: 4 additions & 1 deletion packages/taco/src/conditions/const.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { ChainId } from '@nucypher/shared';
export const USER_ADDRESS_PARAM_DEFAULT = ':userAddress';
export const USER_ADDRESS_PARAM_EIP712 = ':userAddressEIP712';
export const USER_ADDRESS_PARAM_EIP4361 = ':userAddressEIP4361';
export const USER_ADDRESS_PARAM_EXTERNAL_EIP4361 =
':userAddressExternalEIP4361';

export const ETH_ADDRESS_REGEXP = new RegExp('^0x[a-fA-F0-9]{40}$');

Expand All @@ -21,7 +23,8 @@ export const SUPPORTED_CHAIN_IDS = [
export const USER_ADDRESS_PARAMS = [
USER_ADDRESS_PARAM_EIP712,
USER_ADDRESS_PARAM_EIP4361,
// this should always be last
USER_ADDRESS_PARAM_EXTERNAL_EIP4361,
// Ordering matters, this should always be last
USER_ADDRESS_PARAM_DEFAULT,
];

Expand Down
79 changes: 47 additions & 32 deletions packages/taco/src/conditions/context/context.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ import {
USER_ADDRESS_PARAM_DEFAULT,
USER_ADDRESS_PARAM_EIP4361,
USER_ADDRESS_PARAM_EIP712,
USER_ADDRESS_PARAM_EXTERNAL_EIP4361,
} from '../const';

export type CustomContextParam = string | number | boolean;
export type CustomContextParam = string | number | boolean | AuthSignature;
export type ContextParam = CustomContextParam | AuthSignature;

const ERR_RESERVED_PARAM = (key: string) =>
Expand All @@ -32,6 +33,8 @@ const ERR_MISSING_CONTEXT_PARAMS = (params: string[]) =>
`Missing custom context parameter(s): ${params.join(', ')}`;
const ERR_UNKNOWN_CONTEXT_PARAMS = (params: string[]) =>
`Unknown custom context parameter(s): ${params.join(', ')}`;
const ERR_CONFLICTING_CONTEXT_PARAMS = (params: string[]) =>
`Cannot use ${params.join(', ')} together`;

export class ConditionContext {
private readonly eip712SignatureProvider?: EIP712SignatureProvider;
Expand Down Expand Up @@ -102,26 +105,6 @@ export class ConditionContext {
return parameters;
};

// today
// :user_address -> (:user_address, eip712)

// in future
// :user_address -> (:user_address, schema: eip712)
// :user_address_eip712 -> (:user_address, schema: eip712)
// :user_address_eip4361 -> (:user_address, schema: eip4361)

// HasNFTBalance({
// balanceOf(":user_address") > 10
// })

// HasNFTBalance({
// balanceOf(":user_address_eip712") > 10
// })

// HasNFTBalance({
// balanceOf(":user_address_eip4361") > 10
// })

private async fillContextParameters(
requestedParameters: Set<string>,
): Promise<Record<string, ContextParam>> {
Expand All @@ -133,43 +116,67 @@ export class ConditionContext {
// TODO: Figure out if we can simplify/deduplicate this logic for every
// authentication method we use

// Handle legacy EIP712 signature denoted by ":userAddress"
// Handle a legacy EIP712 signature denoted by ":userAddress"
if (requestedParameters.has(USER_ADDRESS_PARAM_DEFAULT)) {
if (!this.eip712SignatureProvider) {
throw new Error(ERR_SIGNER_REQUIRED(USER_ADDRESS_PARAM_EIP712));
}
parameters[USER_ADDRESS_PARAM_DEFAULT] =
await this.eip712SignatureProvider.getOrCreateWalletSignature();
// Remove from requested parameters
requestedParameters.delete(USER_ADDRESS_PARAM_DEFAULT);
}

// Handle a new alias for EIP712, ":userAddressEIP712"
// Handle an alias for EIP712, ":userAddressEIP712"
if (requestedParameters.has(USER_ADDRESS_PARAM_EIP712)) {
if (!this.eip712SignatureProvider) {
throw new Error(ERR_SIGNER_REQUIRED(USER_ADDRESS_PARAM_EIP712));
}
parameters[USER_ADDRESS_PARAM_EIP712] =
await this.eip712SignatureProvider.getOrCreateWalletSignature();
// Remove from requested parameters
requestedParameters.delete(USER_ADDRESS_PARAM_EIP712);
}

// Handle a new method, EIP4361, with a ":userAddressEIP4361" alias
// Handle a EIP4361 method, with a ":userAddressEIP4361" alias
if (requestedParameters.has(USER_ADDRESS_PARAM_EIP4361)) {
if (!this.eip4361SignatureProvider) {
throw new Error(ERR_SIGNER_REQUIRED(USER_ADDRESS_PARAM_EIP4361));
}
parameters[USER_ADDRESS_PARAM_EIP4361] =
await this.eip4361SignatureProvider.getOrCreateWalletSignature();
// Remove from requested parameters
requestedParameters.delete(USER_ADDRESS_PARAM_EIP4361);
}

// Handle a special case for EIP4361, with a ":userAddressExternalEIP4361" alias
// If we have both EIP4361 and an external EIP4361, we should throw
if (
requestedParameters.has(USER_ADDRESS_PARAM_EIP4361) &&
requestedParameters.has(USER_ADDRESS_PARAM_EXTERNAL_EIP4361)
) {
throw new Error(
ERR_CONFLICTING_CONTEXT_PARAMS([
USER_ADDRESS_PARAM_EIP4361,
USER_ADDRESS_PARAM_EXTERNAL_EIP4361,
]),
);
}

// Now, we need to handle a special case for external EIP4361
// We expect to find it in the custom parameters
if (requestedParameters.has(USER_ADDRESS_PARAM_EXTERNAL_EIP4361)) {
if (!this.customParameters[USER_ADDRESS_PARAM_EXTERNAL_EIP4361]) {
throw new Error(
ERR_MISSING_CONTEXT_PARAMS([USER_ADDRESS_PARAM_EXTERNAL_EIP4361]),
);
}
// Notice that we are using the same value for both EIP4361 and external EIP4361
parameters[USER_ADDRESS_PARAM_EIP4361] =
this.customParameters[USER_ADDRESS_PARAM_EXTERNAL_EIP4361];
// TODO: Avoid duplication?
// delete this.customParameters[USER_ADDRESS_PARAM_EXTERNAL_EIP4361];
}

// Fill in custom parameters
for (const key in this.customParameters) {
parameters[key] = this.customParameters[key];
}

return parameters;
}

Expand All @@ -181,12 +188,20 @@ export class ConditionContext {
// First, we want to find all the parameters we need to add
const requestedParameters = new Set<string>();

// Search conditions for parameters
// Check return value test
if (condition.returnValueTest) {
const rvt = condition.returnValueTest.value;
if (this.isContextParameter(rvt)) {
// Return value test can be a single parameter or an array of parameters
if (Array.isArray(rvt)) {
rvt.forEach((value) => {
if (this.isContextParameter(value)) {
requestedParameters.add(value);
}
});
} else if (this.isContextParameter(rvt)) {
requestedParameters.add(rvt);
} else {
// Not a context parameter, we can skip
}
}

Expand Down
103 changes: 91 additions & 12 deletions packages/taco/test/conditions/context.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ import {
USER_ADDRESS_PARAM_DEFAULT,
USER_ADDRESS_PARAM_EIP4361,
USER_ADDRESS_PARAM_EIP712,
USER_ADDRESS_PARAM_EXTERNAL_EIP4361,
} from '../../src/conditions/const';
import { CustomContextParam } from '../../src/conditions/context';
import {
paramOrContextParamSchema,
ReturnValueTestProps,
paramOrContextParamSchema,
} from '../../src/conditions/shared';
import {
testContractConditionObj,
Expand Down Expand Up @@ -279,10 +280,28 @@ describe('context', () => {
});
});

// TODO: Move to a separate file
describe('authentication provider', () => {
let provider: ethers.providers.Provider;
let signer: ethers.Signer;

async function testEIP4361AuthSignature(authSignature: AuthSignature) {
expect(authSignature).toBeDefined();
expect(authSignature.signature).toBeDefined();
expect(authSignature.scheme).toEqual('EIP4361');

const signerAddress = await signer.getAddress();
expect(authSignature.address).toEqual(signerAddress);

expect(authSignature.typedData).toContain(
`localhost wants you to sign in with your Ethereum account:\n${signerAddress}`,
);
expect(authSignature.typedData).toContain('URI: http://localhost:3000');

const chainId = (await provider.getNetwork()).chainId;
expect(authSignature.typedData).toContain(`Chain ID: ${chainId}`);
}

beforeAll(async () => {
await initialize();
provider = fakeProvider();
Expand Down Expand Up @@ -340,6 +359,25 @@ describe('authentication provider', () => {
).not.toThrow();
});

it('rejects when requesting EIP4361 signature and reusing existing signature at the same time', async () => {
const conditionObj = {
...testContractConditionObj,
returnValueTest: {
...testReturnValueTest,
value: [
USER_ADDRESS_PARAM_EIP4361,
USER_ADDRESS_PARAM_EXTERNAL_EIP4361,
],
},
};
const condition = new ContractCondition(conditionObj);
const conditionExpr = new ConditionExpression(condition);
expect(conditionExpr.contextRequiresSigner()).toBe(true);
await expect(
conditionExpr.buildContext(provider, {}, signer).toObj(),
).rejects.toThrow(`Cannot use ${USER_ADDRESS_PARAM_EIP4361}, ${USER_ADDRESS_PARAM_EXTERNAL_EIP4361} together`);
});

async function makeAuthSignature(authMethod: string) {
const conditionObj = {
...testContractConditionObj,
Expand Down Expand Up @@ -392,23 +430,64 @@ describe('authentication provider', () => {
EIP4361SignatureProvider.prototype,
'getOrCreateWalletSignature',
);

const authSignature = await makeAuthSignature(USER_ADDRESS_PARAM_EIP4361);
expect(authSignature).toBeDefined();
expect(authSignature.signature).toBeDefined();
expect(authSignature.scheme).toEqual('EIP4361');
await testEIP4361AuthSignature(authSignature);

const signerAddress = await signer.getAddress();
expect(authSignature.address).toEqual(signerAddress);
expect(eip4361Spy).toHaveBeenCalledOnce();
});

expect(authSignature.typedData).toContain(
`localhost wants you to sign in with your Ethereum account:\n${signerAddress}`,
it('supports reusing external eip4361', async () => {
// Because we are reusing an existing SIWE auth message, we have to pass it as a custom parameter
const authMessage = await makeAuthSignature(USER_ADDRESS_PARAM_EIP4361);
const customParams: Record<string, CustomContextParam> = {
[USER_ADDRESS_PARAM_EXTERNAL_EIP4361]: authMessage as CustomContextParam,
};

// Spying on the EIP4361 provider to make sure it's not called
const eip4361Spy = vi.spyOn(
EIP4361SignatureProvider.prototype,
'getOrCreateWalletSignature',
);
expect(authSignature.typedData).toContain('URI: http://localhost:3000');

const chainId = (await provider.getNetwork()).chainId;
expect(authSignature.typedData).toContain(`Chain ID: ${chainId}`);
// Now, creating the condition context to run the actual test
const conditionObj = {
...testContractConditionObj,
returnValueTest: {
...testReturnValueTest,
value: USER_ADDRESS_PARAM_EXTERNAL_EIP4361,
},
};
const condition = new ContractCondition(conditionObj);
const conditionExpr = new ConditionExpression(condition);
expect(conditionExpr.contextRequiresSigner()).toBe(true);

expect(eip4361Spy).toHaveBeenCalledOnce();
// Should throw an error if we don't pass the custom parameter
await expect(
conditionExpr.buildContext(provider, {}, signer).toObj(),
).rejects.toThrow(
`Missing custom context parameter(s): ${USER_ADDRESS_PARAM_EXTERNAL_EIP4361}`,
);

// Remembering to pass in customParams here:
const builtContext = conditionExpr.buildContext(
provider,
customParams,
signer,
);
const contextVars = await builtContext.toObj();
expect(eip4361Spy).not.toHaveBeenCalledOnce();

// Now, we expect that the auth signature will be available in the context variables
// BUT we want it to be available under the USER_ADDRESS_PARAM_EIP4361 key
const authSignature = contextVars[
USER_ADDRESS_PARAM_EIP4361
] as AuthSignature;
expect(authSignature).toBeDefined();
await testEIP4361AuthSignature(authSignature);

// TODO: Should we enforce that? It does not affect the server-side verification of the signature
// expect(contextVars[USER_ADDRESS_PARAM_EXTERNAL_EIP4361]).not.toBeDefined();
});
});

Expand Down
4 changes: 2 additions & 2 deletions packages/taco/test/test-utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ import {
zip,
} from '@nucypher/shared';
import {
TEST_CHAIN_ID,
TEST_CONTRACT_ADDR,
fakeDkgFlow,
fakeSigner,
fakeTDecFlow,
TEST_CHAIN_ID,
TEST_CONTRACT_ADDR,
} from '@nucypher/test-utils';
import { SpyInstance, vi } from 'vitest';

Expand Down

0 comments on commit a472dec

Please sign in to comment.