Skip to content

Commit

Permalink
refactor: profile-sync-sdk move snap methods to snap auth class (#4708)
Browse files Browse the repository at this point in the history
## Explanation

This is so we can keep some separation of concerns and prevent SIWE auth
SDK from using these public snap methods.

## References

## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/profile-sync-controller`

- **ADDED**: `assertSRP` private method to ensure that only SRP auth can
call the snap methods.
- **CHANGED**: Moved the snap methods: `isSnapConnected` and
`connectSnap` to the Snap Auth SDK.
- **REMOVED**: `getSnap` exported method

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
  • Loading branch information
Prithpal-Sooriya authored Sep 16, 2024
1 parent 2fe4b0b commit 29e75c3
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 86 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { ValidationError } from '../errors';
import { getMetaMaskProviderEIP6963 } from '../utils/eip-6963-metamask-provider';
import { MESSAGE_SIGNING_SNAP } from '../utils/messaging-signing-snap-requests';
import {
MESSAGE_SIGNING_SNAP,
connectSnap,
isSnapConnected,
} from '../utils/messaging-signing-snap-requests';
import { validateLoginResponse } from '../utils/validate-login-response';
import { authenticate, authorizeOIDC, getNonce } from './services';
import type {
Expand All @@ -20,19 +24,21 @@ type JwtBearerAuth_SRP_Options = {
signing?: AuthSigningOptions;
};

const getDefaultEIP6963Provider = async () => {
const provider = await getMetaMaskProviderEIP6963();
if (!provider) {
throw new ValidationError('No MetaMask wallet connected');
}
return provider;
};

const defaultEIP6963SigningOptions: AuthSigningOptions = {
getIdentifier: async (): Promise<string> => {
const provider = await getMetaMaskProviderEIP6963();
if (!provider) {
throw new ValidationError('No MetaMask wallet connected');
}
const provider = await getDefaultEIP6963Provider();
return await MESSAGE_SIGNING_SNAP.getPublicKey(provider);
},
signMessage: async (message: string): Promise<string> => {
const provider = await getMetaMaskProviderEIP6963();
if (!provider) {
throw new ValidationError('No MetaMask wallet connected');
}
const provider = await getDefaultEIP6963Provider();
if (!message.startsWith('metamask:')) {
throw new ValidationError('message must start with "metamask:"');
}
Expand Down Expand Up @@ -85,6 +91,22 @@ export class SRPJwtBearerAuth implements IBaseAuth {
return await this.#options.signing.signMessage(message);
}

async isSnapConnected(): Promise<boolean> {
const provider = await getMetaMaskProviderEIP6963();
if (!provider) {
return false;
}

const isConnected = await isSnapConnected(provider);
return isConnected;
}

async connectSnap(): Promise<string> {
const provider = await getDefaultEIP6963Provider();
const res = await connectSnap(provider);
return res;
}

// convert expiresIn from seconds to milliseconds and use 90% of expiresIn
async #getAuthSession(): Promise<LoginResponse | null> {
const auth = await this.#options.storage.getLoginResponse();
Expand Down
39 changes: 19 additions & 20 deletions packages/profile-sync-controller/src/sdk/authentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@ import type { UserProfile, Pair } from './authentication-jwt-bearer/types';
import { AuthType } from './authentication-jwt-bearer/types';
import type { Env } from './env';
import { PairError, UnsupportedAuthTypeError } from './errors';
import { getMetaMaskProviderEIP6963 } from './utils/eip-6963-metamask-provider';
import {
connectSnap,
isSnapConnected,
} from './utils/messaging-signing-snap-requests';

// Computing the Classes, so we only get back the public methods for the interface.
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
Expand Down Expand Up @@ -54,22 +49,13 @@ export class JwtBearerAuth implements SIWEInterface, SRPInterface {
}

async connectSnap(): Promise<string> {
const provider = await getMetaMaskProviderEIP6963();
if (!provider) {
throw new Error('failed to get MetaMaskProviderEIP6963 provider');
}
const res = await connectSnap(provider);
return res;
this.#assertSRP(this.#type, this.#sdk);
return this.#sdk.connectSnap();
}

async isSnapConnected(): Promise<boolean> {
const provider = await getMetaMaskProviderEIP6963();
if (!provider) {
return false;
}

const isConnected = await isSnapConnected(provider);
return isConnected;
this.#assertSRP(this.#type, this.#sdk);
return this.#sdk.isSnapConnected();
}

async getUserProfile(): Promise<UserProfile> {
Expand Down Expand Up @@ -132,8 +118,8 @@ export class JwtBearerAuth implements SIWEInterface, SRPInterface {

#assertSIWE(
type: AuthType,
sdk: SIWEJwtBearerAuth | SRPJwtBearerAuth,
): asserts sdk is SIWEJwtBearerAuth {
_sdk: SIWEJwtBearerAuth | SRPJwtBearerAuth,
): asserts _sdk is SIWEJwtBearerAuth {
if (type === AuthType.SiWE) {
return;
}
Expand All @@ -142,6 +128,19 @@ export class JwtBearerAuth implements SIWEInterface, SRPInterface {
'This method is only available via SIWE auth type',
);
}

#assertSRP(
type: AuthType,
_sdk: SIWEJwtBearerAuth | SRPJwtBearerAuth,
): asserts _sdk is SRPJwtBearerAuth {
if (type === AuthType.SRP) {
return;
}

throw new UnsupportedAuthTypeError(
'This method is only available via SRP auth type',
);
}
}

export { SIWEJwtBearerAuth } from './authentication-jwt-bearer/flow-siwe';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
MESSAGE_SIGNING_SNAP,
SNAP_ORIGIN,
connectSnap,
getSnap,
getSnaps,
isSnapConnected,
} from './messaging-signing-snap-requests';
Expand Down Expand Up @@ -66,44 +65,6 @@ describe('isSnapConnected() tests', () => {
});
});

