Merge branch 'feature/delete-pod' of github.com:solid/node-solid-server into feature/delete-pod

kjetilk · kjetilk · commit 513778a11402 · 2018-10-04T14:20:23.000+02:00
diff --git a/lib/models/account-manager.js b/lib/models/account-manager.js
@@ -418,7 +418,40 @@ class AccountManager {
    * @return {string} Generated token
    */
   generateResetToken (userAccount) {
-    return this.tokenService.generate({ webId: userAccount.webId })
+    return this.tokenService.generate('reset-password', { webId: userAccount.webId })
+  }
+
+  /**
+   * Generates an expiring one-time-use token for password reset purposes
+   * (the user's Web ID is saved in the token service).
+   *
+   * @param userAccount {UserAccount}
+   *
+   * @return {string} Generated token
+   */
+  generateDeleteToken (userAccount) {
+    return this.tokenService.generate('delete-account', { webId: userAccount.webId })
+  }
+
+  /**
+   * Validates that a token exists and is not expired, and returns the saved
+   * token contents, or throws an error if invalid.
+   * Does not consume / clear the token.
+   *
+   * @param token {string}
+   *
+   * @throws {Error} If missing or invalid token
+   *
+   * @return {Object|false} Saved token data object if verified, false otherwise
+   */
+  validateDeleteToken (token) {
+    let tokenValue = this.tokenService.verify('delete-account', token)
+
+    if (!tokenValue) {
+      throw new Error('Invalid or expired delete account token')
+    }
+
+    return tokenValue
   }
 
   /**
@@ -433,7 +466,7 @@ class AccountManager {
    * @return {Object|false} Saved token data object if verified, false otherwise
    */
   validateResetToken (token) {
-    let tokenValue = this.tokenService.verify(token)
+    let tokenValue = this.tokenService.verify('reset-password', token)
 
     if (!tokenValue) {
       throw new Error('Invalid or expired reset token')
@@ -515,7 +548,7 @@ class AccountManager {
   sendDeleteAccountEmail (userAccount) {
     return Promise.resolve()
       .then(() => this.verifyEmailDependencies(userAccount))
-      .then(() => this.generateResetToken(userAccount))
+      .then(() => this.generateDeleteToken(userAccount))
       .then(resetToken => {
         const deleteUrl = this.getAccountDeleteUrl(resetToken)
 
diff --git a/lib/models/token-service.js b/lib/models/token-service.js
@@ -7,22 +7,26 @@ class TokenService {
     this.tokens = {}
   }
 
-  generate (data = {}) {
+  generate (domain, data = {}) {
     const token = ulid()
+    this.tokens[domain] = this.tokens[domain] || {}
 
     const value = {
       exp: new Date(Date.now() + 20 * 60 * 1000)
     }
-
-    this.tokens[token] = Object.assign({}, value, data)
+    this.tokens[domain][token] = Object.assign({}, value, data)
 
     return token
   }
 
-  verify (token) {
+  verify (domain, token) {
     const now = new Date()
 
-    let tokenValue = this.tokens[token]
+    if (!this.tokens[domain]) {
+      throw new Error(`Invalid domain for tokens: ${domain}`)
+    }
+
+    let tokenValue = this.tokens[domain][token]
 
     if (tokenValue && now < tokenValue.exp) {
       return tokenValue
@@ -31,8 +35,12 @@ class TokenService {
     }
   }
 
-  remove (token) {
-    delete this.tokens[token]
+  remove (domain, token) {
+    if (!this.tokens[domain]) {
+      throw new Error(`Invalid domain for tokens: ${domain}`)
+    }
+
+    delete this.tokens[domain][token]
   }
 }
 
diff --git a/lib/requests/delete-account-confirm-request.js b/lib/requests/delete-account-confirm-request.js
@@ -106,7 +106,7 @@ class DeleteAccountConfirmRequest extends AuthRequest {
       .then(() => {
         if (!this.token) { return false }
 
-        return this.accountManager.validateResetToken(this.token)
+        return this.accountManager.validateDeleteToken(this.token)
       })
       .then(validToken => {
         if (validToken) {
diff --git a/lib/requests/delete-account-request.js b/lib/requests/delete-account-request.js
@@ -49,12 +49,10 @@ class DeleteAccountRequest extends AuthRequest {
    * Renders the Delete form
    */
   renderForm (error) {
-    const params = {
+    this.response.render('account/delete', {
       error,
       multiuser: this.accountManager.multiuser
-    }
-
-    this.response.render('account/delete', params)
+    })
   }
 
   /**
@@ -102,6 +100,18 @@ class DeleteAccountRequest extends AuthRequest {
 
     debug(`User '${request.username}' requested to be sent a delete account email`)
 
+    return DeleteAccountRequest.handlePost(request)
+  }
+
+  /**
+   * Performs a 'send me a password reset email' request operation, after the
+   * user has entered an email into the reset form.
+   *
+   * @param request {DeleteAccountRequest}
+   *
+   * @return {Promise}
+   */
+  static handlePost (request) {
     return Promise.resolve()
       .then(() => request.validate())
       .then(() => request.loadUser())
@@ -111,11 +121,9 @@ class DeleteAccountRequest extends AuthRequest {
   }
 
   static get (req, res) {
-    let request = DeleteAccountRequest.fromParams(req, res)
+    const request = DeleteAccountRequest.fromParams(req, res)
 
-    return Promise.resolve()
-      .then(() => request.renderForm())
-      .catch(error => request.error(error))
+    request.renderForm()
   }
 
   static fromParams (req, res) {
diff --git a/lib/requests/password-reset-email-request.js b/lib/requests/password-reset-email-request.js
@@ -85,7 +85,7 @@ class PasswordResetEmailRequest extends AuthRequest {
    * Performs a 'send me a password reset email' request operation, after the
    * user has entered an email into the reset form.
    *
-   * @param request {IncomingRequest}
+   * @param request {PasswordResetEmailRequest}
    *
    * @return {Promise}
    */
diff --git a/test/unit/account-manager-test.js b/test/unit/account-manager-test.js
@@ -390,6 +390,27 @@ describe('AccountManager', () => {
     })
   })
 
+  describe('generateDeleteToken()', () => {
+    it('should generate and store an expiring delete token', () => {
+      let tokenService = new TokenService()
+      let options = { host, tokenService }
+
+      let accountManager = AccountManager.from(options)
+
+      let aliceWebId = 'https://alice.example.com/#me'
+      let userAccount = {
+        webId: aliceWebId
+      }
+
+      let token = accountManager.generateDeleteToken(userAccount)
+
+      let tokenValue = accountManager.tokenService.verify('delete-account', token)
+
+      expect(tokenValue.webId).to.equal(aliceWebId)
+      expect(tokenValue).to.have.property('exp')
+    })
+  })
+
   describe('generateResetToken()', () => {
     it('should generate and store an expiring reset token', () => {
       let tokenService = new TokenService()
@@ -404,7 +425,7 @@ describe('AccountManager', () => {
 
       let token = accountManager.generateResetToken(userAccount)
 
-      let tokenValue = accountManager.tokenService.verify(token)
+      let tokenValue = accountManager.tokenService.verify('reset-password', token)
 
       expect(tokenValue.webId).to.equal(aliceWebId)
       expect(tokenValue).to.have.property('exp')
@@ -484,4 +505,75 @@ describe('AccountManager', () => {
         })
     })
   })
+
+  describe('sendDeleteAccountEmail()', () => {
+    it('should compose and send a delete account email', () => {
+      let deleteToken = '1234'
+      let tokenService = {
+        generate: sinon.stub().returns(deleteToken)
+      }
+
+      let emailService = {
+        sendWithTemplate: sinon.stub().resolves()
+      }
+
+      let aliceWebId = 'https://alice.example.com/#me'
+      let userAccount = {
+        webId: aliceWebId,
+        email: 'alice@example.com'
+      }
+
+      let options = { host, tokenService, emailService }
+      let accountManager = AccountManager.from(options)
+
+      accountManager.getAccountDeleteUrl = sinon.stub().returns('delete account url')
+
+      let expectedEmailData = {
+        to: 'alice@example.com',
+        webId: aliceWebId,
+        deleteUrl: 'delete account url'
+      }
+
+      return accountManager.sendDeleteAccountEmail(userAccount)
+        .then(() => {
+          expect(accountManager.getAccountDeleteUrl)
+            .to.have.been.calledWith(deleteToken)
+          expect(emailService.sendWithTemplate)
+            .to.have.been.calledWith('delete-account', expectedEmailData)
+        })
+    })
+
+    it('should reject if no email service is set up', done => {
+      let aliceWebId = 'https://alice.example.com/#me'
+      let userAccount = {
+        webId: aliceWebId,
+        email: 'alice@example.com'
+      }
+      let options = { host }
+      let accountManager = AccountManager.from(options)
+
+      accountManager.sendDeleteAccountEmail(userAccount)
+        .catch(error => {
+          expect(error.message).to.equal('Email service is not set up')
+          done()
+        })
+    })
+
+    it('should reject if no user email is provided', done => {
+      let aliceWebId = 'https://alice.example.com/#me'
+      let userAccount = {
+        webId: aliceWebId
+      }
+      let emailService = {}
+      let options = { host, emailService }
+
+      let accountManager = AccountManager.from(options)
+
+      accountManager.sendDeleteAccountEmail(userAccount)
+        .catch(error => {
+          expect(error.message).to.equal('Account recovery email has not been provided')
+          done()
+        })
+    })
+  })
 })
diff --git a/test/unit/delete-account-confirm-request-test.js b/test/unit/delete-account-confirm-request-test.js
diff --git a/test/unit/delete-account-request-test.js b/test/unit/delete-account-request-test.js
diff --git a/test/unit/token-service-test.js b/test/unit/token-service-test.js

Original file line number	Diff line number	Diff line change
`@@ -7,22 +7,26 @@ class TokenService {`
`7`	`7`	`this.tokens = {}`
`8`	`8`	`}`
`9`	`9`
`10`		`- generate (data = {}) {`
	`10`	`+ generate (domain, data = {}) {`
`11`	`11`	`const token = ulid()`
	`12`	`+ this.tokens[domain] = this.tokens[domain] \|\| {}`
`12`	`13`
`13`	`14`	`const value = {`
`14`	`15`	`exp: new Date(Date.now() + 20 * 60 * 1000)`
`15`	`16`	`}`
`16`		`-`
`17`		`- this.tokens[token] = Object.assign({}, value, data)`
	`17`	`+ this.tokens[domain][token] = Object.assign({}, value, data)`
`18`	`18`
`19`	`19`	`return token`
`20`	`20`	`}`
`21`	`21`
`22`		`- verify (token) {`
	`22`	`+ verify (domain, token) {`
`23`	`23`	`const now = new Date()`
`24`	`24`
`25`		`- let tokenValue = this.tokens[token]`
	`25`	`+ if (!this.tokens[domain]) {`
	`26`	+ throw new Error(`Invalid domain for tokens: ${domain}`)
	`27`	`+ }`
	`28`	`+`
	`29`	`+ let tokenValue = this.tokens[domain][token]`
`26`	`30`
`27`	`31`	`if (tokenValue && now < tokenValue.exp) {`
`28`	`32`	`return tokenValue`
`@@ -31,8 +35,12 @@ class TokenService {`
`31`	`35`	`}`
`32`	`36`	`}`
`33`	`37`
`34`		`- remove (token) {`
`35`		`- delete this.tokens[token]`
	`38`	`+ remove (domain, token) {`
	`39`	`+ if (!this.tokens[domain]) {`
	`40`	+ throw new Error(`Invalid domain for tokens: ${domain}`)
	`41`	`+ }`
	`42`	`+`
	`43`	`+ delete this.tokens[domain][token]`
`36`	`44`	`}`
`37`	`45`	`}`
`38`	`46`
Original file line number	Diff line number	Diff line change
`@@ -85,7 +85,7 @@ class PasswordResetEmailRequest extends AuthRequest {`
`85`	`85`	`* Performs a 'send me a password reset email' request operation, after the`
`86`	`86`	`* user has entered an email into the reset form.`
`87`	`87`	`*`
`88`		`- * @param request {IncomingRequest}`
	`88`	`+ * @param request {PasswordResetEmailRequest}`
`89`	`89`	`*`
`90`	`90`	`* @return {Promise}`
`91`	`91`	`*/`