Move IDP selection and login to the client.

RubenVerborgh · RubenVerborgh · commit d364b22f3074 · 2018-08-13T10:35:46.000+02:00
Fixes #747.
diff --git a/default-views/auth/login-required.hbs b/default-views/auth/login-required.hbs
@@ -0,0 +1,43 @@
+<!doctype html>
+<html lang="en">
+<head>
+  <meta charset="utf-8">
+  <meta name="viewport" content="width=device-width, initial-scale=1">
+  <title>Log in</title>
+  <link rel="stylesheet" href="/common/css/bootstrap.min.css">
+  <script src="/common/js/solid-auth-client.bundle.js"></script>
+</head>
+<body>
+<div class="container">
+  <div class="jumbotron">
+    <h2>Log in to access this resource</h2>
+    <p>
+      The resource you are trying to access
+      (<code>{{currentUrl}}</code>)
+      requires you to log in.
+    </p>
+    <button class="btn btn-primary" onclick="login()">Log in</button>
+    <button class="btn btn-primary" onclick="register()">Register</button>
+  </div>
+</div>
+<script>
+  async function login() {
+    // Log in through the popup
+    const popupUri = '/common/popup.html'
+    const session = await SolidAuthClient.popupLogin({ popupUri })
+    if (session) {
+      // Make authenticated request to the server to establish a session cookie
+      await SolidAuthClient.fetch(location)
+      // Now that we have a cookie, reload to display the authenticated page
+      location.reload()
+    }
+  }
+
+  function register() {
+    const registration = new URL('/register', location);
+    registration.searchParams.set('returnToUrl', location);
+    location.href = registration;
+  }
+</script>
+</body>
+</html>
diff --git a/default-views/auth/select-provider.hbs b/default-views/auth/select-provider.hbs
diff --git a/lib/api/authn/webid-oidc.js b/lib/api/authn/webid-oidc.js
@@ -13,8 +13,7 @@ const PasswordChangeRequest = require('../../requests/password-change-request')
 
 const {
   AuthCallbackRequest,
-  LogoutRequest,
-  SelectProviderRequest
+  LogoutRequest
 } = require('oidc-auth-manager').handlers
 
 /**
@@ -67,9 +66,6 @@ function middleware (oidc) {
   const router = express.Router('/')
 
   // User-facing Authentication API
-  router.get('/api/auth/select-provider', SelectProviderRequest.get)
-  router.post('/api/auth/select-provider', bodyParser, SelectProviderRequest.post)
-
   router.get(['/login', '/signin'], LoginRequest.get)
 
   router.post('/login/password', bodyParser, LoginRequest.loginPassword)
diff --git a/lib/handlers/error-pages.js b/lib/handlers/error-pages.js
@@ -3,9 +3,6 @@ const fs = require('fs')
 const util = require('../utils')
 const Auth = require('../api/authn')
 
-// Authentication methods that require a Provider Select page
-const SELECT_PROVIDER_AUTH_METHODS = ['oidc']
-
 /**
  * Serves as a last-stop error handler for all other middleware.
  *
@@ -33,10 +30,7 @@ function handler (err, req, res, next) {
   if (statusCode === 401) {
     debug(err, 'error:', err.error, 'desc:', err.error_description)
     setAuthenticateHeader(req, res, err)
-  }
-
-  if (requiresSelectProvider(authMethod, statusCode, req)) {
-    return redirectToSelectProvider(req, res)
+    return renderLoginRequired(req, res)
   }
 
   // If noErrorPages is set,
@@ -67,26 +61,6 @@ function statusCodeFor (err, req, authMethod) {
   return statusCode
 }
 
-/**
- * Tests whether a given authentication method requires a Select Provider
- * page redirect for 401 error responses.
- *
- * @param authMethod {string}
- * @param statusCode {number}
- * @param req {IncomingRequest}
- *
- * @returns {boolean}
- */
-function requiresSelectProvider (authMethod, statusCode, req) {
-  if (statusCode !== 401) { return false }
-
-  if (!SELECT_PROVIDER_AUTH_METHODS.includes(authMethod)) { return false }
-
-  if (!req.accepts('text/html')) { return false }
-
-  return true
-}
-
 /**
  * Dispatches the writing of the `WWW-Authenticate` response header (used for
  * 401 Unauthorized responses).
@@ -150,26 +124,16 @@ function sendErrorPage (statusCode, res, err, ldp) {
 }
 
 /**
- * Sends a 401 response with an HTML http-equiv type redirect body, to
- * redirect any users requesting a resource directly in the browser to the
- * Select Provider page and login workflow.
- * Implemented as a 401 + redirect body instead of a 302 to provide a useful
- * 401 response to REST/XHR clients.
+ * Renders a 401 response explaining that a login is required.
  *
  * @param req {IncomingRequest}
  * @param res {ServerResponse}
  */
