Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Disable Hardware Authenticator identities for unsupported Neighborhoods #75

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 65 additions & 26 deletions src/api/neighborhoods/provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,39 +12,78 @@
Tokens,
} from "@liftedinit/many-js";
import { useAccountsStore } from "features/accounts";
import { createContext, ReactNode, useMemo } from "react";
import {
createContext,
ReactNode,
useContext,
useEffect,
useMemo,
useState,
} from "react";
import { useNeighborhoodStore } from "./store";

export const NeighborhoodContext = createContext<Network | undefined>(
undefined
);
interface INeighborhoodContext {
query?: Network;
command?: Network;
services: Set<string>;
}
Comment on lines +25 to +29
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of returning just a Network (using the active Identity), the context is now a little more complex with a Network for queries (anonymous) and one for commands (using the active Identity). Also, the services are computed once and stored in the context, instead of queried at the component level.


export const NeighborhoodContext = createContext<INeighborhoodContext>({
services: new Set(),
});

export function NeighborhoodProvider({ children }: { children: ReactNode }) {
const activeNeighborhood = useNeighborhoodStore(
(s) => s.neighborhoods[s.activeId]
);
const activeAccount = useAccountsStore((s) => s.byId.get(s.activeId))!;

const neighborhood = useMemo(() => {
const identity = activeAccount?.identity ?? new AnonymousIdentity();
const network = new Network(activeNeighborhood.url, identity);
network.apply([
Account,
Base,
Blockchain,
Compute,
Events,
IdStore,
KvStore,
Ledger,
Tokens,
]);
return network;
}, [activeAccount, activeNeighborhood]);
const { url } = useNeighborhoodStore((s) => s.neighborhoods[s.activeId]);
const account = useAccountsStore((s) => s.byId.get(s.activeId))!;
const [services, setServices] = useState<Set<string>>(new Set());

const context = useMemo(() => {
const anonymous = new AnonymousIdentity();
const identity = account?.identity ?? anonymous;

const query = new Network(url, anonymous);
const command = new Network(url, identity);
Comment on lines +44 to +45
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Creating the two Networks here...

[query, command].forEach((network) =>
network.apply([
Account,
Base,
Blockchain,
Compute,
Events,
IdStore,
KvStore,
Ledger,
Tokens,
])
);
return { query, command, services };
}, [account, url, services]);

useEffect(() => {
async function updateServices() {
if (!context.query || !context.query.base) {
return;
}
const updated = new Set<string>();
try {
const { endpoints } = await context.query.base.endpoints();
endpoints
.map((endpoint: string) => endpoint.split(".")[0])
.forEach((service: string) => updated.add(service));

Check warning on line 72 in src/api/neighborhoods/provider.tsx

View check run for this annotation

Codecov / codecov/patch

src/api/neighborhoods/provider.tsx#L67-L72

Added lines #L67 - L72 were not covered by tests
} catch (error) {
console.error(`Couldn't update services: ${(error as Error).message}`);

Check warning on line 74 in src/api/neighborhoods/provider.tsx

View check run for this annotation

Codecov / codecov/patch

src/api/neighborhoods/provider.tsx#L74

Added line #L74 was not covered by tests
}
setServices(updated);

Check warning on line 76 in src/api/neighborhoods/provider.tsx

View check run for this annotation

Codecov / codecov/patch

src/api/neighborhoods/provider.tsx#L76

Added line #L76 was not covered by tests
}
updateServices();
// eslint-disable-next-line
}, [url]);

return (
<NeighborhoodContext.Provider value={neighborhood}>
<NeighborhoodContext.Provider value={context}>
{children}
</NeighborhoodContext.Provider>
);
}

export const useNeighborhoodContext = () => useContext(NeighborhoodContext);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finally, return a useNeighborhoodContext custom hook to reduce imports throughout the codebase.

5 changes: 2 additions & 3 deletions src/features/accounts/api/create-account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
AccountMultisigArgument,
CreateAccountResponse,
} from "@liftedinit/many-js";
import { NeighborhoodContext } from "api/neighborhoods";
import { useContext } from "react";
import { useNeighborhoodContext } from "api/neighborhoods";
import { useMutation } from "react-query";
import { accountLedgerFeature, accountMultisigFeature } from "../types";

Expand All @@ -20,7 +19,7 @@
};

