Fix plus-sign recovery email handling

Author: SarthakDudhe

lib/models/account-manager.mjs

@@ -231,14 +231,14 @@ class AccountManager {
         return this.store.getGraph(rootAclUri)
       })
       .then(rootAclGraph => {
-        const matches = rootAclGraph.match(null, ns.acl('agent'))
-        let recoveryMailto = matches.find(agent => agent.object.value.startsWith('mailto:'))
-        if (recoveryMailto) {
-          recoveryMailto = recoveryMailto.object.value.replace('mailto:', '')
-        }
-        return recoveryMailto
-      })
-  }
+        const matches = rootAclGraph.match(null, ns.acl('agent'))
+        let recoveryMailto = matches.find(agent => agent.object.value.startsWith('mailto:'))
+        if (recoveryMailto) {
+          recoveryMailto = decodeURIComponent(recoveryMailto.object.value.replace('mailto:', ''))
+        }
+        return recoveryMailto
+      })
+  }
 
   verifyEmailDependencies (userAccount) {
     if (!this.emailService) {

lib/models/account-template.mjs

@@ -9,7 +9,7 @@ import { URL } from 'url'
 export const TEMPLATE_EXTENSIONS = ['.acl', '.meta', '.json', '.hbs', '.handlebars']
 export const TEMPLATE_FILES = ['card']
 
-class AccountTemplate {
+class AccountTemplate {
   constructor (options = {}) {
     this.substitutions = options.substitutions || {}
     this.templateExtensions = options.templateExtensions || TEMPLATE_EXTENSIONS
@@ -26,17 +26,28 @@ class AccountTemplate {
     return fsUtils.copyTemplateDir(templatePath, accountPath)
   }
 
-  static templateSubstitutionsFor (userAccount) {
-    const webUri = new URL(userAccount.webId)
-    const podRelWebId = userAccount.webId.replace(webUri.origin, '')
-    const substitutions = {
-      name: userAccount.displayName,
-      webId: userAccount.externalWebId ? userAccount.webId : podRelWebId,
-      email: userAccount.email,
-      idp: userAccount.idp
-    }
-    return substitutions
-  }
+  static templateSubstitutionsFor (userAccount) {
+    const webUri = new URL(userAccount.webId)
+    const podRelWebId = userAccount.webId.replace(webUri.origin, '')
+    const substitutions = {
+      name: userAccount.displayName,
+      webId: userAccount.externalWebId ? userAccount.webId : podRelWebId,
+      email: AccountTemplate.encodeMailtoAddress(userAccount.email),
+      idp: userAccount.idp
+    }
+    return substitutions
+  }
+
+  static encodeMailtoAddress (email) {
+    if (!email) {
+      return email
+    }
+    const [localPart, domain] = email.split('@')
+    if (!domain) {
+      return encodeURIComponent(email)
+    }
+    return `${encodeURIComponent(localPart)}@${domain}`
+  }
 
   readAccountFiles (accountPath) {
     return new Promise((resolve, reject) => {

test/unit/account-manager-test.mjs

@@ -358,8 +358,8 @@ describe('AccountManager', () => {
   })
 
   describe('loadAccountRecoveryEmail()', () => {
-    it('parses and returns the agent mailto from the root acl', () => {
-      const userAccount = UserAccount.from({ username: 'alice' })
+    it('parses and returns the agent mailto from the root acl', () => {
+      const userAccount = UserAccount.from({ username: 'alice' })
 
       const rootAclGraph = rdf.graph()
       rootAclGraph.add(
@@ -377,13 +377,37 @@ describe('AccountManager', () => {
       const accountManager = AccountManager.from(options)
 
       return accountManager.loadAccountRecoveryEmail(userAccount)
-        .then(recoveryEmail => {
-          expect(recoveryEmail).to.equal('alice@example.com')
-        })
-    })
-
-    it('should return undefined when agent mailto is missing', () => {
-      const userAccount = UserAccount.from({ username: 'alice' })
+        .then(recoveryEmail => {
+          expect(recoveryEmail).to.equal('alice@example.com')
+        })
+    })
+
+    it('decodes encoded plus signs in the agent mailto from the root acl', () => {
+      const userAccount = UserAccount.from({ username: 'solidgold' })
+
+      const rootAclGraph = rdf.graph()
+      rootAclGraph.add(
+        rdf.namedNode('https://solidgold.example.com/.acl#owner'),
+        ns.acl('agent'),
+        rdf.namedNode('mailto:jon%2Bsolidtest@snap.com')
+      )
+
+      const store = {
+        suffixAcl: '.acl',
+        getGraph: sinon.stub().resolves(rootAclGraph)
+      }
+
+      const options = { host, multiuser: true, store }
+      const accountManager = AccountManager.from(options)
+
+      return accountManager.loadAccountRecoveryEmail(userAccount)
+        .then(recoveryEmail => {
+          expect(recoveryEmail).to.equal('jon+solidtest@snap.com')
+        })
+    })
+
+    it('should return undefined when agent mailto is missing', () => {
+      const userAccount = UserAccount.from({ username: 'alice' })
 
       const emptyGraph = rdf.graph()
 

test/unit/account-template-test.mjs

@@ -40,19 +40,32 @@ describe('AccountTemplate', () => {
   })
 
   describe('templateSubstitutionsFor()', () => {
-    it('should init', () => {
-      const userOptions = {
-        username: 'alice',
-        webId: 'https://alice.example.com/profile/card#me',
-        name: 'Alice Q.',
+    it('should init', () => {
+      const userOptions = {
+        username: 'alice',
+        webId: 'https://alice.example.com/profile/card#me',
+        name: 'Alice Q.',
         email: 'alice@example.com'
       }
       const userAccount = UserAccount.from(userOptions)
 
       const substitutions = AccountTemplate.templateSubstitutionsFor(userAccount)
       expect(substitutions.name).to.equal('Alice Q.')
-      expect(substitutions.email).to.equal('alice@example.com')
-      expect(substitutions.webId).to.equal('/profile/card#me')
-    })
-  })
-})
+      expect(substitutions.email).to.equal('alice@example.com')
+      expect(substitutions.webId).to.equal('/profile/card#me')
+    })
+
+    it('encodes plus signs in email addresses for mailto templates', () => {
+      const userOptions = {
+        username: 'solidgold',
+        webId: 'https://solidgold.example.com/profile/card#me',
+        name: 'Solid Gold',
+        email: 'jon+solidtest@snap.com'
+      }
+      const userAccount = UserAccount.from(userOptions)
+
+      const substitutions = AccountTemplate.templateSubstitutionsFor(userAccount)
+      expect(substitutions.email).to.equal('jon%2Bsolidtest@snap.com')
+    })
+  })
+})