-function redirectToSelectProvider (req, res) {
-  res.status(401)
-  res.header('Content-Type', 'text/html')
-
-  const locals = req.app.locals
+function renderLoginRequired (req, res) {
   const currentUrl = util.fullUrlForReq(req)
-  const loginUrl = `${locals.host.serverUri}/api/auth/select-provider?returnToUrl=${encodeURIComponent(currentUrl)}`
-  debug('Redirecting to Select Provider: ' + loginUrl)
-
-  const body = redirectBody(loginUrl)
-  res.send(body)
+  debug(`Display login-required for ${currentUrl}`)
+  res.status(401)
+  res.render('auth/login-required', { currentUrl })
 }
 
 /**
@@ -199,8 +163,6 @@ follow the <a href='${url}'>link to login</a>
 module.exports = {
   handler,
   redirectBody,
-  redirectToSelectProvider,
-  requiresSelectProvider,
   sendErrorPage,
   sendErrorResponse,
   setAuthenticateHeader
diff --git a/test/integration/authentication-oidc-test.js b/test/integration/authentication-oidc-test.js
@@ -7,13 +7,11 @@ const SolidAuthOIDC = require('solid-auth-oidc')
 
 const fetch = require('node-fetch')
 const localStorage = require('localstorage-memory')
-const url = require('url')
 const URL = require('whatwg-url').URL
 global.URL = URL
 global.URLSearchParams = require('whatwg-url').URLSearchParams
 
 const supertest = require('supertest')
-const nock = require('nock')
 const chai = require('chai')
 const expect = chai.expect
 chai.use(require('dirty-chai'))
@@ -89,48 +87,6 @@ describe('Authentication API (OIDC)', () => {
     fs.removeSync(path.join(bobRootPath, 'index.html.acl'))
   })
 
-  describe('Provider Discovery (POST /api/auth/select-provider)', () => {
-    it('form should load on a get', done => {
-      alice.get('/api/auth/select-provider')
-        .expect(200)
-        .expect((res) => { res.text.match(/Provider Discovery/) })
-        .end(done)
-    })
-
-    it('should complain if WebID URI is missing', (done) => {
-      alice.post('/api/auth/select-provider')
-        .expect(400, done)
-    })
-
-    it('should prepend https:// to webid, if necessary', (done) => {
-      alice.post('/api/auth/select-provider')
-        .type('form')
-        .send({ webid: 'localhost:7000' })
-        .expect(302, done)
-    })
-
-    it("should return a 400 if endpoint doesn't have Link Headers", (done) => {
-      // Fake provider, replies with 200 and no Link headers
-      nock('https://amazingwebsite.tld').intercept('/', 'OPTIONS').reply(204)
-
-      alice.post('/api/auth/select-provider')
-        .send('webid=https://amazingwebsite.tld/')
-        .expect(400)
-        .end(done)
-    })
-
-    it('should redirect user to discovered provider if valid uri', (done) => {
-      bob.post('/api/auth/select-provider')
-        .send('webid=' + aliceServerUri)
-        .expect(302)
-        .end((err, res) => {
-          let loginUri = res.header.location
-          expect(loginUri.startsWith(aliceServerUri + '/authorize'))
-          done(err)
-        })
-    })
-  })
-
   describe('Login page (GET /login)', () => {
     it('should load the user login form', () => {
       return alice.get('/login')
@@ -279,39 +235,13 @@ describe('Authentication API (OIDC)', () => {
     })
   })
 
-  describe('Two Pods + Browser Login workflow', () => {
-    // Step 1: Alice tries to access bob.com/shared-with-alice.txt, and
-    //   gets redirected to bob.com's Provider Discovery endpoint
-    it('401 Unauthorized -> redirect to provider discovery', (done) => {
+  describe('Browser login workflow', () => {
+    it('401 Unauthorized asking the user to log in', (done) => {
       bob.get('/shared-with-alice.txt')
         .expect(401)
         .end((err, res) => {
-          if (err) return done(err)
-          let redirectString = 'http-equiv="refresh" ' +
-            `content="0; url=${bobServerUri}/api/auth/select-provider`
-          expect(res.text).to.match(new RegExp(redirectString))
-          done()
-        })
-    })
-
-    // Step 2: Alice enters her pod's URI to Bob's Provider Discovery endpoint
-    it('Enter webId -> redirect to provider login', () => {
-      return bob.post('/api/auth/select-provider')
-        .send('webid=' + aliceServerUri)
-        .expect(302)
-        .then(res => {
-          // Submitting select-provider form redirects to Alice's pod's /authorize
-          let authorizeUri = res.header.location
-          expect(authorizeUri.startsWith(aliceServerUri + '/authorize'))
-
-          // Follow the redirect to /authorize
-          let authorizePath = url.parse(authorizeUri).path
-          return alice.get(authorizePath)
-        })
-        .then(res => {
-          // Since alice not logged in to her pod, /authorize redirects to /login
-          let loginUri = res.header.location
-          expect(loginUri.startsWith('/login'))
+          expect(res.text).to.contain('Log in')
+          done(err)
         })
     })
   })
diff --git a/test/integration/errors-oidc-test.js b/test/integration/errors-oidc-test.js
@@ -43,12 +43,12 @@ describe('OIDC error handling', function () {
           .expect(401)
       })
 
-      it('should return an html redirect body', () => {
+      it('should return an html login page', () => {
         return server.get('/profile/')
           .set('Accept', 'text/html')
           .expect('Content-Type', 'text/html; charset=utf-8')
           .then(res => {
-            expect(res.text).to.match(/<meta http-equiv="refresh" content="0; url=https:\/\/localhost:3457\/api\/auth\/select-provider/)
+            expect(res.text).to.match(/Log in/)
           })
       })
     })
diff --git a/test/resources/accounts-acl/config/views/auth/login-required.hbs b/test/resources/accounts-acl/config/views/auth/login-required.hbs
diff --git a/test/resources/accounts-acl/config/views/auth/select-provider.hbs b/test/resources/accounts-acl/config/views/auth/select-provider.hbs
diff --git a/test/unit/error-pages-test.js b/test/unit/error-pages-test.js
@@ -39,40 +39,6 @@ describe('handlers/error-pages', () => {
     })
   })
 
-  describe('requiresSelectProvider()', () => {
-    it('should only apply to 401 error codes', () => {
-      let authMethod = 'oidc'
-      let req = { accepts: sinon.stub().withArgs('text/html').returns(true) }
-
-      expect(errorPages.requiresSelectProvider(authMethod, 404, req))
-        .to.equal(false)
-      expect(errorPages.requiresSelectProvider(authMethod, 401, req))
-        .to.equal(true)
-    })
-
-    it('should only apply to oidc auth method', () => {
-      let statusCode = 401
-      let req = { accepts: sinon.stub().withArgs('text/html').returns(true) }
-
-      expect(errorPages.requiresSelectProvider('tls', statusCode, req))
-        .to.equal(false)
-      expect(errorPages.requiresSelectProvider('oidc', statusCode, req))
-        .to.equal(true)
-    })
-
-    it('should only apply to html requests', () => {
-      let authMethod = 'oidc'
-      let statusCode = 401
-      let htmlReq = { accepts: sinon.stub().withArgs('text/html').returns(true) }
-      let nonHtmlReq = { accepts: sinon.stub().withArgs('text/html').returns(false) }
-
-      expect(errorPages.requiresSelectProvider(authMethod, statusCode, nonHtmlReq))
-        .to.equal(false)
-      expect(errorPages.requiresSelectProvider(authMethod, statusCode, htmlReq))
-        .to.equal(true)
-    })
-  })
-
   describe('sendErrorResponse()', () => {
     it('should send http status code and error message', () => {
       let statusCode = 404