export function useCreateAccount() {
const n = useContext(NeighborhoodContext);
const { command: n } = useNeighborhoodContext();

Check warning on line 22 in src/features/accounts/api/create-account.ts

View check run for this annotation

Codecov / codecov/patch

src/features/accounts/api/create-account.ts#L22

Added line #L22 was not covered by tests
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The change to context required a lot of changes. I approached them by changing as little code as possible, mostly destructuring using the existing variable name so I didn't have to touch the rest of the code. I had to decide whether the function needed the query or command network — mutations got command, queries got query.

return useMutation<CreateAccountResponse, Error, CreateAccountFormData>(
async (vars: CreateAccountFormData) => {
const name = vars.accountSettings.name;
Expand Down
5 changes: 2 additions & 3 deletions src/features/accounts/api/get-account-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,12 @@
AccountRole,
GetAccountInfoResponse,
} from "@liftedinit/many-js";
import { NeighborhoodContext } from "api/neighborhoods";
import { useContext } from "react";
import { useNeighborhoodContext } from "api/neighborhoods";
import { useQuery } from "react-query";
import { useAccountsStore } from "../stores";

export function useGetAccountInfo(accountAddress?: string) {
const n = useContext(NeighborhoodContext);
const { query: n } = useNeighborhoodContext();

Check warning on line 11 in src/features/accounts/api/get-account-info.ts

View check run for this annotation

Codecov / codecov/patch

src/features/accounts/api/get-account-info.ts#L11

Added line #L11 was not covered by tests
const activeIdentity = useAccountsStore((s) => s.byId.get(s.activeId));
const address = activeIdentity?.address;

Expand Down
9 changes: 4 additions & 5 deletions src/features/accounts/api/get-webauthn-credential.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
import { NeighborhoodContext } from "api/neighborhoods";
import { useContext } from "react";
import { useNeighborhoodContext } from "api/neighborhoods";
import { useMutation } from "react-query";
import { RecoverOptions } from "../types";

/**
* get webauthn account data from k-v store during import/recovery
*/
export function useGetWebauthnCredential() {
const queryNetwork = useContext(NeighborhoodContext);
const { query } = useNeighborhoodContext();

Check warning on line 9 in src/features/accounts/api/get-webauthn-credential.ts

View check run for this annotation

Codecov / codecov/patch

src/features/accounts/api/get-webauthn-credential.ts#L9

Added line #L9 was not covered by tests
return useMutation(
async ({ getFrom, value }: { getFrom: RecoverOptions; value: string }) => {
const getFn =
getFrom === RecoverOptions.phrase
? queryNetwork?.idStore.getFromRecallPhrase
: queryNetwork?.idStore.getFromAddress;
? query?.idStore.getFromRecallPhrase
: query?.idStore.getFromAddress;

Check warning on line 15 in src/features/accounts/api/get-webauthn-credential.ts

View check run for this annotation

Codecov / codecov/patch

src/features/accounts/api/get-webauthn-credential.ts#L14-L15

Added lines #L14 - L15 were not covered by tests
const res = await getFn(value);
return res;
}
Expand Down
7 changes: 3 additions & 4 deletions src/features/accounts/api/save-webauthn-credential.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { IdStore, Network, WebAuthnIdentity } from "@liftedinit/many-js";
import { NeighborhoodContext } from "api/neighborhoods";
import { useContext } from "react";
import { useNeighborhoodContext } from "api/neighborhoods";
import { useMutation } from "react-query";

export function useSaveWebauthnCredential() {
const network = useContext(NeighborhoodContext);
const { command } = useNeighborhoodContext();

Check warning on line 6 in src/features/accounts/api/save-webauthn-credential.ts

View check run for this annotation

Codecov / codecov/patch

src/features/accounts/api/save-webauthn-credential.ts#L6

Added line #L6 was not covered by tests
return useMutation<
{ phrase: string },
Error,
Expand All @@ -15,7 +14,7 @@
identity: WebAuthnIdentity;
}
>(async ({ address, credentialId, cosePublicKey, identity }) => {
const n = new Network(network?.url ?? "", identity);
const n = new Network(command?.url ?? "", identity);
n.apply([IdStore]);
const res = await n?.idStore.store(address, credentialId, cosePublicKey);
return res;
Expand Down
42 changes: 31 additions & 11 deletions src/features/accounts/components/accounts-menu/accounts-menu.tsx
Original file line number Diff line number Diff line change
@@ -1,37 +1,39 @@
import React from "react";
fmorency marked this conversation as resolved.
Show resolved Hide resolved
import {
AnonymousIdentity,
ANON_IDENTITY,
WebAuthnIdentity,
} from "@liftedinit/many-js";
import { useAccountsStore } from "features/accounts";
import {
AddressText,
Box,
Button,
ChevronDownIcon,
Circle,
EditIcon,
Flex,
HStack,
Icon,
IconButton,
Menu,
MenuButton,
MenuList,
MenuDivider,
MenuItem,
MenuList,
MenuOptionGroup,
MenuDivider,
SimpleGrid,
Text,
UsbIcon,
useDisclosure,
VStack,
AddressText,
ChevronDownIcon,
EditIcon,
UserIcon,
UsbIcon,
useToast,
VStack,
} from "@liftedinit/ui";
import { useNeighborhoodContext } from "api/neighborhoods";
import { useAccountsStore } from "features/accounts";
import { useEffect, useState } from "react";
import { Account, AccountId } from "../../types";
import { AddAccountModal } from "./add-account-modal";
import { EditAccountModal } from "./edit-account-modal";
import { Account, AccountId } from "../../types";

export type AccountItemWithIdDisplayStrings = [
AccountId,
Expand Down Expand Up @@ -66,7 +68,25 @@
})
);

const [editAccount, setEditAccount] = React.useState<
const toast = useToast();
const { services } = useNeighborhoodContext();
useEffect(() => {
(async () => {
const isWebAuthnIdentity =
activeAccount?.identity instanceof WebAuthnIdentity;
if (isWebAuthnIdentity && !services.has("idstore")) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Getting the list of services is no longer asynchronous, which I suspect was causing a problem here.

setActiveId(0); // reset to Anonymous
toast({

Check warning on line 79 in src/features/accounts/components/accounts-menu/accounts-menu.tsx

View check run for this annotation

Codecov / codecov/patch

src/features/accounts/components/accounts-menu/accounts-menu.tsx#L78-L79

Added lines #L78 - L79 were not covered by tests
status: "warning",
title: "Unsupported Identity",
description:
"Selected Neighborhood does not support Hardware Authenticators",
});
}
})();
}, [activeAccount, services, setActiveId, toast]);

const [editAccount, setEditAccount] = useState<
[number, Account] | undefined
>();

Expand Down
Loading