Skip to content

Commit 01f3671

Browse files
committed
Reject cookies from third-party applications.
Otherwise, when a user is logged in to their Solid server, any third-party application could perform authenticated requests without permission by including the credentials set by the Solid server. Closes #524. Breaking change, needs new semver-major.
1 parent 95701d9 commit 01f3671

File tree

3 files changed

+119
-15
lines changed

3 files changed

+119
-15
lines changed

lib/create-app.js

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ const OidcManager = require('./models/oidc-manager')
2020
const config = require('./server-config')
2121
const defaults = require('../config/defaults')
2222
const options = require('./handlers/options')
23+
const debug = require('./debug').authentication
2324

2425
const corsSettings = cors({
2526
methods: [
@@ -143,9 +144,24 @@ function initViews (app, configPath) {
143144
function initWebId (argv, app, ldp) {
144145
config.ensureWelcomePage(argv)
145146

146-
// Use session cookies
147+
// Store in a session cookie whether the user is logged in
148+
// (for direct browsing by people only)
147149
const useSecureCookies = argv.webid // argv.webid forces https and secure cookies
148-
app.use(session(sessionSettings(useSecureCookies, argv.host)))
150+
const sessionHandler = session(sessionSettings(useSecureCookies, argv.host))
151+
app.use(function (req, res, next) {
152+
// Reject cookies from third-party applications.
153+
// Otherwise, when a user is logged in to their Solid server,
154+
// any third-party application could perform authenticated requests
155+
// without permission by including the credentials set by the Solid server.
156+
const origin = req.headers.origin
157+
const expectedOrigin = `${req.protocol}://${req.headers.host}`
158+
if (origin && origin !== expectedOrigin) {
159+
debug(`Rejecting cookie for ${expectedOrigin} because it was sent by ${origin}`)
160+
req.session = {}
161+
return next()
162+
}
163+
sessionHandler(req, res, next)
164+
})
149165

150166
let accountManager = AccountManager.from({
151167
authMethod: argv.auth,

test/integration/authentication-oidc.js

Lines changed: 99 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -145,18 +145,106 @@ describe('Authentication API (OIDC)', () => {
145145
fs.removeSync(path.join(aliceDbPath, 'users/users'))
146146
})
147147

148-
it('should login and be redirected to /authorize', (done) => {
149-
alice.post('/login/password')
150-
.type('form')
151-
.send({ username: 'alice' })
152-
.send({ password: alicePassword })
153-
.expect(302)
154-
.expect('set-cookie', /connect.sid/)
155-
.end((err, res) => {
156-
let loginUri = res.header.location
157-
expect(loginUri.startsWith(aliceServerUri + '/authorize'))
158-
done(err)
148+
describe('after performing a correct login', () => {
149+
let response, cookie
150+
before(done => {
151+
aliceUserStore.initCollections()
152+
aliceUserStore.createUser(aliceAccount, alicePassword)
153+
alice.post('/login/password')
154+
.type('form')
155+
.send({ username: 'alice' })
156+
.send({ password: alicePassword })
157+
.end((err, res) => {
158+
response = res
159+
cookie = response.headers['set-cookie'][0]
160+
done(err)
161+
})
162+
})
163+
164+
it('should redirect to /authorize', () => {
165+
const loginUri = response.headers.location
166+
expect(response).to.have.property('status', 302)
167+
expect(loginUri.startsWith(aliceServerUri + '/authorize'))
168+
})
169+
170+
it('should set the cookie', () => {
171+
expect(cookie).to.match(/connect.sid=/)
172+
})
173+
174+
it('should set the cookie with HttpOnly', () => {
175+
expect(cookie).to.match(/HttpOnly/)
176+
})
177+
178+
it('should set the cookie with Secure', () => {
179+
expect(cookie).to.match(/Secure/)
180+
})
181+
182+
describe('and performing a subsequent request', () => {
183+
describe('without that cookie', () => {
184+
let response
185+
before(done => {
186+
alice.get('/')
187+
.end((err, res) => {
188+
response = res
189+
done(err)
190+
})
191+
})
192+
193+
it('should return a 401', () => {
194+
expect(response).to.have.property('status', 401)
195+
})
196+
})
197+
198+
describe('with that cookie but without origin', () => {
199+
let response
200+
before(done => {
201+
alice.get('/')
202+
.set('Cookie', cookie)
203+
.end((err, res) => {
204+
response = res
205+
done(err)
206+
})
207+
})
208+
209+
it('should return a 200', () => {
210+
expect(response).to.have.property('status', 200)
211+
})
212+
})
213+
214+
describe('with that cookie and the same origin', () => {
215+
let response
216+
before(done => {
217+
alice.get('/')
218+
.set('Cookie', cookie)
219+
.set('Origin', aliceServerUri)
220+
.end((err, res) => {
221+
response = res
222+
done(err)
223+
})
224+
})
225+
226+
it('should return a 200', () => {
227+
expect(response).to.have.property('status', 200)
228+
})
229+
})
230+
231+
describe('with that cookie and a different origin', () => {
232+
let response
233+
before(done => {
234+
alice.get('/')
235+
.set('Cookie', cookie)
236+
.set('Origin', bobServerUri)
237+
.end((err, res) => {
238+
response = res
239+
done(err)
240+
})
241+
})
242+
243+
it('should return a 401', () => {
244+
expect(response).to.have.property('status', 401)
245+
})
159246
})
247+
})
160248
})
161249

162250
it('should throw a 400 if no username is provided', (done) => {
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<#Owner>
22
a <http://www.w3.org/ns/auth/acl#Authorization> ;
33
<http://www.w3.org/ns/auth/acl#accessTo> <./>;
4-
<http://www.w3.org/ns/auth/acl#agent> <https://127.0.0.1:5000/profile/card#me>;
5-
<http://www.w3.org/ns/auth/acl#mode> <http://www.w3.org/ns/auth/acl#Read>, <http://www.w3.org/ns/auth/acl#Write>, <http://www.w3.org/ns/auth/acl#Control> .
4+
<http://www.w3.org/ns/auth/acl#agent> <https://localhost:7000/profile/card#me>;
5+
<http://www.w3.org/ns/auth/acl#mode> <http://www.w3.org/ns/auth/acl#Read>, <http://www.w3.org/ns/auth/acl#Write>, <http://www.w3.org/ns/auth/acl#Control> .

0 commit comments

Comments
 (0)