From c1df8d8a2cfcf3a8706028a367b1d6b75f5ebb4f Mon Sep 17 00:00:00 2001
From: Irakli Gozalishvili <contact@gozala.io>
Date: Thu, 16 Nov 2023 10:04:10 +0300
Subject: [PATCH] fix: make login idempotent (#1149)

Logging with the same account several times creates and stores redundant
proofs and then sends them over during invocations.

Changes here avoid this redundancy by returning an account using
existing proofs if you already happen to have them, essentially making
login idempotent.
---
 packages/w3up-client/src/account.js       | 26 ++++++++++++++++++++
 packages/w3up-client/test/account.test.js | 29 +++++++++++++++++++++--
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/packages/w3up-client/src/account.js b/packages/w3up-client/src/account.js
index 597e82aba..d86962a3b 100644
--- a/packages/w3up-client/src/account.js
+++ b/packages/w3up-client/src/account.js
@@ -80,6 +80,22 @@ export const list = ({ agent }, { account } = {}) => {
  */
 export const login = async ({ agent }, email) => {
   const account = fromEmail(email)
+
+  // If we already have a session for this account we
+  // skip the authentication process, otherwise we will
+  // end up adding more UCAN proofs and attestations to
+  // the store which we then will be sending when using
+  // this account.
+  // Note: This is not a robust solution as there may be
+  // reasons to re-authenticate e.g. previous session is
+  // no longer valid because it was revoked. But dropping
+  // revoked UCANs from store is something we should do
+  // anyway.
+  const session = list({ agent }, { account })[account]
+  if (session) {
+    return { ok: session }
+  }
+
   const result = await Access.request(
     { agent },
     {
@@ -140,6 +156,16 @@ export class Account {
     this.proofs.push(proof)
   }
 
+  toJSON() {
+    return {
+      id: this.did(),
+      proofs: this.proofs
+        // we sort proofs to get a deterministic JSON representation.
+        .sort((a, b) => a.cid.toString().localeCompare(b.cid.toString()))
+        .map((proof) => proof.toJSON()),
+    }
+  }
+
   /**
    * Provisions given `space` with this account.
    *
diff --git a/packages/w3up-client/test/account.test.js b/packages/w3up-client/test/account.test.js
index 9336b682c..f2358ec44 100644
--- a/packages/w3up-client/test/account.test.js
+++ b/packages/w3up-client/test/account.test.js
@@ -67,6 +67,33 @@ export const testAccount = {
     assert.ok(two[Account.fromEmail(bobEmail)].toEmail(), bobEmail)
   },
 
+  'login idempotence': async (assert, { client, mail, grantAccess }) => {
+    const email = 'alice@web.mail'
+    const login = client.login(email)
+    await grantAccess(await mail.take())
+    const alice = await login
+
+    assert.deepEqual(
+      Object.keys(client.accounts()),
+      [alice.did()],
+      'no accounts have been saved'
+    )
+
+    const retry = await client.login(email)
+    assert.deepEqual(
+      alice.toJSON(),
+      retry.toJSON(),
+      'same account view is returned'
+    )
+
+    const loginResult = await Account.login(client, email)
+    assert.deepEqual(
+      alice.toJSON(),
+      loginResult.ok?.toJSON(),
+      'same account is returned with low level API'
+    )
+  },
+
   'client.login': async (assert, { client, mail, grantAccess }) => {
     const account = client.login('alice@web.mail')
 
@@ -217,8 +244,6 @@ export const testAccount = {
     const space = await client.createSpace('test')
     assert.deepEqual(client.spaces(), [])
 
-    console.log(space)
-
     const result = await space.save()
     assert.ok(result.ok)