Pass returnToUrl to /register, refactor

Author: dmitrizagidulin

default-views/auth/login.hbs

@@ -42,7 +42,9 @@
     </div>
     <button type="submit" class="btn btn-primary" id="login">Login</button>
 
-    <div>Don't have an account? <a href="/register?{{{authParams}}}">Register</a></div>
+    <div>Don't have an account?
+      <a href="/register?returnToUrl={{{postRegisterUrl}}}">Register</a>
+    </div>
   </form>
 </div>
 </body>

lib/api/authn/webid-oidc.js

@@ -41,11 +41,13 @@ function middleware (oidc) {
   })
   router.post('/api/auth/select-provider', bodyParser, selectProvider)
 
-  router.get(['/login', '/signin'], (req, res) => {
+  router.get(['/login', '/signin'], LoginByPasswordRequest.get)
+  router.post(['/login', '/signin'], bodyParser, LoginByPasswordRequest.post)
+
+  router.get('/register', (req, res) => {
     let params = Object.assign({}, req.query, { authParams: url.parse(req.url).query })
-    res.render('auth/login', params)
+    res.render('auth/register', params)
   })
-  router.post(['/login', '/signin'], bodyParser, login)
 
   router.get('/goodbye', (req, res) => {
     res.render('auth/goodbye')
@@ -83,14 +85,6 @@ function selectProvider (req, res, next) {
     })
 }
 
-function login (req, res, next) {
-  return LoginByPasswordRequest.handle(req, res)
-    .catch(error => {
-      error.status = error.status || 400
-      next(error)
-    })
-}
-
 function authCodeFlowCallback (oidc, req) {
   debug.oidc('in authCodeFlowCallback()')
 
@@ -157,7 +151,6 @@ function resumeUserFlow (req, res) {
 module.exports = {
   middleware,
   selectProvider,
-  login,
   extractWebId,
   authCodeFlowCallback,
   getIssuerId,

lib/requests/login-request.js

@@ -10,7 +10,7 @@ const UserAccount = require('../models/user-account')
  * Models a Login request, a POST submit from a Login form with a username and
  * password. Used with authMethod of 'oidc'.
  *
- * For usage example, see `handle()` docstring, below.
+ * For usage example, see `post()` and `get()` docstrings, below.
  */
 class LoginByPasswordRequest {
   /**
@@ -42,24 +42,36 @@ class LoginByPasswordRequest {
   }
 
   /**
-   * Handles a Login request on behalf of a middleware handler. Usage:
+   * Handles a Login GET request on behalf of a middleware handler. Usage:
    *
    *   ```
-   *   app.post('/login', (req, res, next) = {
-   *     LoginByPasswordRequest.handle(req, res)
-   *       .catch(next)
-   *   })
+   *   app.get('/login', LoginByPasswordRequest.get)
    *   ```
    *
    * @param req {IncomingRequest}
    * @param res {ServerResponse}
    *
-   * @throws {Error} HTTP 400 error if required parameters are missing, or
-   *   if the user is not found or the password does not match.
+   * @return {Promise}
+   */
+  static get (req, res) {
+    const request = LoginByPasswordRequest.fromParams(req, res)
+
+    request.renderView()
+  }
+
+  /**
+   * Handles a Login POST request on behalf of a middleware handler. Usage:
+   *
+   *   ```
+   *   app.post('/login', LoginByPasswordRequest.post)
+   *   ```
+   *
+   * @param req {IncomingRequest}
+   * @param res {ServerResponse}
    *
    * @return {Promise}
    */
-  static handle (req, res) {
+  static post (req, res) {
     const request = LoginByPasswordRequest.fromParams(req, res)
 
     return LoginByPasswordRequest.login(request)
@@ -97,7 +109,7 @@ class LoginByPasswordRequest {
       session: req.session,
       userStore,
       accountManager,
-      authQueryParams: LoginByPasswordRequest.extractQueryParams(body)
+      authQueryParams: LoginByPasswordRequest.extractParams(req)
     }
 
     return new LoginByPasswordRequest(options)
@@ -132,18 +144,25 @@ class LoginByPasswordRequest {
    * Initializes query params required by OIDC work flow from the request body.
    * Only authorized params are loaded, all others are discarded.
    *
-   * @param body {Object} Key/value hashmap, ie `req.body`.
+   * @param req {IncomingRequest}
    *
    * @return {Object}
    */
-  static extractQueryParams (body) {
+  static extractParams (req) {
+    let params
+    if (req.method === 'POST') {
+      params = req.body || {}
+    } else {
+      params = req.query || {}
+    }
+
     let extracted = {}
 
     let paramKeys = LoginByPasswordRequest.AUTH_QUERY_PARAMS
     let value
 
     for (let p of paramKeys) {
-      value = body[p]
+      value = params[p]
       value = value === 'undefined' ? undefined : value
       extracted[p] = value
     }
@@ -153,8 +172,18 @@ class LoginByPasswordRequest {
 
   error (error) {
     let res = this.response
-    let params = Object.assign({}, this.authQueryParams, { error: error.message })
-    res.statusCode(error.statusCode || 400)
+    let params = Object.assign({}, this.authQueryParams, {error: error.message})
+
+    res.status(error.statusCode || 400)
+
+    res.render('auth/login', params)
+  }
+
+  renderView () {
+    let res = this.response
+    let params = Object.assign({}, this.authQueryParams,
+      { postRegisterUrl: this.postRegisterUrl() })
+
     res.render('auth/login', params)
   }
 
@@ -259,20 +288,45 @@ class LoginByPasswordRequest {
     return url.format(authUrl)
   }
 
-  /**
-   * Redirects the Login request to continue on the OIDC auth workflow.
-   */
-  redirectPostLogin (validUser) {
+  postLoginUrl (validUser) {
     let uri
 
     if (this.authQueryParams['redirect_uri']) {
       // Login request is part of an app's auth flow
       uri = this.authorizeUrl()
-    } else {
+    } else if (validUser) {
       // Login request is a user going to /login in browser
       uri = this.accountManager.accountUriFor(validUser.username)
+    } else {
+      let host = this.accountManager.host
+      uri = host.serverUri
+    }
+
+    return uri
+  }
+
+  postRegisterUrl () {
+    let uri
+
+    if (this.authQueryParams['redirect_uri']) {
+      // Login/register request is part of an app's auth flow
+      uri = this.authorizeUrl()
+    } else {
+      let host = this.accountManager.host
+      uri = host.serverUri
     }
 
+    uri = encodeURIComponent(uri)
+
+    return uri
+  }
+
+  /**
+   * Redirects the Login request to continue on the OIDC auth workflow.
+   */
+  redirectPostLogin (validUser) {
+    let uri = this.postLoginUrl(validUser)
+
     debug('Login successful, redirecting to ', uri)
 
     this.response.redirect(uri)

test/unit/login-by-password-request.js

@@ -27,7 +27,7 @@ const host = SolidHost.from({ serverUri: 'https://localhost:8443' })
 const accountManager = AccountManager.from({ host, authMethod })
 
 describe('LoginByPasswordRequest', () => {
-  describe('handle()', () => {
+  describe('post()', () => {
     let res, req
 
     beforeEach(() => {
@@ -43,7 +43,7 @@ describe('LoginByPasswordRequest', () => {
       let loginStub = sinon.stub(LoginByPasswordRequest, 'login')
         .returns(Promise.resolve())
 
-      return LoginByPasswordRequest.handle(req, res)
+      return LoginByPasswordRequest.post(req, res)
         .then(() => {
           expect(fromParams).to.have.been.calledWith(req, res)
           fromParams.reset()
@@ -59,7 +59,7 @@ describe('LoginByPasswordRequest', () => {
     it('should invoke login()', () => {
       let login = sinon.spy(LoginByPasswordRequest, 'login')
 
-      return LoginByPasswordRequest.handle(req, res)
+      return LoginByPasswordRequest.post(req, res)
         .then(() => {
           expect(login).to.have.been.called
           login.reset()
@@ -89,10 +89,10 @@ describe('LoginByPasswordRequest', () => {
     })
 
     it('should initialize the query params', () => {
-      let extractQueryParams = sinon.spy(LoginByPasswordRequest, 'extractQueryParams')
+      let extractParams = sinon.spy(LoginByPasswordRequest, 'extractParams')
       LoginByPasswordRequest.fromParams(req, res)
 
-      expect(extractQueryParams).to.be.calledWith(req.body)
+      expect(extractParams).to.be.calledWith(req)
     })
   })
 
@@ -349,12 +349,13 @@ describe('LoginByPasswordRequest', () => {
     return body
   }
 
-  describe('extractQueryParams()', () => {
+  describe('extractParams()', () => {
     let body = testAuthQueryParams()
     body['other_key'] = 'whatever'
+    let req = { body, method: 'POST' }
 
     it('should initialize the auth url query object from params', () => {
-      let extracted = LoginByPasswordRequest.extractQueryParams(body)
+      let extracted = LoginByPasswordRequest.extractParams(req)
 
       for (let param of LoginByPasswordRequest.AUTH_QUERY_PARAMS) {
         expect(extracted[param]).to.equal(body[param])
@@ -376,9 +377,10 @@ describe('LoginByPasswordRequest', () => {
 
     it('should pass through relevant auth query params from request body', () => {
       let body = testAuthQueryParams()
+      let req = { body, method: 'POST' }
 
       let request = new LoginByPasswordRequest({ accountManager })
-      request.authQueryParams = LoginByPasswordRequest.extractQueryParams(body)
+      request.authQueryParams = LoginByPasswordRequest.extractParams(req)
 
       let authUrl = request.authorizeUrl()
 
@@ -441,7 +443,7 @@ describe('LoginByPasswordRequest', () => {
 
     let options = { accountManager, response: res }
     let request = new LoginByPasswordRequest(options)
-    request.authQueryParams = LoginByPasswordRequest.extractQueryParams(body)
+    request.authQueryParams = LoginByPasswordRequest.extractParams(body)
 
     request.authorizeUrl = sinon.stub().returns(authUrl)