Skip to content

Commit 83d5630

Browse files
committed
fix(oidc): make logout endpoint handling robust for client signout flows
accept both /.well-known/solid/logout and /.well-known/solid/logout/ in OIDC auth routes classify both well-known logout paths as logout requests in session/origin checks keep logout flow consistent for cookie-backed sessions used by NSS add integration coverage for the well-known logout endpoint in OIDC authentication tests This fixes cases where client logout changed UI state but did not reliably trigger NSS server-side signout behavior.
1 parent 19765e5 commit 83d5630

4 files changed

Lines changed: 61 additions & 6 deletions

File tree

lib/api/authn/webid-oidc.mjs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,24 @@ export function middleware (oidc) {
102102
router.get('/account/password/change', restrictToTopDomain, PasswordChangeRequest.get)
103103
router.post('/account/password/change', restrictToTopDomain, bodyParser, PasswordChangeRequest.post)
104104

105-
router.get('/.well-known/solid/logout/', (req, res) => res.redirect('/logout'))
105+
router.get(['/.well-known/solid/logout', '/.well-known/solid/logout/'], (req, res) => res.redirect('/logout'))
106106

107-
router.get('/goodbye', (req, res) => { res.render('auth/goodbye') })
107+
router.get('/goodbye', (req, res, next) => {
108+
if (!req.session) {
109+
return res.render('auth/goodbye')
110+
}
111+
112+
const domain = req.session.cookie && req.session.cookie.domain
113+
req.session.destroy((err) => {
114+
if (err) {
115+
return next(err)
116+
}
117+
118+
const cookieOptions = domain ? { domain, path: '/' } : { path: '/' }
119+
res.clearCookie('nssidp.sid', cookieOptions)
120+
res.render('auth/goodbye')
121+
})
122+
})
108123

109124
// The relying party callback is called at the end of the OIDC signin process
110125
router.get('/api/oidc/rp/:issuer_id', AuthCallbackRequest.get)

lib/create-app.mjs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,10 @@ function initLoggers () {
321321
function isLogoutRequest (req) {
322322
// TODO: this is a hack that hard-codes OIDC paths,
323323
// this code should live in the OIDC module
324-
return req.path === '/logout' || req.path === '/goodbye'
324+
return req.path === '/logout' ||
325+
req.path === '/goodbye' ||
326+
req.path === '/.well-known/solid/logout' ||
327+
req.path === '/.well-known/solid/logout/'
325328
}
326329

327330
/**

test/integration/authentication-oidc-test.mjs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,41 @@ describe('Authentication API (OIDC)', () => {
532532

533533
it('should return a 200', () => expect(response).to.have.property('status', 200))
534534
})
535+
536+
describe('after logging out', () => {
537+
let logoutResponse
538+
539+
before(done => {
540+
alice.get('/logout')
541+
.set('Cookie', cookie)
542+
.end((err, res) => {
543+
logoutResponse = res
544+
done(err)
545+
})
546+
})
547+
548+
it('should redirect to the post-logout flow', () => {
549+
expect(logoutResponse).to.have.property('status', 302)
550+
})
551+
552+
it('should invalidate the previous session cookie', (done) => {
553+
alice.get('/private-for-alice.txt')
554+
.set('Cookie', cookie)
555+
.end((err, res) => {
556+
expect(res).to.have.property('status', 401)
557+
done(err)
558+
})
559+
})
560+
561+
it('should also accept the well-known logout endpoint used by solid-auth-client', (done) => {
562+
alice.get('/.well-known/solid/logout')
563+
.set('Cookie', cookie)
564+
.end((err, res) => {
565+
expect(res).to.have.property('status', 302)
566+
done(err)
567+
})
568+
})
569+
})
535570
})
536571
})
537572

test/integration/formats-test.mjs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,13 @@ describe('formats', function () {
8282
})
8383

8484
describe('turtle', function () {
85-
it('should return turtle document if Accept is set to turtle', function (done) {
86-
server.get('/patch-5-initial.ttl')
85+
this.timeout(10000)
86+
87+
it('should return turtle document if Accept is set to turtle', function () {
88+
return server.get('/patch-5-initial.ttl')
8789
.set('accept', 'text/turtle;q=0.9,application/rdf+xml;q=0.8,text/plain;q=0.7,*/*;q=0.5')
8890
.expect('content-type', /text\/turtle/)
89-
.expect(200, done)
91+
.expect(200)
9092
})
9193

9294
it('should return turtle document if Accept is set to turtle', function (done) {

0 commit comments

Comments
 (0)