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

feat(ConnectWalletForm): ask consent for automatic key addition #637

Merged
merged 9 commits into from
Oct 4, 2024
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
29 changes: 22 additions & 7 deletions src/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,6 @@
"connectWallet_action_connect": {
"message": "Connect"
},
"connectWallet_text_footerNotice": {
"message": "We'll automatically connect with your wallet provider."
},
"connectWallet_text_footerNoticeLearnMore": {
"message": "Learn more",
"description": "Learn more about how this works"
},
"connectWallet_error_urlRequired": {
"message": "Wallet address is required."
},
Expand Down Expand Up @@ -172,9 +165,31 @@
"connectWallet_error_grantRejected": {
"message": "Connect wallet cancelled. You rejected the request."
},
"connectWalletKeyService_text_consentP1": {
"message": "We'll automatically connect with your wallet provider."
},
"connectWalletKeyService_text_consentLearnMore": {
"message": "Learn more",
"description": "Learn more about how this works"
},
"connectWalletKeyService_text_consentP2": {
"message": "By agreeing, you provide us consent to automatically access your wallet to securely add a key."
},
"connectWalletKeyService_text_consentP3": {
"message": "Please note, this process does not involve accessing or handling your funds."
},
"connectWalletKeyService_label_consentAccept": {
"message": "Agree"
},
"connectWalletKeyService_label_consentDecline": {
"message": "Decline"
},
"connectWalletKeyService_error_notImplemented": {
"message": "Automatic key addition is not implemented for given wallet provider yet."
},
"connectWalletKeyService_error_noConsent": {
"message": "You declined consent for automatic key addition."
},
"connectWalletKeyService_error_failed": {
"message": "Automatic key addition failed at step “$STEP_ID$” with message “$MESSAGE$”.",
"placeholders": {
Expand Down
9 changes: 9 additions & 0 deletions src/background/services/keyAutoAdd.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,15 @@ export class KeyAutoAddService {
}));
}
}

static supports(walletAddress: WalletAddress): boolean {
try {
walletAddressToProvider(walletAddress);
return true;
} catch {
return false;
}
}
}

