From 4e1c1155bca86436a680887fa440847a315ec565 Mon Sep 17 00:00:00 2001 From: Dmitri Zagidulin Date: Wed, 11 Jan 2017 11:41:46 -0500 Subject: [PATCH 1/3] feat(RPInitiatedLogoutRequest): First draft of RP Initiated Logout Request handling --- src/Provider.js | 13 ++ src/handlers/AuthenticationRequest.js | 4 +- src/handlers/DynamicRegistrationRequest.js | 8 ++ src/handlers/RPInitiatedLogoutRequest.js | 156 +++++++++++++++++++++ src/schemas/ClientSchema.js | 29 +++- 5 files changed, 207 insertions(+), 3 deletions(-) create mode 100644 src/handlers/RPInitiatedLogoutRequest.js diff --git a/src/Provider.js b/src/Provider.js index 4499a6c..eaeb3a0 100644 --- a/src/Provider.js +++ b/src/Provider.js @@ -12,6 +12,7 @@ const DynamicRegistrationRequest = require('./handlers/DynamicRegistrationReques const JWKSetRequest = require('./handlers/JWKSetRequest') const TokenRequest = require('./handlers/TokenRequest') const UserInfoRequest = require('./handlers/UserInfoRequest') +const RPInitiatedLogoutRequest = require('./handlers/RPInitiatedLogoutRequest') /** * OpenID Connect Provider @@ -123,6 +124,18 @@ class Provider extends JSONDocument { AuthenticationRequest.handle(req, res, this) } + /** + * Logout + * + * Bound to the OP's `end_session_endpoint` uri + * + * @param req {HTTPRequest} + * @param res {HTTPResponse} + */ + logout (req, res) { + RPInitiatedLogoutRequest.handle(req, res, this) + } + /** * Discover * diff --git a/src/handlers/AuthenticationRequest.js b/src/handlers/AuthenticationRequest.js index 4db7e78..42418d8 100644 --- a/src/handlers/AuthenticationRequest.js +++ b/src/handlers/AuthenticationRequest.js @@ -28,8 +28,8 @@ class AuthenticationRequest extends BaseRequest { Promise .resolve(request) .then(request.validate) - .then(host.authenticate) // QUESTION, what's the cleanest way to - .then(host.obtainConsent) // bind this here? + .then(host.authenticate) + .then(host.obtainConsent) .then(request.authorize) .catch(request.internalServerError) } diff --git a/src/handlers/DynamicRegistrationRequest.js b/src/handlers/DynamicRegistrationRequest.js index 71e41a2..397f61c 100644 --- a/src/handlers/DynamicRegistrationRequest.js +++ b/src/handlers/DynamicRegistrationRequest.js @@ -54,6 +54,14 @@ class DynamicRegistrationRequest extends BaseRequest { registration.client_secret = request.secret() } + /** + * TODO: Validate that the `frontchannel_logout_uri` domain and port is the same as one of the `redirect_uris` values + * @see https://openid.net/specs/openid-connect-frontchannel-1_0.html#RPLogout + * + * The domain, port, and scheme of this URL MUST be the same as that of a + * registered Redirection URI value. + */ + // initialize and validate a client let client = new Client(registration) let validation = client.validate() diff --git a/src/handlers/RPInitiatedLogoutRequest.js b/src/handlers/RPInitiatedLogoutRequest.js new file mode 100644 index 0000000..0c69e3b --- /dev/null +++ b/src/handlers/RPInitiatedLogoutRequest.js @@ -0,0 +1,156 @@ +'use strict' + +/** + * Dependencies + * @ignore + */ +const qs = require('qs') +const BaseRequest = require('./BaseRequest') + +class RPInitiatedLogoutRequest extends BaseRequest { + /** + * RP Initiated Logout Request Handler + * + * @see https://openid.net/specs/openid-connect-session-1_0.html#RPLogout + * @see https://openid.net/specs/openid-connect-frontchannel-1_0.html#RPInitiated + * @see https://openid.net/specs/openid-connect-backchannel-1_0.html#RPInitiated + * + * @param req {HTTPRequest} + * @param res {HTTPResponse} + * @param provider {Provider} + * @returns {Promise} + */ + static handle (req, res, provider) { + let { host } = provider + let request = new RPInitiatedLogoutRequest(req, res, provider) + + Promise + .resolve(request) + .then(request.validate) + + // From: https://openid.net/specs/openid-connect-session-1_0.html#RPLogout + // At the logout endpoint, the OP SHOULD ask the End-User whether they want + // to log out of the OP as well. If the End-User says "yes", then the OP + // MUST log out the End-User. + .then(host.logout) + + .then(request.redirectOrRespond) + .catch(request.internalServerError) + } + + /** + * Constructor + * + * Session spec defines the following params to the RP Initiated Logout request: + * - `id_token_hint` + * - `post_logout_redirect_uri` + * - `state` + * + * @param req {HTTPRequest} + * @param res {HTTPResponse} + * @param provider {Provider} + */ + constructor (req, res, provider) { + super(req, res, provider) + this.params = RPInitiatedLogoutRequest.getParams(this) + } + + /** + * Validate + * + * @see https://openid.net/specs/openid-connect-session-1_0.html#RPLogout + * + * @param request {RPInitiatedLogoutRequest} + */ + validate (request) { + /** + * `state` parameter - no validation need it. Will be passed back to the RP + * in the `redirectToRP()` step. + */ + + /** + * TODO: Validate `id_token_hint` (validate as ID Token *except* for `aud`) + * + * RECOMMENDED. Previously issued ID Token passed to the logout endpoint as + * a hint about the End-User's current authenticated session with the Client. + * This is used as an indication of the identity of the End-User that the RP + * is requesting be logged out by the OP. The OP *need not* be listed as an + * audience of the ID Token when it is used as an `id_token_hint` value. + */ + + /** + * TODO: Validate that `post_logout_redirect_uri` has been registered + * + * The value MUST have been previously registered with the OP, either using + * the `post_logout_redirect_uris` Registration parameter + * or via another mechanism. + * + * Question: what's this about 'another mechanism'? + */ + } + + /** + * Redirect to RP or Respond + * + * In some cases, the RP will request that the End-User's User Agent to be + * redirected back to the RP after a logout has been performed. Post-logout + * redirection is only done when the logout is RP-initiated, in which case the + * redirection target is the `post_logout_redirect_uri` query parameter value + * used by the initiating RP; otherwise it is not done. + * + * @see https://openid.net/specs/openid-connect-session-1_0.html#RedirectionAfterLogout + * + * @param request {RPInitiatedLogoutRequest} + * @returns {null} + */ + redirectOrRespond () { + let { params: { post_logout_redirect_uri: postLogoutRedirectUri } } = this + + if (postLogoutRedirectUri) { + this.redirectToRP() + } else { + this.respond() + } + } + + /** + * Redirect To RP + * + * Redirects the user-agent back to the RP, if requested (by the RP providing + * a `post_logout_redirect_uri` parameter). Also passes through the `state` + * parameter, if supplied by the RP. + * + * @returns {null} + */ + redirectToRP () { + let { res, params: { post_logout_redirect_uri: uri, state } } = this + + if (state) { + let response = qs.stringify({ state }) + uri = `${uri}?${response}` + } + + res.redirect(uri) // 302 redirect + } + + /** + * Respond + * + * Responds to the RP Initiated Logout request with a 204 No Content, if the + * RP did not supply a `post_logout_redirect_uri` parameter. + * + * @returns {null} + */ + respond () { + let { res } = this + + res.set({ + 'Cache-Control': 'no-store', + 'Pragma': 'no-cache' + }) + + res.status(204).send() + } +} + +module.exports = RPInitiatedLogoutRequest diff --git a/src/schemas/ClientSchema.js b/src/schemas/ClientSchema.js index cfe8a04..90b9e5b 100644 --- a/src/schemas/ClientSchema.js +++ b/src/schemas/ClientSchema.js @@ -72,7 +72,7 @@ const schema = new JSONSchema({ }, /** - * application_typepost_logout_redirect_uris + * application_type * OPTIONAL. Kind of the application. The default, if omitted, is web. The * defined values are native or web. Web Clients using the OAuth Implicit * Grant Type MUST only register URLs using the https scheme as @@ -245,6 +245,33 @@ const schema = new JSONSchema({ items: { format: 'uri' } + }, + + /** + * frontchannel_logout_uri + * + * OPTIONAL. RP URL that will cause the RP to log itself out when rendered + * in an iframe by the OP. + * + * @see https://openid.net/specs/openid-connect-frontchannel-1_0.html#RPLogout + */ + frontchannel_logout_uri: { + type: 'string', + format: 'uri' + }, + + /** + * frontchannel_logout_session_required + * + * OPTIONAL. Boolean value specifying whether the RP requires that `iss` + * (issuer) and `sid` (session ID) query parameters be included to identify + * the RP session with the OP when the `frontchannel_logout_uri` is used. + * + * @see https://openid.net/specs/openid-connect-frontchannel-1_0.html#RPLogout + */ + frontchannel_logout_session_required: { + type: 'boolean', + default: false } }, required: ['redirect_uris'] From d7b1b96bf822d2cf8ab260ecae8795d09b9062f9 Mon Sep 17 00:00:00 2001 From: Dmitri Zagidulin Date: Wed, 11 Jan 2017 14:36:26 -0500 Subject: [PATCH 2/3] test(RPInitiatedLogoutRequest): add test stubs --- src/handlers/RPInitiatedLogoutRequest.js | 1 - test/handlers/RPInitiatedLogoutRequestSpec.js | 28 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 test/handlers/RPInitiatedLogoutRequestSpec.js diff --git a/src/handlers/RPInitiatedLogoutRequest.js b/src/handlers/RPInitiatedLogoutRequest.js index 0c69e3b..e5b0cc2 100644 --- a/src/handlers/RPInitiatedLogoutRequest.js +++ b/src/handlers/RPInitiatedLogoutRequest.js @@ -100,7 +100,6 @@ class RPInitiatedLogoutRequest extends BaseRequest { * * @see https://openid.net/specs/openid-connect-session-1_0.html#RedirectionAfterLogout * - * @param request {RPInitiatedLogoutRequest} * @returns {null} */ redirectOrRespond () { diff --git a/test/handlers/RPInitiatedLogoutRequestSpec.js b/test/handlers/RPInitiatedLogoutRequestSpec.js new file mode 100644 index 0000000..65cea12 --- /dev/null +++ b/test/handlers/RPInitiatedLogoutRequestSpec.js @@ -0,0 +1,28 @@ +'use strict' + +// const chai = require('chai') +// const expect = chai.expect + +const RPInitiatedLogoutRequest = require('../../src/handlers/RPInitiatedLogoutRequest') + +describe('RPInitiatedLogoutRequest', () => { + describe('handle()', () => { + it('should validate the request') + it('should invoke injected host.logout') + it('should invoke redirectOrRespond() if validated') + }) + + describe('constructor()', () => { + it('should parse the incoming request params') + }) + + describe('redirectOrRespond()', () => { + it('should redirect to RP if logout uri provided') + it('should respond with a 204 if no logout uri provided') + }) + + describe('redirectToRP()', () => { + it('should redirect to provided URI') + it('should pass through the state param') + }) +}) From 4eb816d0e4233268b8e0d869fcbdfc598ff6ec07 Mon Sep 17 00:00:00 2001 From: Dmitri Zagidulin Date: Wed, 11 Jan 2017 18:54:05 -0500 Subject: [PATCH 3/3] test(RPInitiatedLogoutRequest): add more tests --- package.json | 1 + src/handlers/RPInitiatedLogoutRequest.js | 5 +- test/ProviderSpec.js | 21 +++++ test/handlers/RPInitiatedLogoutRequestSpec.js | 86 +++++++++++++++++-- 4 files changed, 102 insertions(+), 11 deletions(-) diff --git a/package.json b/package.json index 11e10ad..e89adf1 100644 --- a/package.json +++ b/package.json @@ -44,6 +44,7 @@ "devDependencies": { "chai": "^3.5.0", "mocha": "^3.2.0", + "node-mocks-http": "^1.5.5", "sinon": "^1.17.4", "sinon-chai": "^2.8.0" } diff --git a/src/handlers/RPInitiatedLogoutRequest.js b/src/handlers/RPInitiatedLogoutRequest.js index e5b0cc2..c861aa8 100644 --- a/src/handlers/RPInitiatedLogoutRequest.js +++ b/src/handlers/RPInitiatedLogoutRequest.js @@ -24,7 +24,7 @@ class RPInitiatedLogoutRequest extends BaseRequest { let { host } = provider let request = new RPInitiatedLogoutRequest(req, res, provider) - Promise + return Promise .resolve(request) .then(request.validate) @@ -34,7 +34,7 @@ class RPInitiatedLogoutRequest extends BaseRequest { // MUST log out the End-User. .then(host.logout) - .then(request.redirectOrRespond) + .then(request.redirectOrRespond.bind(request)) .catch(request.internalServerError) } @@ -104,7 +104,6 @@ class RPInitiatedLogoutRequest extends BaseRequest { */ redirectOrRespond () { let { params: { post_logout_redirect_uri: postLogoutRedirectUri } } = this - if (postLogoutRedirectUri) { this.redirectToRP() } else { diff --git a/test/ProviderSpec.js b/test/ProviderSpec.js index 9af4e8a..19d126c 100644 --- a/test/ProviderSpec.js +++ b/test/ProviderSpec.js @@ -27,6 +27,7 @@ const DynamicRegistrationRequest = require(path.join(cwd, 'src', 'handlers', 'Dy const JWKSetRequest = require(path.join(cwd, 'src', 'handlers', 'JWKSetRequest')) const TokenRequest = require(path.join(cwd, 'src', 'handlers', 'TokenRequest')) const UserInfoRequest = require(path.join(cwd, 'src', 'handlers', 'UserInfoRequest')) +const RPInitiatedLogoutRequest = require(path.join(cwd, 'src', 'handlers', 'RPInitiatedLogoutRequest')) /** * Tests @@ -223,4 +224,24 @@ describe('OpenID Connect Provider', () => { }) }) + describe('logout', () => { + let req, res, provider + + before(() => { + req = {} + res = {} + sinon.stub(RPInitiatedLogoutRequest, 'handle') + provider = new Provider({}, {}, {}) + provider.logout(req, res, provider) + }) + + after(() => { + RPInitiatedLogoutRequest.handle.restore() + }) + + it('should invoke the RPInitiatedLogoutRequest handler', () => { + RPInitiatedLogoutRequest.handle.should.have.been + .calledWith(req, res, provider) + }) + }) }) diff --git a/test/handlers/RPInitiatedLogoutRequestSpec.js b/test/handlers/RPInitiatedLogoutRequestSpec.js index 65cea12..12f58fb 100644 --- a/test/handlers/RPInitiatedLogoutRequestSpec.js +++ b/test/handlers/RPInitiatedLogoutRequestSpec.js @@ -1,24 +1,94 @@ 'use strict' -// const chai = require('chai') -// const expect = chai.expect +const chai = require('chai') +const expect = chai.expect +const sinon = require('sinon') +const sinonChai = require('sinon-chai') +chai.use(sinonChai) +chai.should() +const HttpMocks = require('node-mocks-http') const RPInitiatedLogoutRequest = require('../../src/handlers/RPInitiatedLogoutRequest') +const provider = { + host: { + logout: () => {} + } +} + +const postLogoutRedirectUri = 'https://rp.example.com/goodbye' +const reqNoParams = HttpMocks.createRequest({ method: 'GET', params: {} }) +const reqWithParams = HttpMocks.createRequest({ + method: 'GET', + query: { + 'id_token_hint': {}, + 'state': 'abc123', + 'post_logout_redirect_uri': postLogoutRedirectUri + } +}) + describe('RPInitiatedLogoutRequest', () => { describe('handle()', () => { - it('should validate the request') - it('should invoke injected host.logout') - it('should invoke redirectOrRespond() if validated') + it('should invoke injected host.logout', done => { + let res = HttpMocks.createResponse() + let logoutSpy = sinon.stub(provider.host, 'logout', () => Promise.resolve()) + + RPInitiatedLogoutRequest.handle(reqNoParams, res, provider) + .then(() => { + expect(logoutSpy).to.have.been.called + done() + }) + }) }) describe('constructor()', () => { - it('should parse the incoming request params') + it('should parse the incoming request params', done => { + let res = {} + let request = new RPInitiatedLogoutRequest(reqWithParams, res, provider) + + expect(request).to.have.property('params') + expect(Object.keys(request.params).length).to.equal(3) + expect(request.params.state).to.equal('abc123') + done() + }) + }) + + describe('validate()', () => { + it('should validate the `id_token_hint` param') + it('should validate that `post_logout_redirect_uri` has been registered') }) describe('redirectOrRespond()', () => { - it('should redirect to RP if logout uri provided') - it('should respond with a 204 if no logout uri provided') + it('should redirect to RP if logout uri provided', done => { + let res = HttpMocks.createResponse() + let req = HttpMocks.createRequest({ + method: 'GET', + query: { + 'post_logout_redirect_uri': postLogoutRedirectUri + } + }) + let request = new RPInitiatedLogoutRequest(req, res, provider) + request.respond = sinon.stub().throws() + + request.redirectOrRespond() + + expect(request.respond).to.not.have.been.called + expect(res.statusCode).to.equal(302) + expect(res._getRedirectUrl()).to.equal(postLogoutRedirectUri) + done() + }) + + it('should respond with a 204 if no logout uri provided', done => { + let res = HttpMocks.createResponse() + let request = new RPInitiatedLogoutRequest(reqNoParams, res, provider) + request.redirectToRP = sinon.stub().throws() + + request.redirectOrRespond() + + expect(request.redirectToRP).to.not.have.been.called + expect(res.statusCode).to.equal(204) + done() + }) }) describe('redirectToRP()', () => {