describe('getSnap() tests', () => {
it('tests invocation', async () => {
const { mockProvider, mockRequest } = arrangeMockProvider();

const mockSnap: Snap = { id: SNAP_ORIGIN } as MockVariable;
mockRequest.mockResolvedValue({ [SNAP_ORIGIN]: mockSnap });

const result = await getSnap(mockProvider);
expect(mockRequest).toHaveBeenCalled();
expect(result).toBeDefined();
});

it('returns undefined if unable to find snap', async () => {
const { mockProvider, mockRequest } = arrangeMockProvider();

const mockSnap: Snap = { id: 'A differentSnap' } as MockVariable;
mockRequest.mockResolvedValue({ diffSnap: mockSnap });

const result1 = await getSnap(mockProvider);
expect(mockRequest).toHaveBeenCalled();
expect(result1).toBeUndefined();

// Another test in case the wallet request returns null
mockRequest.mockResolvedValue(null);
const result2 = await getSnap(mockProvider);
expect(result2).toBeUndefined();
});

it('returns undefined if an error is thrown when making provider request', async () => {
const { mockProvider, mockRequest } = arrangeMockProvider();
mockRequest.mockRejectedValue(new Error('MOCK ERROR'));

const result = await getSnap(mockProvider);
expect(mockRequest).toHaveBeenCalled();
expect(result).toBeUndefined();
});
});

describe('MESSAGE_SIGNING_SNAP.getPublicKey() tests', () => {
it('tests invocation', async () => {
const { mockProvider, mockRequest } = arrangeMockProvider();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ export type GetSnapsResponse = Record<string, Snap>;

export const SNAP_ORIGIN = 'npm:@metamask/message-signing-snap';

const foundSnap = (snap: Snap) => snap.id === SNAP_ORIGIN;

/**
* Requests Connection to the Message Signing Snap
*
Expand Down Expand Up @@ -66,22 +64,6 @@ export async function isSnapConnected(
}
}

/**
* Will return the message signing snap if installed
* @param provider - MetaMask Wallet Provider
*/
export async function getSnap(
provider: Eip1193Provider,
): Promise<Snap | undefined> {
try {
const snaps = await getSnaps(provider);
return Object.values(snaps ?? {}).find((snap) => foundSnap(snap));
} catch (e) {
console.error('Failed to obtain installed snap', e);
return undefined;
}
}

export const MESSAGE_SIGNING_SNAP = {
async getPublicKey(provider: Eip1193Provider) {
const publicKey: string = await provider.request({
Expand Down

0 comments on commit 29e75c3

Please sign in to comment.