export function walletAddressToProvider(walletAddress: WalletAddress): {
Expand Down
21 changes: 19 additions & 2 deletions src/background/services/openPayments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,13 @@ export class OpenPaymentsService {
this.setConnectState(null);
return;
}
const { walletAddressUrl, amount, recurring, skipAutoKeyShare } = params;
const {
walletAddressUrl,
amount,
recurring,
autoKeyAdd,
autoKeyAddConsent,
sidvishnoi marked this conversation as resolved.
Show resolved Hide resolved
} = params;

const walletAddress = await getWalletInformation(walletAddressUrl);
const exchangeRates = await getExchangeRates();
Expand Down Expand Up @@ -385,8 +391,19 @@ export class OpenPaymentsService {
if (
isErrorWithKey(error) &&
error.key === 'connectWallet_error_invalidClient' &&
!skipAutoKeyShare
autoKeyAdd
) {
if (!KeyAutoAddService.supports(walletAddress)) {
this.updateConnectStateError(error);
throw new ErrorWithKey(
'connectWalletKeyService_error_notImplemented',
);
}
if (!autoKeyAddConsent) {
this.updateConnectStateError(error);
throw new ErrorWithKey('connectWalletKeyService_error_noConsent');
}

// add key to wallet and try again
try {
const tabId = await this.addPublicKeyToWallet(walletAddress);
Expand Down
99 changes: 71 additions & 28 deletions src/popup/components/ConnectWalletForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ interface Inputs {
walletAddressUrl: string;
amount: string;
recurring: boolean;
autoKeyAddConsent: boolean;
}

type ErrorInfo = { message: string; info?: ErrorWithKeyLike };
Expand Down Expand Up @@ -69,6 +70,10 @@ export const ConnectWalletForm = ({
const [autoKeyShareFailed, setAutoKeyShareFailed] = React.useState(
isAutoKeyAddFailed(state),
);
const [showConsent, setShowConsent] = React.useState(false);
const autoKeyAddConsent = React.useRef<boolean>(
defaultValues.autoKeyAddConsent || false,
);

const resetState = React.useCallback(async () => {
await clearConnectState();
Expand Down Expand Up @@ -162,8 +167,8 @@ export const ConnectWalletForm = ({
[saveValue, currencySymbol, toErrorInfo],
);

const handleSubmit = async (ev: React.FormEvent<HTMLFormElement>) => {
ev.preventDefault();
const handleSubmit = async (ev?: React.FormEvent<HTMLFormElement>) => {
ev?.preventDefault();

const errWalletAddressUrl = validateWalletAddressUrl(walletAddressUrl);
const errAmount = validateAmount(amount, currencySymbol.symbol);
Expand All @@ -188,14 +193,19 @@ export const ConnectWalletForm = ({
walletAddressUrl: toWalletAddressUrl(walletAddressUrl),
amount,
recurring,
skipAutoKeyShare,
autoKeyAdd: !skipAutoKeyShare,
autoKeyAddConsent: autoKeyAddConsent.current,
});
if (res.success) {
onConnect();
} else {
if (isErrorWithKey(res.error)) {
const error = res.error;
if (error.key.startsWith('connectWalletKeyService_error_')) {
if (error.key === 'connectWalletKeyService_error_noConsent') {
setShowConsent(true);
return;
}
setErrors((prev) => ({ ...prev, keyPair: toErrorInfo(error) }));
} else {
setErrors((prev) => ({ ...prev, connect: toErrorInfo(error) }));
Expand Down Expand Up @@ -225,6 +235,24 @@ export const ConnectWalletForm = ({
}
}, [defaultValues.walletAddressUrl, handleWalletAddressUrlChange]);

if (showConsent) {
return (
<AutoKeyAddConsent
onAccept={() => {
autoKeyAddConsent.current = true;
// saveValue('autoKeyAddConsent', true);
Copy link
Member

Choose a reason for hiding this comment

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

Probably related to my comment above - saving this preference.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I'm not sure if we want to save, but I kept the code here in case we decide to.
Or, maybe we can ask for consent once per wallet address? Like we'll store the wallet address instead of true there.
If it's false, we most likely want to ask again, as that's a happier path for user (in case they find adding key manually is too much after seeing that input).

Copy link
Member

Choose a reason for hiding this comment

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

setShowConsent(false);
handleSubmit();
}}
onDecline={() => {
const error = errorWithKey('connectWalletKeyService_error_noConsent');
setErrors((prev) => ({ ...prev, keyPair: toErrorInfo(error) }));
setShowConsent(false);
}}
/>
);
}

return (
<form
data-testid="connect-wallet-form"
Expand Down Expand Up @@ -380,13 +408,46 @@ export const ConnectWalletForm = ({
>
{t('connectWallet_action_connect')}
</Button>
</div>
</form>
);
};

{!errors.keyPair && !autoKeyShareFailed && (
<Footer
text={t('connectWallet_text_footerNotice')}
learnMoreText={t('connectWallet_text_footerNoticeLearnMore')}
/>
)}
const AutoKeyAddConsent: React.FC<{
onAccept: () => void;
onDecline: () => void;
}> = ({ onAccept, onDecline }) => {
const t = useTranslation();
return (
<form
className="space-y-4 text-center"
data-testid="connect-wallet-auto-key-consent"
>
<p className="text-lg leading-snug text-weak">
{t('connectWalletKeyService_text_consentP1')}{' '}
<a
hidden
href="https://webmonetization.org"
className="text-primary hover:underline"
target="_blank"
rel="noreferrer"
>
{t('connectWalletKeyService_text_consentLearnMore')}
</a>
</p>

<div className="space-y-2 pt-12 text-medium">
<p>{t('connectWalletKeyService_text_consentP2')}</p>
<p>{t('connectWalletKeyService_text_consentP3')}</p>
</div>

<div className="mx-auto flex w-3/4 justify-around gap-4">
<Button onClick={onAccept}>
{t('connectWalletKeyService_label_consentAccept')}
</Button>
<Button onClick={onDecline} variant="destructive">
{t('connectWalletKeyService_label_consentDecline')}
</Button>
</div>
</form>
);
Expand Down Expand Up @@ -462,30 +523,12 @@ function isAutoKeyAddFailed(state: PopupTransientState['connect']) {
function canRetryAutoKeyAdd(err?: ErrorInfo['info']) {
if (!err) return false;
return (
err.key === 'connectWalletKeyService_error_noConsent' ||
err.cause?.key === 'connectWalletKeyService_error_timeoutLogin' ||
err.cause?.key === 'connectWalletKeyService_error_accountNotFound'
);
}

const Footer: React.FC<{
text: string;
learnMoreText: string;
}> = ({ text, learnMoreText }) => {
return (
<p className="text-center text-xs text-weak">
{text}{' '}
<a
href="https://webmonetization.org"
className="text-primary hover:underline"
target="_blank"
rel="noreferrer"
>
{learnMoreText}
</a>
</p>
);
};

function validateWalletAddressUrl(value: string): null | ErrorWithKeyLike {
if (!value) {
return errorWithKey('connectWallet_error_urlRequired');
Expand Down
2 changes: 2 additions & 0 deletions src/popup/pages/Settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ export const Component = () => {
amount: localStorage?.getItem('connect.amount') || undefined,
walletAddressUrl:
localStorage?.getItem('connect.walletAddressUrl') || undefined,
autoKeyAddConsent:
localStorage?.getItem('connect.autoKeyAddConsent') === 'true',
}}
saveValue={(key, val) => {
localStorage?.setItem(`connect.${key}`, val.toString());
Expand Down
3 changes: 2 additions & 1 deletion src/shared/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ export interface ConnectWalletPayload {
walletAddressUrl: string;
amount: string;
recurring: boolean;
skipAutoKeyShare: boolean;
autoKeyAdd: boolean;
autoKeyAddConsent: boolean | null;
}

export interface AddFundsPayload {
Expand Down
10 changes: 9 additions & 1 deletion tests/e2e/connectAutoKeyTestWallet.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,17 @@ test('Connect to test wallet with automatic key addition when not logged-in to w
await page.close();
});

page = await test.step('shows login page', async () => {
await test.step('asks for key-add consent', async () => {
await connectButton.click();
await popup.waitForSelector(
`[data-testid="connect-wallet-auto-key-consent"]`,
);

expect(popup.getByTestId('connect-wallet-auto-key-consent')).toBeVisible();
await popup.getByRole('button', { name: 'Accept' }).click();
});

page = await test.step('shows login page', async () => {
const openedPage = await context.waitForEvent('page', {
predicate: (page) => page.url().startsWith(loginPageUrl),
timeout: 3 * 1000,
Expand Down