Skip to content

Commit

Permalink
feat/1349 - Require unlocked keychain for all access (#1352)
Browse files Browse the repository at this point in the history
* feat: extension must be unlocked to connect

* feat: wrap approve connection with auth component

* feat: wrap all top-level approvals with auth

* fix: clean up naming conventions

* feat: validate origin & chain Id to see if already approved

* fix: fix failing tests

* feat: subscribe to unlock event
  • Loading branch information
jurevans authored Dec 2, 2024
1 parent ec92436 commit 231d0a0
Show file tree
Hide file tree
Showing 27 changed files with 356 additions and 134 deletions.
2 changes: 2 additions & 0 deletions apps/extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"start:firefox": "yarn clean:firefox && NODE_ENV=development TARGET=firefox REVISION=$(git rev-parse HEAD) yarn watch",
"start:firefox:proxy": "NAMADA_INTERFACE_PROXY=true yarn start:firefox",
"test": "yarn wasm:build:test && yarn jest --coverage",
"test:only": "yarn jest --coverage",
"test:watch": "yarn wasm:build:test && yarn jest --watchAll=true",
"test:ci": "jest",
"wasm:build": "node ./scripts/build.js --release",
Expand All @@ -47,6 +48,7 @@
"bignumber.js": "^9.1.1",
"buffer": "^6.0.3",
"dompurify": "^3.0.2",
"fp-ts": "^2.16.1",
"framer-motion": "^11.3.28",
"io-ts": "^2.2.21",
"js-sha256": "^0.10.1",
Expand Down
133 changes: 90 additions & 43 deletions apps/extension/src/Approvals/Approvals.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import React, { useState } from "react";
import React, { Dispatch, SetStateAction, useEffect, useState } from "react";
import { Route, Routes } from "react-router-dom";

import { Container } from "@namada/components";
import { AccountType, TxDetails } from "@namada/types";

import { AppHeader } from "App/Common/AppHeader";
import { TopLevelRoute } from "Approvals/types";
import { CheckIsLockedMsg } from "background/vault";
import { useRequester } from "hooks/useRequester";
import { Ports } from "router";
import { ApproveConnection } from "./ApproveConnection";
import { ApproveDisconnection } from "./ApproveDisconnection";
import { ApproveSignArbitrary } from "./ApproveSignArbitrary";
Expand All @@ -14,13 +17,21 @@ import { ApproveUpdateDefaultAccount } from "./ApproveUpdateDefaultAccount";
import { ConfirmSignature } from "./ConfirmSignArbitrary";
import { ConfirmSignLedgerTx } from "./ConfirmSignLedgerTx";
import { ConfirmSignTx } from "./ConfirmSignTx";
import { WithAuth } from "./WithAuth";

export enum Status {
Completed,
Pending,
Failed,
}

export type ExtensionLockContextType = {
isUnlocked?: boolean;
setIsUnlocked?: Dispatch<SetStateAction<boolean>>;
};
export const ExtensionLockContext =
React.createContext<ExtensionLockContextType>({});

export type ApprovalDetails = {
signer: string;
accountType: AccountType;
Expand All @@ -38,6 +49,17 @@ export const Approvals: React.FC = () => {
const [details, setDetails] = useState<ApprovalDetails>();
const [signArbitraryDetails, setSignArbitraryDetails] =
useState<SignArbitraryDetails>();
const [isUnlocked, setIsUnlocked] = useState(false);
const requester = useRequester();

useEffect(() => {
requester
.sendMessage(Ports.Background, new CheckIsLockedMsg())
.then((isLocked) => {
setIsUnlocked!(!isLocked);
})
.catch(() => setIsUnlocked!(false));
}, []);

return (
<Container
Expand All @@ -50,48 +72,73 @@ export const Approvals: React.FC = () => {
/>
}
>
<Routes>
<Route
path={`${TopLevelRoute.ApproveSignTx}/:msgId/:accountType/:signer`}
element={<ApproveSignTx details={details} setDetails={setDetails} />}
/>
<Route
path={TopLevelRoute.ConfirmSignTx}
element={details && <ConfirmSignTx details={details} />}
/>
<Route
path={TopLevelRoute.ConfirmLedgerTx}
element={details && <ConfirmSignLedgerTx details={details} />}
/>
<Route
path={TopLevelRoute.ApproveConnection}
element={<ApproveConnection />}
/>
<Route
path={TopLevelRoute.ApproveDisconnection}
element={<ApproveDisconnection />}
/>
<Route
path={TopLevelRoute.ApproveUpdateDefaultAccount}
element={<ApproveUpdateDefaultAccount />}
/>
<Route
path={`${TopLevelRoute.ApproveSignArbitrary}/:signer`}
element={
<ApproveSignArbitrary
setSignArbitraryDetails={setSignArbitraryDetails}
/>
}
/>
<Route
path={TopLevelRoute.ConfirmSignArbitrary}
element={
signArbitraryDetails && (
<ConfirmSignature details={signArbitraryDetails} />
)
}
/>
</Routes>
<ExtensionLockContext.Provider
value={{
isUnlocked,
setIsUnlocked,
}}
>
<Routes>
<Route
path={`${TopLevelRoute.ApproveSignTx}/:msgId/:accountType/:signer`}
element={
<WithAuth>
<ApproveSignTx details={details} setDetails={setDetails} />
</WithAuth>
}
/>
<Route
path={TopLevelRoute.ConfirmSignTx}
element={details && <ConfirmSignTx details={details} />}
/>
<Route
path={TopLevelRoute.ConfirmLedgerTx}
element={details && <ConfirmSignLedgerTx details={details} />}
/>
<Route
path={TopLevelRoute.ApproveConnection}
element={
<WithAuth>
<ApproveConnection />
</WithAuth>
}
/>
<Route
path={TopLevelRoute.ApproveDisconnection}
element={
<WithAuth>
<ApproveDisconnection />
</WithAuth>
}
/>
<Route
path={TopLevelRoute.ApproveUpdateDefaultAccount}
element={
<WithAuth>
<ApproveUpdateDefaultAccount />
</WithAuth>
}
/>
<Route
path={`${TopLevelRoute.ApproveSignArbitrary}/:signer`}
element={
<WithAuth>
<ApproveSignArbitrary
setSignArbitraryDetails={setSignArbitraryDetails}
/>
</WithAuth>
}
/>
<Route
path={TopLevelRoute.ConfirmSignArbitrary}
element={
signArbitraryDetails && (
<ConfirmSignature details={signArbitraryDetails} />
)
}
/>
</Routes>
</ExtensionLockContext.Provider>
</Container>
);
};
36 changes: 33 additions & 3 deletions apps/extension/src/Approvals/ApproveConnection.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,46 @@
import { ActionButton, Alert, GapPatterns, Stack } from "@namada/components";
import { PageHeader } from "App/Common";
import { ConnectInterfaceResponseMsg } from "background/approvals";
import {
CheckIsApprovedSiteMsg,
ConnectInterfaceResponseMsg,
} from "background/approvals";
import { useQuery } from "hooks";
import { useRequester } from "hooks/useRequester";
import { useContext, useEffect } from "react";
import { Ports } from "router";
import { closeCurrentTab } from "utils";
import { ExtensionLockContext } from "./Approvals";

export const ApproveConnection: React.FC = () => {
const requester = useRequester();
const params = useQuery();
const interfaceOrigin = params.get("interfaceOrigin");
const chainId = params.get("chainId");
const interfaceOrigin = params.get("interfaceOrigin")!;
const chainId = params.get("chainId")!;
const { isUnlocked } = useContext(ExtensionLockContext);

const checkIsApproved = async (): Promise<void> => {
requester
.sendMessage(
Ports.Background,
new CheckIsApprovedSiteMsg(interfaceOrigin, chainId)
)
.then((isApproved) => {
if (isApproved) {
// Go ahead and connect as this domain is already approved
void handleResponse(true);
}
})
.catch((e) => {
console.error(e);
});
};

useEffect(() => {
if (isUnlocked) {
// Test to see if domain is already approved
void checkIsApproved();
}
}, [isUnlocked]);

const handleResponse = async (allowConnection: boolean): Promise<void> => {
if (interfaceOrigin) {
Expand Down
37 changes: 37 additions & 0 deletions apps/extension/src/Approvals/WithAuth.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { Login } from "App/Login";
import { UnlockVaultMsg } from "background/vault";
import { useRequester } from "hooks/useRequester";
import { useContext } from "react";
import { Ports } from "router";
import { ExtensionLockContext } from "./Approvals";

type Props = {
children: React.ReactNode;
};

export const WithAuth: React.FC<Props> = ({ children }) => {
const requester = useRequester();
const { isUnlocked, setIsUnlocked } = useContext(ExtensionLockContext);

const handleOnLogin = async (password: string): Promise<boolean> => {
try {
const isUnlocked = await requester.sendMessage(
Ports.Background,
new UnlockVaultMsg(password)
);
setIsUnlocked!(isUnlocked);
return isUnlocked;
} catch (_) {
setIsUnlocked!(false);
}
return false;
};

return (
<>
{isUnlocked ?
<>{children}</>
: <Login onLogin={handleOnLogin} />}
</>
);
};
14 changes: 14 additions & 0 deletions apps/extension/src/background/approvals/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from "provider";
import { Env, Handler, InternalHandler, Message } from "router";
import {
CheckIsApprovedSiteMsg,
ConnectInterfaceResponseMsg,
DisconnectInterfaceResponseMsg,
QueryPendingTxBytesMsg,
Expand Down Expand Up @@ -41,6 +42,11 @@ export const getHandler: (service: ApprovalsService) => Handler = (service) => {
env,
msg as ConnectInterfaceResponseMsg
);
case CheckIsApprovedSiteMsg:
return handleCheckIsApprovedSite(service)(
env,
msg as CheckIsApprovedSiteMsg
);
case ApproveDisconnectInterfaceMsg:
return handleApproveDisconnectInterfaceMsg(service)(
env,
Expand Down Expand Up @@ -272,3 +278,11 @@ const handleSubmitApprovedSignLedgerTxMsg: (
return await service.submitSignLedgerTx(popupTabId, msgId, responseSign);
};
};

const handleCheckIsApprovedSite: (
service: ApprovalsService
) => InternalHandler<CheckIsApprovedSiteMsg> = (service) => {
return async (_, { interfaceOrigin, chainId }) => {
return await service.isConnectionApproved(interfaceOrigin, chainId);
};
};
2 changes: 2 additions & 0 deletions apps/extension/src/background/approvals/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from "provider";
import { Router } from "router";
import {
CheckIsApprovedSiteMsg,
ConnectInterfaceResponseMsg,
DisconnectInterfaceResponseMsg,
QueryPendingTxBytesMsg,
Expand All @@ -29,6 +30,7 @@ import { ApprovalsService } from "./service";
export function init(router: Router, service: ApprovalsService): void {
router.registerMessage(ApproveSignTxMsg);
router.registerMessage(ApproveSignArbitraryMsg);
router.registerMessage(CheckIsApprovedSiteMsg);
router.registerMessage(RejectSignTxMsg);
router.registerMessage(RejectSignArbitraryMsg);
router.registerMessage(SubmitApprovedSignTxMsg);
Expand Down
26 changes: 26 additions & 0 deletions apps/extension/src/background/approvals/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export enum MessageType {
QueryTxDetails = "query-tx-details",
QuerySignArbitraryData = "query-sign-arbitrary-data",
QueryPendingTxBytes = "query-pending-tx-bytes",
CheckIsApprovedSite = "check-is-approved-site",
}

export class SubmitApprovedSignTxMsg extends Message<void> {
Expand Down Expand Up @@ -300,3 +301,28 @@ export class QuerySignArbitraryDataMsg extends Message<string> {
return QuerySignArbitraryDataMsg.type();
}
}

export class CheckIsApprovedSiteMsg extends Message<boolean> {
public static type(): MessageType {
return MessageType.CheckIsApprovedSite;
}

constructor(
public readonly interfaceOrigin: string,
public readonly chainId: string
) {
super();
}

validate(): void {
validateProps(this, ["interfaceOrigin", "chainId"]);
}

route(): string {
return ROUTE;
}

type(): string {
return CheckIsApprovedSiteMsg.type();
}
}
4 changes: 2 additions & 2 deletions apps/extension/src/background/approvals/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ describe("approvals service", () => {
},
]);

jest.spyOn(service as any, "_clearPendingTx");
jest.spyOn(service as any, "clearPendingTx");

(webextensionPolyfill.windows.create as any).mockResolvedValue({
tabs: [{ id: tabId }],
Expand All @@ -248,7 +248,7 @@ describe("approvals service", () => {
await service.rejectSignTx(tabId, "msgId");

// rejectSignTx should clear promise resolver for that msgId
expect((service as any)._clearPendingTx).toHaveBeenCalledWith("msgId");
expect((service as any).clearPendingTx).toHaveBeenCalledWith("msgId");

await expect(signaturePromise).rejects.toBeDefined();
});
Expand Down
Loading

1 comment on commit 231d0a0

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.