Skip to content

Commit eca2d4b

Browse files
dmitrizagidulindmitrizagidulin
committed
Fix 401 error handling, add tests (#507)
* Add WWW-Authenticate to exposed CORS headers * Fix 401 error handling, add tests * Add a test for expired token error * Update to supertest 3.0.0 * Convert 401 tests to Promise interface * Rewrite error-pages handler, add tests * Change 401 error logic on empty bearer token * Return a 400 error on empty bearer token header
1 parent 9b19974 commit eca2d4b

File tree

11 files changed

+635
-53
lines changed

11 files changed

+635
-53
lines changed

‎lib/api/authn/webid-oidc.js‎

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,78 @@ function middleware (oidc) {
6767
return router
6868
}
6969

70+
/**
71+
* Sets the `WWW-Authenticate` response header for 401 error responses.
72+
* Used by error-pages handler.
73+
*
74+
* @param req {IncomingRequest}
75+
* @param res {ServerResponse}
76+
* @param err {Error}
77+
*/
78+
function setAuthenticateHeader (req, res, err) {
79+
let locals = req.app.locals
80+
81+
let errorParams = {
82+
realm: locals.host.serverUri,
83+
scope: 'openid',
84+
error: err.error,
85+
error_description: err.error_description,
86+
error_uri: err.error_uri
87+
}
88+
89+
let challengeParams = Object.keys(errorParams)
90+
.filter(key => !!errorParams[key])
91+
.map(key => `${key}="${errorParams[key]}"`)
92+
.join(', ')
93+
94+
res.set('WWW-Authenticate', 'Bearer ' + challengeParams)
95+
}
96+
97+
/**
98+
* Provides custom logic for error status code overrides.
99+
*
100+
* @param statusCode {number}
101+
* @param req {IncomingRequest}
102+
*
103+
* @returns {number}
104+
*/
105+
function statusCodeOverride (statusCode, req) {
106+
if (isEmptyToken(req)) {
107+
return 400
108+
} else {
109+
return statusCode
110+
}
111+
}
112+
113+
/**
114+
* Tests whether the `Authorization:` header includes an empty or missing Bearer
115+
* token.
116+
*
117+
* @param req {IncomingRequest}
118+
*
119+
* @returns {boolean}
120+
*/
121+
function isEmptyToken (req) {
122+
let header = req.get('Authorization')
123+
124+
if (!header) { return false }
125+
126+
if (header.startsWith('Bearer')) {
127+
let fragments = header.split(' ')
128+
129+
if (fragments.length === 1) {
130+
return true
131+
} else if (!fragments[1]) {
132+
return true
133+
}
134+
}
135+
136+
return false
137+
}
138+
70139
module.exports = {
71-
middleware
140+
isEmptyToken,
141+
middleware,
142+
setAuthenticateHeader,
143+
statusCodeOverride
72144
}

‎lib/api/authn/webid-tls.js‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
module.exports = handler
2-
module.exports.authenticate = authenticate
3-
41
var webid = require('webid/tls')
52
var debug = require('../../debug').authentication
63

@@ -43,3 +40,23 @@ function setEmptySession (req) {
4340
req.session.userId = ''
4441
req.session.identified = false
4542
}
43+
44+
/**
45+
* Sets the `WWW-Authenticate` response header for 401 error responses.
46+
* Used by error-pages handler.
47+
*
48+
* @param req {IncomingRequest}
49+
* @param res {ServerResponse}
50+
*/
51+
function setAuthenticateHeader (req, res) {
52+
let locals = req.app.locals
53+
54+
res.set('WWW-Authenticate', `WebID-TLS realm="${locals.host.serverUri}"`)
55+
}
56+
57+
module.exports = {
58+
authenticate,
59+
handler,
60+
setAuthenticateHeader,
61+
setEmptySession
62+
}

‎lib/create-app.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ const corsSettings = cors({
2525
methods: [
2626
'OPTIONS', 'HEAD', 'GET', 'PATCH', 'POST', 'PUT', 'DELETE'
2727
],
28-
exposedHeaders: 'Authorization, User, Location, Link, Vary, Last-Modified, ETag, Accept-Patch, Accept-Post, Updates-Via, Allow, Content-Length',
28+
exposedHeaders: 'Authorization, User, Location, Link, Vary, Last-Modified, ETag, Accept-Patch, Accept-Post, Updates-Via, Allow, Content-Length, WWW-Authenticate',
2929
credentials: true,
3030
maxAge: 1728000,
3131
origin: true,
@@ -71,7 +71,7 @@ function createApp (argv = {}) {
7171
app.use('/', LdpMiddleware(corsSettings))
7272

7373
// Errors
74-
app.use(errorPages)
74+
app.use(errorPages.handler)
7575

7676
return app
7777
}

‎lib/handlers/error-pages.js‎

Lines changed: 176 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,53 +1,188 @@
1-
module.exports = handler
1+
const debug = require('../debug').server
2+
const fs = require('fs')
3+
const util = require('../utils')
4+
const Auth = require('../api/authn')
25

3-
var debug = require('../debug').server
4-
var fs = require('fs')
5-
var util = require('../utils')
6+
// Authentication methods that require a Provider Select page
7+
const SELECT_PROVIDER_AUTH_METHODS = ['oidc']
68

9+
/**
10+
* Serves as a last-stop error handler for all other middleware.
11+
*
12+
* @param err {Error}
13+
* @param req {IncomingRequest}
14+
* @param res {ServerResponse}
15+
* @param next {Function}
16+
*/
717
function handler (err, req, res, next) {
818
debug('Error page because of ' + err.message)
919

10-
var ldp = req.app.locals.ldp
11-
12-
if (err.status === 401 &&
13-
req.accepts('text/html') &&
14-
ldp.auth === 'oidc') {
15-
debug('401 error - redirect to Select Provider')
16-
res.status(err.status)
17-
redirectToLogin(req, res, next)
18-
return
19-
}
20+
let locals = req.app.locals
21+
let authMethod = locals.authMethod
22+
let ldp = locals.ldp
2023

21-
// If the user specifies this function
22-
// then, they can customize the error programmatically
24+
// If the user specifies this function,
25+
// they can customize the error programmatically
2326
if (ldp.errorHandler) {
27+
debug('Using custom error handler')
2428
return ldp.errorHandler(err, req, res, next)
2529
}
2630

31+
let statusCode = statusCodeFor(err, req, authMethod)
32+
33+
if (statusCode === 401) {
34+
setAuthenticateHeader(req, res, err)
35+
}
36+
37+
if (requiresSelectProvider(authMethod, statusCode, req)) {
38+
return redirectToSelectProvider(req, res)
39+
}
40+
2741
// If noErrorPages is set,
28-
// then use built-in express default error handler
42+
// then return the response directly
2943
if (ldp.noErrorPages) {
30-
res
31-
.status(err.status)
32-
.send(err.message + '\n' || '')
33-
return
44+
sendErrorResponse(statusCode, res, err)
45+
} else {
46+
sendErrorPage(statusCode, res, err, ldp)
47+
}
48+
}
49+
50+
/**
51+
* Returns the HTTP status code for a given request error.
52+
*
53+
* @param err {Error}
54+
* @param req {IncomingRequest}
55+
* @param authMethod {string}
56+
*
57+
* @returns {number}
58+
*/
59+
function statusCodeFor (err, req, authMethod) {
60+
let statusCode = err.status || err.statusCode || 500
61+
62+
if (authMethod === 'oidc') {
63+
statusCode = Auth.oidc.statusCodeOverride(statusCode, req)
3464
}
3565

36-
// Check if error page exists
37-
var errorPage = ldp.errorPages + err.status.toString() + '.html'
38-
fs.readFile(errorPage, 'utf8', function (readErr, text) {
39-
if (readErr) {
40-
return res
41-
.status(err.status)
42-
.send(err.message || '')
43-
}
44-
45-
res.status(err.status)
46-
res.header('Content-Type', 'text/html')
47-
res.send(text)
66+
return statusCode
67+
}
68+
69+
/**
70+
* Tests whether a given authentication method requires a Select Provider
71+
* page redirect for 401 error responses.
72+
*
73+
* @param authMethod {string}
74+
* @param statusCode {number}
75+
* @param req {IncomingRequest}
76+
*
77+
* @returns {boolean}
78+
*/
79+
function requiresSelectProvider (authMethod, statusCode, req) {
80+
if (statusCode !== 401) { return false }
81+
82+
if (!SELECT_PROVIDER_AUTH_METHODS.includes(authMethod)) { return false }
83+
84+
if (!req.accepts('text/html')) { return false }
85+
86+
return true
87+
}
88+
89+
/**
90+
* Dispatches the writing of the `WWW-Authenticate` response header (used for
91+
* 401 Unauthorized responses).
92+
*
93+
* @param req {IncomingRequest}
94+
* @param res {ServerResponse}
95+
* @param err {Error}
96+
*/
97+
function setAuthenticateHeader (req, res, err) {
98+
let locals = req.app.locals
99+
let authMethod = locals.authMethod
100+
101+
switch (authMethod) {
102+
case 'oidc':
103+
Auth.oidc.setAuthenticateHeader(req, res, err)
104+
break
105+
case 'tls':
106+
Auth.tls.setAuthenticateHeader(req, res)
107+
break
108+
default:
109+
break
110+
}
111+
}
112+
113+
/**
114+
* Sends the HTTP status code and error message in the response.
115+
*
116+
* @param statusCode {number}
117+
* @param res {ServerResponse}
118+
* @param err {Error}
119+
*/
120+
function sendErrorResponse (statusCode, res, err) {
121+
res.status(statusCode)
122+
res.send(err.message + '\n')
123+
}
124+
125+
/**
126+
* Sends the HTTP status code and error message as a custom error page.
127+
*
128+
* @param statusCode {number}
129+
* @param res {ServerResponse}
130+
* @param err {Error}
131+
* @param ldp {LDP}
132+
*/
133+
function sendErrorPage (statusCode, res, err, ldp) {
134+
let errorPage = ldp.errorPages + statusCode.toString() + '.html'
135+
136+
return new Promise((resolve) => {
137+
fs.readFile(errorPage, 'utf8', (readErr, text) => {
138+
if (readErr) {
139+
// Fall back on plain error response
140+
return resolve(sendErrorResponse(statusCode, res, err))
141+
}
142+
143+
res.status(statusCode)
144+
res.header('Content-Type', 'text/html')
145+
res.send(text)
146+
resolve()
147+
})
48148
})
49149
}
50150

151+
/**
152+
* Sends a 401 response with an HTML http-equiv type redirect body, to
153+
* redirect any users requesting a resource directly in the browser to the
154+
* Select Provider page and login workflow.
155+
* Implemented as a 401 + redirect body instead of a 302 to provide a useful
156+
* 401 response to REST/XHR clients.
157+
*
158+
* @param req {IncomingRequest}
159+
* @param res {ServerResponse}
160+
*/
161+
function redirectToSelectProvider (req, res) {
162+
res.status(401)
163+
res.header('Content-Type', 'text/html')
164+
165+
let currentUrl = util.fullUrlForReq(req)
166+
req.session.returnToUrl = currentUrl
167+
168+
let locals = req.app.locals
169+
let loginUrl = locals.host.serverUri +
170+
'/api/auth/select-provider?returnToUrl=' + currentUrl
171+
debug('Redirecting to Select Provider: ' + loginUrl)
172+
173+
let body = redirectBody(loginUrl)
174+
res.send(body)
175+
}
176+
177+
/**
178+
* Returns a response body for redirecting browsers to a Select Provider /
179+
* login workflow page. Uses either a JS location.href redirect or an
180+
* http-equiv type html redirect for no-script conditions.
181+
*
182+
* @param url {string}
183+
*
184+
* @returns {string} Response body
185+
*/
51186
function redirectBody (url) {
52187
return `<!DOCTYPE HTML>
53188
<meta charset="UTF-8">
@@ -63,15 +198,12 @@ follow the <a href='${url}'>link to login</a>
63198
`
64199
}
65200

66-
function redirectToLogin (req, res) {
67-
res.header('Content-Type', 'text/html')
68-
var currentUrl = util.fullUrlForReq(req)
69-
req.session.returnToUrl = currentUrl
70-
let locals = req.app.locals
71-
let loginUrl = locals.host.serverUri +
72-
'/api/auth/select-provider?returnToUrl=' + currentUrl
73-
debug('Redirecting to Select Provider: ' + loginUrl)
74-
75-
var body = redirectBody(loginUrl)
76-
res.send(body)
201+
module.exports = {
202+
handler,
203+
redirectBody,
204+
redirectToSelectProvider,
205+
requiresSelectProvider,
206+
sendErrorPage,
207+
sendErrorResponse,
208+
setAuthenticateHeader
77209
}

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@
8888
"sinon": "^2.1.0",
8989
"sinon-chai": "^2.8.0",
9090
"standard": "^8.6.0",
91-
"supertest": "^1.2.0"
91+
"supertest": "^3.0.0"
9292
},
9393
"main": "index.js",
9494
"scripts": {

0 commit comments

Comments
 (0)