From 3ebb860dbea157cc3a4b36f78e81895c0f0f71b9 Mon Sep 17 00:00:00 2001 From: Henrich Kraemer Date: Mon, 19 Oct 2015 17:02:48 +0200 Subject: [PATCH 1/9] Remove mailer#getMailer method and remove caching In oidc/requireVerifiedEmail.js the following pattern was used: var mailer = require('../boot/mailer').getMailer() ... later in the code ... the mailer is referenced return function requireVerifiedEmail (req, res, next) { ... from: options.locals.from || mailer.from, Having a mailer variable on the module level does not allow to easily swap in test stubs or properties. If this is done requireVerifiedEmail will continue to use the mailer as it was initialized during the initial server setup. The code above could have been changed to call getMailer() in the requireVerifiedEmail(req, res, next) function body. However it appeared simpler to simply remove the getMailer function so that all mailer users would use the same module reference as cached by node. --- boot/mailer.js | 58 +++++++++------------ boot/server.js | 2 +- oidc/requireVerifiedEmail.js | 2 +- oidc/sendVerificationEmail.js | 2 +- routes/recovery.js | 2 +- routes/resendEmail.js | 2 +- routes/signin.js | 2 +- test/unit/oidc/sendVerificationEmail.coffee | 43 +++++++-------- 8 files changed, 50 insertions(+), 63 deletions(-) diff --git a/boot/mailer.js b/boot/mailer.js index 3c61ce36..242351e4 100644 --- a/boot/mailer.js +++ b/boot/mailer.js @@ -66,38 +66,32 @@ function sendMail (template, locals, options, callback) { /** * Get mailer */ +function setup () { + var fromVerifier = /^(?:\w|\s)+<[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,}>$/igm + var transport = settings.mailer && + nodemailer.createTransport(settings.mailer) + + engineName = (settings.mailer && settings.mailer.view_engine) || + settings.view_engine || + 'hogan' + engine = cons[engineName] + + if (transport && (typeof settings.mailer.from !== 'string' || + !fromVerifier.test(settings.mailer.from))) { + console.error(settings.mailer.from) + throw new Error('From field not provided for mailer. ' + + 'Expected "Display Name "') + } + + defaultFrom = settings.mailer && settings.mailer.from -var mailer - -exports.getMailer = function () { - if (mailer) { - return mailer - } else { - var fromVerifier = /^(?:\w|\s)+<[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,}>$/igm - var transport = settings.mailer && - nodemailer.createTransport(settings.mailer) - - engineName = (settings.mailer && settings.mailer.view_engine) || - settings.view_engine || - 'hogan' - engine = cons[engineName] - - if (transport && (typeof settings.mailer.from !== 'string' || - !fromVerifier.test(settings.mailer.from))) { - console.error(settings.mailer.from) - throw new Error('From field not provided for mailer. ' + - 'Expected "Display Name "') - } - - defaultFrom = settings.mailer && settings.mailer.from - - mailer = { - from: defaultFrom, - render: render, - transport: transport, - sendMail: sendMail - } - - return mailer + var mailer = { + from: defaultFrom, + render: render, + transport: transport, + sendMail: sendMail } + return mailer } + +module.exports = setup() diff --git a/boot/server.js b/boot/server.js index 3b684e30..5c7894d3 100644 --- a/boot/server.js +++ b/boot/server.js @@ -21,7 +21,7 @@ var settings = require('./settings') var setup = require('./setup') var client = require('./redis').getClient() var logger = require('./logger')(settings.logger) -require('./mailer').getMailer() +require('./mailer') require('./migrate')() var authenticator = require('../lib/authenticator') var express = require('express') diff --git a/oidc/requireVerifiedEmail.js b/oidc/requireVerifiedEmail.js index b667867f..02356c75 100644 --- a/oidc/requireVerifiedEmail.js +++ b/oidc/requireVerifiedEmail.js @@ -12,7 +12,7 @@ */ var Role = require('../models/Role') -var mailer = require('../boot/mailer').getMailer() +var mailer = require('../boot/mailer') var settings = require('../boot/settings') var url = require('url') diff --git a/oidc/sendVerificationEmail.js b/oidc/sendVerificationEmail.js index f924fe4d..fe6462a4 100644 --- a/oidc/sendVerificationEmail.js +++ b/oidc/sendVerificationEmail.js @@ -58,7 +58,7 @@ function sendVerificationEmail (req, res, next) { } // Send verification email - mailer.getMailer().sendMail('verifyEmail', locals, { + mailer.sendMail('verifyEmail', locals, { to: user.email, subject: 'Verify your e-mail address' }, function (err, responseStatus) { diff --git a/routes/recovery.js b/routes/recovery.js index 5c8a1530..56ccf069 100644 --- a/routes/recovery.js +++ b/routes/recovery.js @@ -5,7 +5,7 @@ var url = require('url') var revalidator = require('revalidator') var settings = require('../boot/settings') -var mailer = require('../boot/mailer').getMailer() +var mailer = require('../boot/mailer') var User = require('../models/User') var OneTimeToken = require('../models/OneTimeToken') var PasswordsDisabledError = require('../errors/PasswordsDisabledError') diff --git a/routes/resendEmail.js b/routes/resendEmail.js index af79175e..7805ba64 100644 --- a/routes/resendEmail.js +++ b/routes/resendEmail.js @@ -2,7 +2,7 @@ * Module dependencies */ -var mailer = require('../boot/mailer').getMailer() +var mailer = require('../boot/mailer') var oidc = require('../oidc') var User = require('../models/User') diff --git a/routes/signin.js b/routes/signin.js index 64538dab..f8dc271f 100644 --- a/routes/signin.js +++ b/routes/signin.js @@ -4,7 +4,7 @@ var oidc = require('../oidc') var settings = require('../boot/settings') -var mailer = require('../boot/mailer').getMailer() +var mailer = require('../boot/mailer') var authenticator = require('../lib/authenticator') var qs = require('qs') var InvalidRequestError = require('../errors/InvalidRequestError') diff --git a/test/unit/oidc/sendVerificationEmail.coffee b/test/unit/oidc/sendVerificationEmail.coffee index fb8d274b..05e053ae 100644 --- a/test/unit/oidc/sendVerificationEmail.coffee +++ b/test/unit/oidc/sendVerificationEmail.coffee @@ -3,36 +3,31 @@ sinon = require 'sinon' sinonChai = require 'sinon-chai' expect = chai.expect - - - chai.use sinonChai chai.should() - +_ = require 'lodash' mailer = require '../../../boot/mailer' -fakeMailer = - sendMail: (tmpl, loc, opts, cb) -> - cb() + +mailer_state = {} {sendVerificationEmail} = require '../../../oidc' OneTimeToken = require '../../../models/OneTimeToken' - describe 'Send Verification Email', -> before -> - sinon.stub mailer, 'getMailer', -> - fakeMailer + _.assign(mailer_state, mailer) + mailer.sendMail = (tmpl, loc, opts, cb) -> + cb() after -> - mailer.getMailer.restore() + _.assign(mailer, mailer_state) {req,res,next} = {} - describe 'when not requested', -> before -> @@ -45,13 +40,13 @@ describe 'Send Verification Email', -> next = sinon.spy() sinon.spy(OneTimeToken, 'issue') - sinon.spy(fakeMailer, 'sendMail') + sinon.spy(mailer, 'sendMail') sendVerificationEmail req, res, next after -> OneTimeToken.issue.restore() - fakeMailer.sendMail.restore() + mailer.sendMail.restore() it 'should continue', -> next.should.have.been.called @@ -60,7 +55,7 @@ describe 'Send Verification Email', -> OneTimeToken.issue.should.not.have.been.called it 'should not send an email', -> - fakeMailer.sendMail.should.not.have.been.called + mailer.sendMail.should.not.have.been.called @@ -91,13 +86,13 @@ describe 'Send Verification Email', -> ttl: 3600 * 24 * 7 use: 'emailVerification' }) - sinon.stub(fakeMailer, 'sendMail').callsArgWith 3, null, null + sinon.stub(mailer, 'sendMail').callsArgWith 3, null, null sendVerificationEmail req, res, next after -> OneTimeToken.issue.restore() - fakeMailer.sendMail.restore() + mailer.sendMail.restore() it 'should issue a token to the user', -> OneTimeToken.issue.should.have.been.calledWith sinon.match({ @@ -115,25 +110,25 @@ describe 'Send Verification Email', -> }) it 'should send to the user', -> - fakeMailer.sendMail.should.have.been + mailer.sendMail.should.have.been .calledWith 'verifyEmail', sinon.match.object, sinon.match({ to: req.user.email }) it 'should provide a subject', -> - fakeMailer.sendMail.should.have.been + mailer.sendMail.should.have.been .calledWith 'verifyEmail', sinon.match.object, sinon.match({ subject: sinon.match.string }) it 'should render with the user email', -> - fakeMailer.sendMail.should.have.been + mailer.sendMail.should.have.been .calledWith 'verifyEmail', sinon.match({ email: req.user.email }) it 'should render with the user given name', -> - fakeMailer.sendMail.should.have.been + mailer.sendMail.should.have.been .calledWith 'verifyEmail', sinon.match({ name: { first: req.user.givenName @@ -141,7 +136,7 @@ describe 'Send Verification Email', -> }) it 'should render with the user family name', -> - fakeMailer.sendMail.should.have.been + mailer.sendMail.should.have.been .calledWith 'verifyEmail', sinon.match({ name: { last: req.user.familyName @@ -149,7 +144,7 @@ describe 'Send Verification Email', -> }) it 'should render with the verification url', -> - fakeMailer.sendMail.should.have.been + mailer.sendMail.should.have.been .calledWith 'verifyEmail', sinon.match({ verifyURL: sinon.match.string }) @@ -157,5 +152,3 @@ describe 'Send Verification Email', -> it 'should continue', -> next.should.have.been.called next.should.not.have.been.calledWith sinon.match.any - - From 1ac8f8e0b27a3ed55fe22522102d4927e900792d Mon Sep 17 00:00:00 2001 From: Henrich Kraemer Date: Mon, 19 Oct 2015 17:20:51 +0200 Subject: [PATCH 2/9] Add verifyMailerConfigured.js and test --- oidc/verifyMailerConfigured.js | 17 +++++++ test/unit/oidc/verifyMailerConfigured.coffee | 52 ++++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 oidc/verifyMailerConfigured.js create mode 100644 test/unit/oidc/verifyMailerConfigured.coffee diff --git a/oidc/verifyMailerConfigured.js b/oidc/verifyMailerConfigured.js new file mode 100644 index 00000000..2d29d55c --- /dev/null +++ b/oidc/verifyMailerConfigured.js @@ -0,0 +1,17 @@ +/** + * Module dependencies + */ + +// TODO share this with other email validations + +var mailer = require('../boot/mailer') + +function verifyMailerConfigured (req, res, next) { + if (!mailer.transport) { + return next(new Error('Mailer not configured.')) + } else { + return next() + } +} + +module.exports = verifyMailerConfigured diff --git a/test/unit/oidc/verifyMailerConfigured.coffee b/test/unit/oidc/verifyMailerConfigured.coffee new file mode 100644 index 00000000..1eaf65af --- /dev/null +++ b/test/unit/oidc/verifyMailerConfigured.coffee @@ -0,0 +1,52 @@ +chai = require 'chai' +should = chai.should() +expect = chai.expect + +_ = require 'lodash' + +{verifyMailerConfigured} = require '../../../oidc' +mailer = require '../../../boot/mailer' +mailer_state = {} + +# these are not used in this middleware. +# TODO: perhaps this check should not be a middleware check +# as it is not actually looking at the request. +req = {} +res = {} +err = {} + +describe 'Verify Mailer Configuration', -> + + before (done) -> + _.assign(mailer_state, mailer) + done() + + after -> + _.assign(mailer, mailer_state) + + describe 'with missing mailer configuration data', -> + + before (done) -> + delete mailer.transport + + verifyMailerConfigured req, res, (error) -> + err = error + done() + + it 'should call next with an Error', -> + err.name.should.equal 'Error' + + it 'Error should reveal that mailer is not configured', -> + err.message.should.equal 'Mailer not configured.' + + describe 'with mailer configuration data', -> + + before (done) -> + mailer.transport = {} + + verifyMailerConfigured req, res, (error) -> + err = error + done() + + it 'should call next with no error', -> + should.not.exist(err) From c4a98430ca688a382be63b3e77ec631c5935e105 Mon Sep 17 00:00:00 2001 From: Henrich Kraemer Date: Mon, 19 Oct 2015 22:34:47 +0200 Subject: [PATCH 3/9] Initial passwordless code with tests. tests and changes to make tests pass: - integration test route resending passwordless email - integration test route signin passwordless - unit test verify email valid - unit test determine provider pwless_resend.coffee to pass even if passwordless not in config settings --- email/passwordlessSignin.hogan | 28 ++ email/passwordlessSignup.hogan | 25 ++ errors/PasswordlessDisabledError.js | 23 ++ oidc/determineProvider.js | 24 +- oidc/index.js | 2 + oidc/verifyEmailValid.js | 32 +++ protocols/Passwordless.js | 41 +++ providers/index.js | 5 + providers/passwordless.js | 19 ++ routes/connect.js | 2 +- routes/passwordless.js | 252 ++++++++++++++++++ routes/signin.js | 72 ++--- server.js | 1 + test/integration/routes/pwless_resend.coffee | 106 ++++++++ test/integration/routes/pwless_signin.coffee | 104 ++++++++ test/unit/oidc/determineProvider.coffee | 35 ++- test/unit/oidc/verifyEmailValid.coffee | 92 +++++++ views/passwordless/pwlessSentEmail.jade | 34 +++ .../pwlessSigninLinkVerified.jade | 21 ++ 19 files changed, 871 insertions(+), 47 deletions(-) create mode 100644 email/passwordlessSignin.hogan create mode 100644 email/passwordlessSignup.hogan create mode 100644 errors/PasswordlessDisabledError.js create mode 100644 oidc/verifyEmailValid.js create mode 100644 protocols/Passwordless.js create mode 100644 providers/passwordless.js create mode 100644 routes/passwordless.js create mode 100644 test/integration/routes/pwless_resend.coffee create mode 100644 test/integration/routes/pwless_signin.coffee create mode 100644 test/unit/oidc/verifyEmailValid.coffee create mode 100644 views/passwordless/pwlessSentEmail.jade create mode 100644 views/passwordless/pwlessSigninLinkVerified.jade diff --git a/email/passwordlessSignin.hogan b/email/passwordlessSignin.hogan new file mode 100644 index 00000000..a23f4f1a --- /dev/null +++ b/email/passwordlessSignin.hogan @@ -0,0 +1,28 @@ + + + +
+

{{providerName}}

+

+ Sign in to {{providerName}} +

+

Follow the link below by clicking the button if you + want to sign in to {{providerName}}. +

+ +

+ Sign in to {{providerName}}

+

If you don't see a link above, paste the following URL into your browser: {{pwlessURL}}

+

This e-mail was addressed to {{email}}

+
+ + diff --git a/email/passwordlessSignup.hogan b/email/passwordlessSignup.hogan new file mode 100644 index 00000000..63d28b9f --- /dev/null +++ b/email/passwordlessSignup.hogan @@ -0,0 +1,25 @@ + + + +
+

{{providerName}}

+

+ Verify your e-mail address +

+

Great! You're almost done setting up your new {{providerName}} account.

+

All that's left is to + Verify your e-mail address

+

If you don't see a link above, paste the following URL into your browser: {{verifyURL}}

+

This e-mail was addressed to {{email}}

+
+ + diff --git a/errors/PasswordlessDisabledError.js b/errors/PasswordlessDisabledError.js new file mode 100644 index 00000000..42d214ea --- /dev/null +++ b/errors/PasswordlessDisabledError.js @@ -0,0 +1,23 @@ +/** + * Module dependencies + */ + +var util = require('util') + +/** + * PasswordlessDisabledError + */ + +function PasswordlessDisabledError () { + this.name = 'PasswordlessDisabledError' + this.message = 'Email sign-in is disabled' + this.statusCode = 400 +} + +util.inherits(PasswordlessDisabledError, Error) + +/** + * Exports + */ + +module.exports = PasswordlessDisabledError diff --git a/oidc/determineProvider.js b/oidc/determineProvider.js index b2fcf7fc..f77994cf 100644 --- a/oidc/determineProvider.js +++ b/oidc/determineProvider.js @@ -4,21 +4,33 @@ var settings = require('../boot/settings') var providers = require('../providers') +var InvalidRequestError = require('../errors/InvalidRequestError') /** * Determine provider middleware */ -function determineProvider (req, res, next) { - var providerID = req.params.provider || req.body.provider +function determineProvider (options, req, res, next) { + var providerID = req.params.provider || req.connectParams.provider if (providerID && settings.providers[providerID]) { req.provider = providers[providerID] } + if (options.requireProvider && !req.provider) { + return next(new InvalidRequestError('Invalid provider')) + } next() } -/** - * Module export - */ +module.exports = function (req, res, next) { + determineProvider({}, req, res, next) +} -module.exports = determineProvider +module.exports.setup = function (options) { + options = options || {} + options = { + requireProvider: options.requireProvider || false + } + return function (req, res, next) { + determineProvider(options, req, res, next) + } +} diff --git a/oidc/index.js b/oidc/index.js index 83946615..b516f56f 100644 --- a/oidc/index.js +++ b/oidc/index.js @@ -45,6 +45,8 @@ var oidc = { verifyClientToken: require('./verifyClientToken'), verifyClientIdentifiers: require('./verifyClientIdentifiers'), verifyEmail: require('./verifyEmail'), + verifyEmailValid: require('./verifyEmailValid'), + verifyMailerConfigured: require('./verifyMailerConfigured'), verifyRedirectURI: require('./verifyRedirectURI'), verifyAuthorizationCode: require('./verifyAuthorizationCode') } diff --git a/oidc/verifyEmailValid.js b/oidc/verifyEmailValid.js new file mode 100644 index 00000000..9985e18b --- /dev/null +++ b/oidc/verifyEmailValid.js @@ -0,0 +1,32 @@ +/** + * Module dependencies + */ + +// TODO share this with other email validations + +var revalidator = require('revalidator') + +/** + * Verify Email in req.connectParams.email is valid. + * + * This checks the validate on a syntax level. + * + * This middleware will not cause any errors, but instead remove + * the invalid values off of req.connectParams object. + */ + +function verifyEmailValid (req, res, next) { + if ( + !req.connectParams.email || + !revalidator.validate.formats.email.test(req.connectParams.email) + ) { + delete req.connectParams.email + } + next() +} + +/** + * Exports + */ + +module.exports = verifyEmailValid diff --git a/protocols/Passwordless.js b/protocols/Passwordless.js new file mode 100644 index 00000000..13b2bf94 --- /dev/null +++ b/protocols/Passwordless.js @@ -0,0 +1,41 @@ +/** + * Passwordless Protocol + * + * This is essentially empty but is used as stub object so that other + * code dealing with protocols can stay ignorant of this. + * + * The logic which in other case is implemented in the + * Protocol class is implemented in routes/passwordless. + */ + +function Passwordless (provider, configuration) { + this._provider = provider + this._configuration = configuration + this._emailField = 'email' +} + +/** + * Initialize + */ + +function initialize (provider, configuration) { + return new Passwordless(provider, configuration) +} + +Passwordless.initialize = initialize + +/* + * Authenticate request based on the contents of a form submission. + * + * @param {Object} req + * @api protected + */ +Passwordless.prototype.authenticate = function (req, options) { + throw new Error('Passwordless authenticate must not be called') +} + +/** + * Exports + */ + +module.exports = Passwordless diff --git a/providers/index.js b/providers/index.js index 1b3951e1..0a24f8d7 100644 --- a/providers/index.js +++ b/providers/index.js @@ -97,6 +97,11 @@ function loadProviders (dir, files) { if (typeof oamr !== 'undefined') { provider.amr = oamr } + // override the default token TTL (expiration) for the provider + var ottl = settings.providers[providerName].tokenTTL + if (typeof ottl !== 'undefined') { + provider.tokenTTL = ottl + } // provider-specific refresh_userinfo setting var orefuser_info = settings.providers[providerName].refresh_userinfo diff --git a/providers/passwordless.js b/providers/passwordless.js new file mode 100644 index 00000000..dd7bc4bc --- /dev/null +++ b/providers/passwordless.js @@ -0,0 +1,19 @@ +/** + * Passwordless + * + * Signin/Signup by clicking a link send by email. + */ + +module.exports = function (config) { + return { + id: 'passwordless', + name: 'Email only', + protocol: 'Passwordless', + amr: 'pwd', // TODO: is there a proper amr value for passwordless? + tokenTTL: 60 * 15, + fields: [ + { name: 'email', type: 'email' } + ], + usernameField: 'email' // TODO: write test that this is used. + } +} diff --git a/routes/connect.js b/routes/connect.js index 81d80d15..10a1b380 100644 --- a/routes/connect.js +++ b/routes/connect.js @@ -4,7 +4,7 @@ var settings = require('../boot/settings') var oidc = require('../oidc') -var mailer = require('../boot/mailer').getMailer() +var mailer = require('../boot/mailer') var authenticator = require('../lib/authenticator') var qs = require('qs') var NotFoundError = require('../errors/NotFoundError') diff --git a/routes/passwordless.js b/routes/passwordless.js new file mode 100644 index 00000000..4bbef1e0 --- /dev/null +++ b/routes/passwordless.js @@ -0,0 +1,252 @@ +/** + * Module dependencies + */ + +var express = require('express') +var url = require('url') +var settings = require('../boot/settings') +var mailer = require('../boot/mailer') +var authenticator = require('../lib/authenticator') +var oidc = require('../oidc') +var User = require('../models/User') +var OneTimeToken = require('../models/OneTimeToken') +var InvalidRequestError = require('../errors/InvalidRequestError') +var PasswordlessDisabledError = require('../errors/PasswordlessDisabledError') + +/** + * Passwordless + * + * Both new and existing users start at the signin page + * + * Common flow to signin or signup, starting at /signin : + + * 1. User provides email + * 2. Validate input as valid email, display error if not. + * 3. Check whether user account exists. + * 4. Send link in either case with a one time token expiring + * after 15 minutes by default. + * 4.1 If user account exists email tells user to click and confirm + * that they want to sign in. + * Link to route /signin/pwless/... + * 4.2 If new user email tells user to click the link if that want + * to create a new user account. + * Link to route /signup/pwless/... + * + * New user passwordless signup flow: (/signup/pwless) + * + * 1. Verify token exists and is for pwless. + * 2. Direct user to new passwordless user view, where user can enter additional details such as last name, first name. + * 3. Create user based on form information, revoke token. + * 4. Send welcome mail. + * 5. Authorize user + * + * Existing user passwordless sigin flow: (/signin/pwless) + * + * 1. Verify token exists and is for pwless. + * 2. Update user email verified status with current time. + * 3. Authorize user. + */ + +var TOKEN_USAGE_SIGNIN = 'pwless-signin' +var TOKEN_USAGE_SIGNUP = 'pwless-signup' + +function verifyPasswordlessEnabled (req, res, next) { + if (!settings.providers.passwordless) { + return next(new PasswordlessDisabledError()) + } else { + return next() + } +} + +// perhaps this should be dispached by the authenticator +// however I am not sure I see what that would bring? +function verifyPasswordlessSigninToken (req, res, next) { + if (!req.query.token) { + return next(new InvalidRequestError('Missing token')) + } + + var view = 'passwordless/pwlessSigninLinkVerified' + + // consume the token + OneTimeToken.consume(req.query.token, function (err, token) { + if (err) { return next(err) } + + // Invalid or expired token + if (!token || token.use !== TOKEN_USAGE_SIGNIN) { + return res.render(view, { + error: 'Invalid or expired link' + }) + } + + // Update the user + User.patch(token.sub, { + dateEmailVerified: Date.now(), + emailVerified: true + }, function (err, user) { + if (err) { return next(err) } + + // unknown user, might happen if token expired or + // link activated twice. + if (!user) { + return res.render(view, { + error: 'Unable to verify email for this user.' + }) + } + + // analog to Password signin handler after authenticator.dispatch + // authenticated user based on password. + req.user = user + authenticator.login(req, user) + next() + }) + }) +} + +function signinRenderErrorInvalidEmail (req, res, next) { + if (typeof req.connectParams.email === 'undefined') { + return res.render('signin', { + error: 'Please enter a valid e-mail address.' + }) + } + next() +} + +function sendMail (req, res, next) { + User.getByEmail(req.connectParams.email, function (err, user) { + if (err) { return next(err) } + + // TODO: There may be differences in processing delays + // whether user has an account or not. This could potentially + // be used for an attack. + + // user = null is treated as new signup. + // The email send is tailored to whether it is a new sign up or + // a sign in of an existing user + + var email = req.connectParams.email + + var tokenOptions = { + ttl: settings.providers.passwordless.tokenTTL || 60 * 15, + use: user ? TOKEN_USAGE_SIGNIN : TOKEN_USAGE_SIGNUP, + sub: user ? user._id : email + } + OneTimeToken.issue(tokenOptions, function (err, token) { + if (err) { return next(err) } + + var verifyURL = url.parse(settings.issuer) + verifyURL.pathname = user ? 'signin/pwless' : 'signup/pwless' + verifyURL.query = { token: token._id } + + var template = user ? 'passwordlessSignin' : 'passwordlessSignup' + var subject = user + ? 'Sign in to ' + req.client.client_name + : 'Create your account on ' + req.client.client_name + + // TODO: test is {{providerName}} available in template? + // TODO: see also usage of #{signin.client.client_name} in verifyEmail.jade, oidc.verifyClient + + mailer.sendMail(template, { + email: email, + verifyURL: url.format(verifyURL) + }, { + to: email, + subject: subject + }, function (err, responseStatus) { + if (err) { } // TODO: REQUIRES REFACTOR TO MAIL QUEUE + renderSentMail(req, res, next) + }) + }) + }) +} + +function renderSentMail (req, res, next) { + // this is similar to the middleware in requireVerifiedEmail + // but there are differences: + // 1. Not sure role.name authority handling makes sense here. + // 2. emailVerified is no reason to skip this. + // 3. Resend URL has different path ('resend/passwordless' instead of 'email/resend') + // 3. The messages are differently worded to the case. + // So perhaps not sharing will make this somewhat easier. + + var resendURL = url.parse(settings.issuer) + resendURL.pathname = 'resend/passwordless' + resendURL.query = { + email: req.connectParams.email + } + if (req.connectParams) { + resendURL.query.redirect_uri = req.connectParams.redirect_uri + resendURL.query.client_id = req.connectParams.client_id + resendURL.query.response_type = req.connectParams.response_type + resendURL.query.scope = req.connectParams.scope + } + var locals = { + from: mailer.from, + resendURL: url.format(resendURL) + } + res.render('passwordless/pwlessSentEmail', locals) +} + +/* + * It is expected that the following middleware is used prior + * to this: + oidc.selectConnectParams, + oidc.verifyClient, + oidc.validateAuthorizationParams, + oidc.determineProvider.setup({requireProvider: true}), + oidc.enforceReferrer('/signin'), + In addition it was checked that req.body.provider === 'passwordless' + */ + +function postSigninMiddleware () { + var middleware = express.Router() + var handler = [ + verifyPasswordlessEnabled, + oidc.verifyMailerConfigured, + oidc.verifyEmailValid, + signinRenderErrorInvalidEmail, + sendMail + ] + middleware.use(handler) + function passwordlessSignin (req, res, next) { + // allows setting a breakpoint here and some nicer stack traces. + middleware(req, res, next) + } + return passwordlessSignin +} + +// signin/pwless, signup/pwless, +function routes (server) { + server.get('/resend/:provider', + oidc.selectConnectParams, + oidc.verifyClient, + oidc.validateAuthorizationParams, + oidc.determineProvider.setup({requireProvider: true}), + verifyPasswordlessEnabled, + oidc.verifyMailerConfigured, + oidc.verifyEmailValid, + function (req, res, next) { + if (typeof req.connectParams.email === 'undefined') { + next(new InvalidRequestError('invalid email')) + } + next() + }, + sendMail + ) + server.get('/signin/pwless', + oidc.selectConnectParams, + oidc.verifyClient, + oidc.validateAuthorizationParams, + oidc.determineProvider.setup({requireProvider: true}), + verifyPasswordlessEnabled, + verifyPasswordlessSigninToken, + oidc.determineUserScope, + oidc.promptToAuthorize, + oidc.authorize + ) +} + +module.exports = { + routes: routes, + signin: postSigninMiddleware, + verifyEnabled: verifyPasswordlessEnabled +} diff --git a/routes/signin.js b/routes/signin.js index f8dc271f..238c683f 100644 --- a/routes/signin.js +++ b/routes/signin.js @@ -2,12 +2,13 @@ * Module dependencies */ +var express = require('express') var oidc = require('../oidc') var settings = require('../boot/settings') var mailer = require('../boot/mailer') var authenticator = require('../lib/authenticator') +var passwordless = require('./passwordless') var qs = require('qs') -var InvalidRequestError = require('../errors/InvalidRequestError') var providers = require('../providers') var providerInfo = {} @@ -47,44 +48,53 @@ module.exports = function (server) { }) /** - * Password signin handler + * Password signin and Passwordless post handler. */ + var passwordSignin = express.Router() // initialized further down. + var handler = [ oidc.selectConnectParams, oidc.verifyClient, oidc.validateAuthorizationParams, - oidc.determineProvider, + oidc.determineProvider.setup({requireProvider: true}), oidc.enforceReferrer('/signin'), function (req, res, next) { - if (!req.provider) { - next(new InvalidRequestError('Invalid provider')) + if (req.body.provider === 'passwordless') { + // for passwordless flow see comments in passwordless + passwordless.signin()(req, res, next) } else { - authenticator.dispatch(req.body.provider, req, res, next, function (err, user, info) { - if (err) { - res.render('signin', { - params: qs.stringify(req.body), - request: req.body, - providers: visibleProviders, - providerInfo: providerInfo, - mailSupport: !!(mailer.transport), - error: err.message - }) - } else if (!user) { - res.render('signin', { - params: qs.stringify(req.body), - request: req.body, - providers: visibleProviders, - providerInfo: providerInfo, - mailSupport: !!(mailer.transport), - formError: info.message - }) - } else { - authenticator.login(req, user) - next() - } - }) + passwordSignin(req, res, next) } + } + ] + + var passwordSigninHandler = [ + function (req, res, next) { + authenticator.dispatch(req.body.provider, req, res, next, function (err, user, info) { + if (err) { + res.render('signin', { + params: qs.stringify(req.body), + request: req.body, + providers: visibleProviders, + providerInfo: providerInfo, + mailSupport: !!(mailer.transport), + error: err.message + }) + } else if (!user) { + res.render('signin', { + params: qs.stringify(req.body), + request: req.body, + providers: visibleProviders, + providerInfo: providerInfo, + mailSupport: !!(mailer.transport), + formError: info.message + }) + } else { + authenticator.login(req, user) + next() + } + }) }, oidc.requireVerifiedEmail(), oidc.determineUserScope, @@ -93,8 +103,10 @@ module.exports = function (server) { ] if (oidc.beforeAuthorize) { - handler.splice(handler.length - 1, 0, oidc.beforeAuthorize) + passwordSigninHandler.splice(passwordSigninHandler.length - 1, 0, oidc.beforeAuthorize) } + passwordSignin.use(passwordSigninHandler) + server.post('/signin', handler) } diff --git a/server.js b/server.js index 564bfcd9..75dd814f 100644 --- a/server.js +++ b/server.js @@ -28,6 +28,7 @@ require('./routes/userinfo')(server) require('./routes/signin')(server) require('./routes/signup')(server) require('./routes/signout')(server) +require('./routes/passwordless').routes(server) require('./routes/recovery')(server) require('./routes/resendEmail')(server) require('./routes/verifyEmail')(server) diff --git a/test/integration/routes/pwless_resend.coffee b/test/integration/routes/pwless_resend.coffee new file mode 100644 index 00000000..526eaa82 --- /dev/null +++ b/test/integration/routes/pwless_resend.coffee @@ -0,0 +1,106 @@ +# Test dependencies +chai = require 'chai' +sinon = require 'sinon' +sinonChai = require 'sinon-chai' +supertest = require 'supertest' +expect = chai.expect +qs = require 'qs' + + +# Assertions +chai.use sinonChai +chai.should() + +_ = require 'lodash' + +# Code under test +server = require '../../../server' +Client = require('../../../models/Client') +User = require('../../../models/User') +server = require '../../../server' + +settings = require '../../../boot/settings' + +request = supertest(server) + +describe 'Passwordless resend email route', -> + + mailer = require '../../../boot/mailer' + mailer_state = {} + fakeMailer = + from: "from@example.com" + render: {} + sendMail: (tmpl, loc, opts, cb) -> + cb() + transport: {} + + before -> + _.assign(mailer_state, mailer) + _.assign(mailer, fakeMailer) + + after -> + _.assign(mailer, mailer_state) + + {err, res} = {} + + + describe 'GET resend/passwordless', -> + + describe 'success flow', -> + + settings_providers_state = {} + pwless_settings = + passwordless: + "tokenTTL-foo": 600 + + + + before (done) -> + + _.assign(settings_providers_state, settings.providers) + _.assign(settings.providers, pwless_settings) + + sinon.stub(Client, 'get').callsArgWith(2, null, + { + client_name: 'unit test pwless_resend' + redirect_uris: [ + 'http://localhost:9000/callback_popup.html' + ] + trusted: true + response_types: ['id_token token'] + grant_types: ['implicit'] + }) + + sinon.stub(User, 'getByEmail').callsArgWith(1, null, null) + + query = + client_id: '4a2c1a31-150d-49e3-9946-2909220cdb16' + redirect_uri: 'http://localhost:9000/callback_popup.html' + response_type: 'id_token token' + scope: 'openid profile' + nonce: 'KG4vsD0bfAjbEdCMurmiPxzEcpFGoguYGR7b3cj3AMs' + email: 'foo@example.com' + + request + .get('/resend/passwordless?' + qs.stringify(query)) + .end (error, response) -> + err = error + res = response + done() + + after -> + settings.providers = settings_providers_state; + User.getByEmail.restore() + Client.get.restore() + + it 'should respond 200', -> + res.statusCode.should.equal 200 + + it 'should respond with an html page', -> + res.headers['content-type'].should.contain 'text/html' + + it 'should respond with html page containing the sender', -> + res.text.should.contain 'from@example.com' + + it 'should respond with html page containing the resend link', -> + res.text.should.match (new RegExp("href=\"http(s){0,1}://[^/]{0,}/resend/passwordless\\?email=foo%40example\\.com&", "g")) diff --git a/test/integration/routes/pwless_signin.coffee b/test/integration/routes/pwless_signin.coffee new file mode 100644 index 00000000..bbe57440 --- /dev/null +++ b/test/integration/routes/pwless_signin.coffee @@ -0,0 +1,104 @@ +# Test dependencies +chai = require 'chai' +sinon = require 'sinon' +sinonChai = require 'sinon-chai' +supertest = require 'supertest' +expect = chai.expect + + +# Assertions +chai.use sinonChai +chai.should() + +_ = require 'lodash' + +# Code under test +server = require '../../../server' + +settings = require '../../../boot/settings' + +request = supertest(server) + +describe 'Passwordless signin post', -> + + mailer = require '../../../boot/mailer' + mailer_state = {} + fakeMailer = + from: "from@example.com" + render: {} + sendMail: (tmpl, loc, opts, cb) -> + cb() + transport: {} + + before -> + _.assign(mailer_state, mailer) + _.assign(mailer, fakeMailer) + + after -> + _.assign(mailer, mailer_state) + + {err, res} = {} + + + describe 'POST signin body passwordless', -> + + describe 'success flow', -> + + settings_providers_state = {} + pwless_settings = + passwordless: + "tokenTTL-foo": 600 + + before (done) -> + + _.assign(settings_providers_state, settings.providers) + _.assign(settings.providers, pwless_settings) + + # # + # sinon.stub(Client, 'get').callsArgWith(2, null, + # { + # client_name: 'unit test pwless_signin' + # redirect_uris: [ + # 'http://localhost:9000/callback_popup.html' + # ] + # trusted: true + # response_types: ['id_token token'] + # grant_types: ['implicit'] + # }) + # + # sinon.stub(User, 'getByEmail').callsArgWith(1, null, null) + + fields = + client_id: '4a2c1a31-150d-49e3-9946-2909220cdb16' + redirect_uri: 'http://localhost:9000/callback_popup.html' + response_type: 'id_token token' + scope: 'openid profile' + nonce: 'KG4vsD0bfAjbEdCMurmiPxzEcpFGoguYGR7b3cj3AMs' + email: 'foo@example.com' + provider: 'passwordless' + + request + .post('/signin') + .set('referer', settings.issuer + '/signin') + .send(fields) + .end (error, response) -> + err = error + res = response + done() + + after -> + settings.providers = settings_providers_state; + # User.getByEmail.restore() + # Client.get.restore() + + it 'should respond 200', -> + res.statusCode.should.equal 200 + + it 'should respond with an html page', -> + res.headers['content-type'].should.contain 'text/html' + + it 'should respond with html page containing the sender', -> + res.text.should.contain 'from@example.com' + + it 'should respond with html page containing the resend link', -> + res.text.should.match (new RegExp("href=\"http(s){0,1}://[^/]{0,}/resend/passwordless\\?email=foo%40example\\.com&", "g")) diff --git a/test/unit/oidc/determineProvider.coffee b/test/unit/oidc/determineProvider.coffee index 19158ca0..6da0ae13 100644 --- a/test/unit/oidc/determineProvider.coffee +++ b/test/unit/oidc/determineProvider.coffee @@ -15,6 +15,7 @@ chai.should() {determineProvider} = require '../../../oidc' settings = require '../../../boot/settings' providers = require '../../../providers' +InvalidRequestError = require '../../../errors/InvalidRequestError' @@ -30,7 +31,7 @@ describe 'Determine Provider', -> describe 'with provider on params', -> before -> - req = { method: 'GET', params: { provider: 'password' }, body: {} } + req = { method: 'GET', params: { provider: 'password' }, connectParams: {}} res = {} next = sinon.spy() settingsProviders = settings.providers @@ -48,10 +49,10 @@ describe 'Determine Provider', -> - describe 'with provider on body', -> + describe 'with provider on connectParams', -> before -> - req = { method: 'GET', params: { }, body: { provider: 'password' } } + req = { method: 'GET', params: { }, connectParams: { provider: 'password' } } res = {} next = sinon.spy() settingsProviders = settings.providers @@ -73,7 +74,7 @@ describe 'Determine Provider', -> describe 'with unknown provider on body', -> before -> - req = { method: 'GET', params: {}, body: { provider: '/\\~!@#$%^&*(_+' } } + req = { method: 'GET', params: {}, connectParams: { provider: '/\\~!@#$%^&*(_+' } } res = {} next = sinon.spy() settingsProviders = settings.providers @@ -89,13 +90,32 @@ describe 'Determine Provider', -> it 'should continue', -> next.should.have.been.called + describe 'with unknown provider on body with requireProvider options', -> + + before -> + req = { method: 'GET', params: {}, connectParams: { provider: '/\\~!@#$%^&*(_+' } } + res = {} + next = sinon.spy() + settingsProviders = settings.providers + settings.providers = { 'password': {} } + determineProvider = determineProvider.setup {requireProvider: true} + determineProvider req, res, next + + after -> + settings.providers = settingsProviders + + it 'should not load a provider', -> + req.should.not.have.property 'provider' + + it 'should continue with InvalidRequestError: Invalid provider', -> + next.should.have.been.calledWith new InvalidRequestError('Invalid provider') describe 'with unconfigured provider on body', -> before -> - req = { method: 'GET', params: {}, body: { provider: 'password' } } + req = { method: 'GET', params: {}, connectParams: { provider: 'password' } } res = {} next = sinon.spy() settingsProviders = settings.providers @@ -110,8 +130,3 @@ describe 'Determine Provider', -> it 'should continue', -> next.should.have.been.called - - - - - diff --git a/test/unit/oidc/verifyEmailValid.coffee b/test/unit/oidc/verifyEmailValid.coffee new file mode 100644 index 00000000..f3a20ea5 --- /dev/null +++ b/test/unit/oidc/verifyEmailValid.coffee @@ -0,0 +1,92 @@ +chai = require 'chai' +sinon = require 'sinon' +sinonChai = require 'sinon-chai' +expect = chai.expect + + + + +chai.use sinonChai +should = chai.should() + +{selectConnectParams, verifyEmailValid} = require '../../../oidc' + + +describe 'Verify Email Valid', -> + + + {req,res,next,err} = {} + + describe 'with missing email', -> + + before (done) -> + req = { connectParams: {} } + err = 'foo' + verifyEmailValid req, res, (error) -> + err = error + done() + + it 'should call callback with no error', -> + should.not.exist(err) + + describe 'with missing connectParams', -> + + before (done) -> + req = {} + err = 'foo' + done() + + it 'should throw TypeError', -> + expect( -> + verifyEmailValid req, res, (error) -> + err = error + ).to.throw TypeError + + describe 'requires selectConnectParams to be called first', -> + before (done) -> + req = { method: 'GET' } + err = 'foo' + selectConnectParams req, res, (error) -> + verifyEmailValid req, res, (error2) -> + err = error2 + done() + + it 'should call callback with no error', -> + should.not.exist(err) + + + describe 'with email in query', -> + before (done) -> + req = + method: 'GET' + query: + email: "bar@example.com" + err = 'foo' + selectConnectParams req, res, (error) -> + verifyEmailValid req, res, (error2) -> + err = error2 + done() + + it 'should call callback with no error', -> + should.not.exist(err) + + it 'should leave connectParams email alone.', -> + req.connectParams.email.should.equal('bar@example.com') + + describe 'with bad email in body', -> + before (done) -> + req = + method: 'POST' + query: + email: "bar@salamander@example.com" + err = 'foo' + selectConnectParams req, res, (error) -> + verifyEmailValid req, res, (error2) -> + err = error2 + done() + + it 'should call callback with no error', -> + should.not.exist(err) + + it 'should delete connectParams email.', -> + should.not.exist(req.connectParams.email) diff --git a/views/passwordless/pwlessSentEmail.jade b/views/passwordless/pwlessSentEmail.jade new file mode 100644 index 00000000..a685fd34 --- /dev/null +++ b/views/passwordless/pwlessSentEmail.jade @@ -0,0 +1,34 @@ +doctype html +//- + Worded so that we do not give away whether user has an + account already. + +html(lang="en") + head + title E-mail sent + + link(rel='stylesheet', href='//maxcdn.bootstrapcdn.com/font-awesome/4.1.0/css/font-awesome.min.css') + link(rel='stylesheet', href='/stylesheets/app.css') + link(rel='stylesheet', href='//fonts.googleapis.com/css?family=Roboto:400,100,100italic,400italic,700,700italic|Raleway:400,100,600,300|Playfair+Display+SC:900,400') + body + + .anvilform + img.logo(src='/images/anvil.svg', alt='Anvil Connect') + + .panel + if error + .error= error + if message + .message= message + h3 Please check your email. + p + | We sent you a link. Please check your e-mail. + p + | At most, it should take several minutes to receive this e-mail. + p + | If you're having trouble receiving it, make sure that #{from} + | is in your contacts/whitelist. Otherwise the e-mail may be in the + | spam folder or discarded. + p Still having trouble? + div + a.button.full-width(href=resendURL) Resend e-mail. diff --git a/views/passwordless/pwlessSigninLinkVerified.jade b/views/passwordless/pwlessSigninLinkVerified.jade new file mode 100644 index 00000000..312c0970 --- /dev/null +++ b/views/passwordless/pwlessSigninLinkVerified.jade @@ -0,0 +1,21 @@ +doctype html +html(lang="en") + head + title Email link processed | Anvil Connect + + link(rel='stylesheet', href='//maxcdn.bootstrapcdn.com/font-awesome/4.1.0/css/font-awesome.min.css') + link(rel='stylesheet', href='/stylesheets/app.css') + link(rel='stylesheet', href='//fonts.googleapis.com/css?family=Roboto:400,100,100italic,400italic,700,700italic|Raleway:400,100,600,300|Playfair+Display+SC:900,400') + body + + .anvilform + img.logo(src='/images/anvil.svg', alt='Anvil Connect') + + .panel + if error + .error= error + else + div Your e-mail link has now been verified! + if signin + div You may now continue to sign in to #{signin.client.client_name}. + a.button(href=signin.url) Continue to sign in From 11f44bed794e3b8d5c172ee995da01f9b9035984 Mon Sep 17 00:00:00 2001 From: Henrich Kraemer Date: Mon, 19 Oct 2015 18:01:53 +0200 Subject: [PATCH 4/9] Added lib/testSettings.coffee to stub modules Other changes: 1. Changed enforceReferrer.js to read settings not during module initialization to allow changing settings during test. 2. Used new TestSettings in exising passwordless integration tests. --- oidc/enforceReferrer.js | 6 +- test/integration/routes/pwless_resend.coffee | 41 +++++------ test/integration/routes/pwless_signin.coffee | 75 +++++++++++--------- test/lib/testSettings.coffee | 38 ++++++++++ test/unit/oidc/sendVerificationEmail.coffee | 18 ++--- 5 files changed, 113 insertions(+), 65 deletions(-) create mode 100644 test/lib/testSettings.coffee diff --git a/oidc/enforceReferrer.js b/oidc/enforceReferrer.js index 53324855..11e7de9f 100644 --- a/oidc/enforceReferrer.js +++ b/oidc/enforceReferrer.js @@ -18,9 +18,11 @@ module.exports = function (pathname) { pathname = [ pathname ] } - var host = url.parse(settings.issuer).host - return function enforceReferrer (req, res, next) { + // allows changing settings for test by not reading them + // during module initialization. + + var host = url.parse(settings.issuer).host var referrer = req.get('referrer') // Only allow requests with a referrer defined diff --git a/test/integration/routes/pwless_resend.coffee b/test/integration/routes/pwless_resend.coffee index 526eaa82..6ffd6a6f 100644 --- a/test/integration/routes/pwless_resend.coffee +++ b/test/integration/routes/pwless_resend.coffee @@ -17,49 +17,47 @@ _ = require 'lodash' server = require '../../../server' Client = require('../../../models/Client') User = require('../../../models/User') -server = require '../../../server' +OneTimeToken = require '../../../models/OneTimeToken' -settings = require '../../../boot/settings' +TestSettings = require '../../lib/testSettings' request = supertest(server) describe 'Passwordless resend email route', -> + settings = require '../../../boot/settings' mailer = require '../../../boot/mailer' - mailer_state = {} - fakeMailer = + + tsSettings = new TestSettings(settings, + _.pick(settings, ['response_types_supported'])) + + tsSettings.addSettings + issuer: 'https://test.issuer.com' + providers: + passwordless: + "tokenTTL-foo": 600 + + tsMailer = new TestSettings(mailer, from: "from@example.com" render: {} sendMail: (tmpl, loc, opts, cb) -> cb() transport: {} - - before -> - _.assign(mailer_state, mailer) - _.assign(mailer, fakeMailer) + ) after -> - _.assign(mailer, mailer_state) + tsMailer.restore() + tsSettings.restore() {err, res} = {} - describe 'GET resend/passwordless', -> describe 'success flow', -> - settings_providers_state = {} - pwless_settings = - passwordless: - "tokenTTL-foo": 600 - - before (done) -> - _.assign(settings_providers_state, settings.providers) - _.assign(settings.providers, pwless_settings) - sinon.stub(Client, 'get').callsArgWith(2, null, { client_name: 'unit test pwless_resend' @@ -73,6 +71,9 @@ describe 'Passwordless resend email route', -> sinon.stub(User, 'getByEmail').callsArgWith(1, null, null) + sinon.stub(OneTimeToken, 'issue') + .callsArgWith(1, null, new OneTimeToken {}) + query = client_id: '4a2c1a31-150d-49e3-9946-2909220cdb16' redirect_uri: 'http://localhost:9000/callback_popup.html' @@ -89,7 +90,7 @@ describe 'Passwordless resend email route', -> done() after -> - settings.providers = settings_providers_state; + OneTimeToken.issue.restore() User.getByEmail.restore() Client.get.restore() diff --git a/test/integration/routes/pwless_signin.coffee b/test/integration/routes/pwless_signin.coffee index bbe57440..e4ac7a19 100644 --- a/test/integration/routes/pwless_signin.coffee +++ b/test/integration/routes/pwless_signin.coffee @@ -14,28 +14,39 @@ _ = require 'lodash' # Code under test server = require '../../../server' +Client = require('../../../models/Client') +User = require('../../../models/User') +OneTimeToken = require '../../../models/OneTimeToken' -settings = require '../../../boot/settings' +TestSettings = require '../../lib/testSettings' request = supertest(server) describe 'Passwordless signin post', -> + settings = require '../../../boot/settings' mailer = require '../../../boot/mailer' - mailer_state = {} - fakeMailer = + + tsSettings = new TestSettings(settings, + _.pick(settings, ['response_types_supported'])) + + tsSettings.addSettings + issuer: 'https://test.issuer.com' + providers: + passwordless: + "tokenTTL-foo": 600 + + tsMailer = new TestSettings(mailer, from: "from@example.com" render: {} sendMail: (tmpl, loc, opts, cb) -> cb() transport: {} - - before -> - _.assign(mailer_state, mailer) - _.assign(mailer, fakeMailer) + ) after -> - _.assign(mailer, mailer_state) + tsMailer.restore() + tsSettings.restore() {err, res} = {} @@ -44,29 +55,25 @@ describe 'Passwordless signin post', -> describe 'success flow', -> - settings_providers_state = {} - pwless_settings = - passwordless: - "tokenTTL-foo": 600 - before (done) -> - _.assign(settings_providers_state, settings.providers) - _.assign(settings.providers, pwless_settings) - - # # - # sinon.stub(Client, 'get').callsArgWith(2, null, - # { - # client_name: 'unit test pwless_signin' - # redirect_uris: [ - # 'http://localhost:9000/callback_popup.html' - # ] - # trusted: true - # response_types: ['id_token token'] - # grant_types: ['implicit'] - # }) - # - # sinon.stub(User, 'getByEmail').callsArgWith(1, null, null) + sinon.stub(Client, 'get').callsArgWith(2, null, + { + client_name: 'unit test pwless_signin' + redirect_uris: [ + 'http://localhost:9000/callback_popup.html' + ] + trusted: true + response_types: ['id_token token'] + grant_types: ['implicit'] + }) + + sinon.stub(User, 'getByEmail').callsArgWith(1, null, null) + + sinon.stub(OneTimeToken, 'issue') + .callsArgWith(1, null, new OneTimeToken {}) + # the exp, sub and use will be checked when verifying the link. + # However this is not needed in this test, only the generated token id. fields = client_id: '4a2c1a31-150d-49e3-9946-2909220cdb16' @@ -74,7 +81,7 @@ describe 'Passwordless signin post', -> response_type: 'id_token token' scope: 'openid profile' nonce: 'KG4vsD0bfAjbEdCMurmiPxzEcpFGoguYGR7b3cj3AMs' - email: 'foo@example.com' + email: 'user@test.com' provider: 'passwordless' request @@ -87,9 +94,9 @@ describe 'Passwordless signin post', -> done() after -> - settings.providers = settings_providers_state; - # User.getByEmail.restore() - # Client.get.restore() + OneTimeToken.issue.restore() + User.getByEmail.restore() + Client.get.restore() it 'should respond 200', -> res.statusCode.should.equal 200 @@ -101,4 +108,4 @@ describe 'Passwordless signin post', -> res.text.should.contain 'from@example.com' it 'should respond with html page containing the resend link', -> - res.text.should.match (new RegExp("href=\"http(s){0,1}://[^/]{0,}/resend/passwordless\\?email=foo%40example\\.com&", "g")) + res.text.should.match (new RegExp("href=\"https://test.issuer.com/resend/passwordless\\?email=user%40test\\.com&", "g")) diff --git a/test/lib/testSettings.coffee b/test/lib/testSettings.coffee new file mode 100644 index 00000000..1f1312cb --- /dev/null +++ b/test/lib/testSettings.coffee @@ -0,0 +1,38 @@ +_ = require 'lodash' + +clearObj = (dest) -> + for own key of dest + delete dest[key] + +copyObj = (dest, source) -> + clearObj(dest) + _.assign(dest, source) + # for own key, value of source + # dest[key] = value + +# Allows setting different properties during a test +class TestSettings + settingsState = {} + + constructor: (@settingsObject , source = {}, clear = true) -> + _.assign(settingsState, @settingsObject) + if (clear) + clearObj(@settingsObject) + @setSettings(source) + + restore: () -> + copyObj(@settingsObject, settingsState) + + # not yet needed. + # getSettings: -> + # return @settingsObject + # + + setSettings: (sourceObject)-> + copyObj(@settingsObject, sourceObject) + + addSettings: (sourceObject)-> + _.assign(@settingsObject, sourceObject) + + +module.exports = TestSettings diff --git a/test/unit/oidc/sendVerificationEmail.coffee b/test/unit/oidc/sendVerificationEmail.coffee index 05e053ae..edd0ba20 100644 --- a/test/unit/oidc/sendVerificationEmail.coffee +++ b/test/unit/oidc/sendVerificationEmail.coffee @@ -6,24 +6,24 @@ expect = chai.expect chai.use sinonChai chai.should() -_ = require 'lodash' - -mailer = require '../../../boot/mailer' - -mailer_state = {} {sendVerificationEmail} = require '../../../oidc' OneTimeToken = require '../../../models/OneTimeToken' +TestSettings = require '../../lib/testSettings' describe 'Send Verification Email', -> + mailer = require '../../../boot/mailer' + tsMailer = {} + before -> - _.assign(mailer_state, mailer) - mailer.sendMail = (tmpl, loc, opts, cb) -> - cb() + tsMailer = new TestSettings(mailer, + sendMail : (tmpl, loc, opts, cb) -> + cb() + ) after -> - _.assign(mailer, mailer_state) + tsMailer.restore() {req,res,next} = {} From e3744344a7c119968be79dd06e41371d59a120d6 Mon Sep 17 00:00:00 2001 From: Henrich Kraemer Date: Tue, 29 Sep 2015 17:06:01 +0200 Subject: [PATCH 5/9] Completing passwordless code and test test and changes for passwordless /signin of new and existing users test and changes for /signup/passwordless add unit tests passwordless and fixes needed to make them pass The following unit tests were added: - test to assert some default settings The assertions made in this test can be assumed to be true in other code. For example other code can assume that settings.providers is never undefined. The following middleware unit tests were added: - verify passwordless enabled - consume token - extract token - verify token - render invalid email - verify signup token - verify new user signin token add test and fix issues for /signup/passwordless get and post route --- routes/passwordless.js | 223 +++++++++++--- test/integration/routes/pwless_signin.coffee | 177 +++++++++-- .../routes/pwless_signin_link.coffee | 168 +++++++++++ .../routes/pwless_signup_get.coffee | 113 +++++++ .../routes/pwless_signup_post.coffee | 180 +++++++++++ .../unit/oidc/passwordlessConsumeToken.coffee | 142 +++++++++ .../oidc/passwordlessExtractTokenSub.coffee | 230 ++++++++++++++ test/unit/oidc/passwordlessPeekToken.coffee | 147 +++++++++ .../passwordlessRenderInvalidEmail.coffee | 91 ++++++ .../oidc/passwordlessVerifyEnablement.coffee | 67 +++++ ...asswordlessVerifyNewUserSigninToken.coffee | 207 +++++++++++++ .../oidc/passwordlessVerifySignupToken.coffee | 186 ++++++++++++ test/unit/oidc/passwordlessVerifyToken.coffee | 284 ++++++++++++++++++ ...rified.jade => pwlessSigninLinkError.jade} | 4 +- views/passwordless/pwlessSignup.jade | 23 ++ 15 files changed, 2187 insertions(+), 55 deletions(-) create mode 100644 test/integration/routes/pwless_signin_link.coffee create mode 100644 test/integration/routes/pwless_signup_get.coffee create mode 100644 test/integration/routes/pwless_signup_post.coffee create mode 100644 test/unit/oidc/passwordlessConsumeToken.coffee create mode 100644 test/unit/oidc/passwordlessExtractTokenSub.coffee create mode 100644 test/unit/oidc/passwordlessPeekToken.coffee create mode 100644 test/unit/oidc/passwordlessRenderInvalidEmail.coffee create mode 100644 test/unit/oidc/passwordlessVerifyEnablement.coffee create mode 100644 test/unit/oidc/passwordlessVerifyNewUserSigninToken.coffee create mode 100644 test/unit/oidc/passwordlessVerifySignupToken.coffee create mode 100644 test/unit/oidc/passwordlessVerifyToken.coffee rename views/passwordless/{pwlessSigninLinkVerified.jade => pwlessSigninLinkError.jade} (79%) create mode 100644 views/passwordless/pwlessSignup.jade diff --git a/routes/passwordless.js b/routes/passwordless.js index 4bbef1e0..81da9d0f 100644 --- a/routes/passwordless.js +++ b/routes/passwordless.js @@ -3,6 +3,7 @@ */ var express = require('express') +var _ = require('lodash') var url = require('url') var settings = require('../boot/settings') var mailer = require('../boot/mailer') @@ -49,6 +50,11 @@ var PasswordlessDisabledError = require('../errors/PasswordlessDisabledError') var TOKEN_USAGE_SIGNIN = 'pwless-signin' var TOKEN_USAGE_SIGNUP = 'pwless-signup' +var TOKEN_USAGE_SIGNIN_NEW_USER = 'pwless-new-user' + +var CONNECT_SUB_FIELDS = [ + 'email', 'redirect_uri', 'client_id', 'response_type', 'scope', 'nonce' +] function verifyPasswordlessEnabled (req, res, next) { if (!settings.providers.passwordless) { @@ -58,50 +64,153 @@ function verifyPasswordlessEnabled (req, res, next) { } } +function consumeToken (req, res, next) { + if (!req.query.token) { + return next(new InvalidRequestError('Missing token')) + } + // consume the token + OneTimeToken.consume(req.query.token, function (err, token) { + if (err) { return next(err) } + req.token = token + next() + }) +} + +function extractTokenSub (req, res, next) { + if (!req.token) { + return next() + } + var token = req.token + if (!token.sub) { + return next() + } + req.connectParams = req.connectParams || {} + _.assign(req.connectParams, _.pick(token.sub, CONNECT_SUB_FIELDS)) + if (token.sub) { + req.user_id = token.sub.user + } + next() +} + // perhaps this should be dispached by the authenticator // however I am not sure I see what that would bring? function verifyPasswordlessSigninToken (req, res, next) { - if (!req.query.token) { - return next(new InvalidRequestError('Missing token')) + var view = 'passwordless/pwlessSigninLinkError' + var token = req.token + + // Invalid or expired token + if (!token || token.use !== TOKEN_USAGE_SIGNIN) { + return res.render(view, { + error: 'Invalid or expired link' + }) } - var view = 'passwordless/pwlessSigninLinkVerified' + if (!req.user_id || !req.user_id.trim()) { + return res.render(view, { + error: 'Invalid or expired link' + }) + } - // consume the token - OneTimeToken.consume(req.query.token, function (err, token) { + // Update the user + User.patch(req.user_id, { + dateEmailVerified: Date.now(), + emailVerified: true + }, function (err, user) { if (err) { return next(err) } - // Invalid or expired token - if (!token || token.use !== TOKEN_USAGE_SIGNIN) { + // unknown user, might happen if token expired or + // link activated twice. + if (!user) { return res.render(view, { - error: 'Invalid or expired link' + error: 'Unable to verify email for this user.' }) } - // Update the user - User.patch(token.sub, { - dateEmailVerified: Date.now(), - emailVerified: true - }, function (err, user) { - if (err) { return next(err) } + // analog to Password signin handler after authenticator.dispatch + // authenticated user based on password. + req.user = user + authenticator.login(req, user) + next() + }) +} - // unknown user, might happen if token expired or - // link activated twice. - if (!user) { - return res.render(view, { - error: 'Unable to verify email for this user.' - }) - } +function verifyPasswordlessSignupToken (req, res, next) { + var errView = 'passwordless/pwlessSigninLinkError' + var token = req.token - // analog to Password signin handler after authenticator.dispatch - // authenticated user based on password. - req.user = user - authenticator.login(req, user) - next() + // Invalid or expired token + if (!token || token.use !== TOKEN_USAGE_SIGNUP) { + return res.render(errView, { + error: 'Invalid or expired link' + }) + } + + // We have a token and we now send the user to form to fill in + // additional information to create the user account. + // When that form is submitted the user should be created and + // then immediately signed in. + // We issue a new token with a different expiration so that the user can fill in the form with more leisure. + // The token also captures request parameters. + var tokenOptions = { + use: TOKEN_USAGE_SIGNIN_NEW_USER, + ttl: 60 * 60 * 24, + sub: token.sub + } + issueToken(req, tokenOptions, function (err, token, tokenOptions) { + if (err) { + return next(err) + } + res.render('passwordless/pwlessSignup', { + 'email': tokenOptions.sub.email, + token: token._id }) }) } +function peekToken (req, res, next) { + if (!req.connectParams.token) { + return next(new InvalidRequestError('Missing token')) + } + // consume the token + OneTimeToken.peek(req.connectParams.token, function (err, token) { + if (err) { return next(err) } + req.token = token + next() + }) +} + +function verifyPasswordlessNewUserSigninToken (req, res, next) { + var view = 'passwordless/pwlessSignup' + var token = req.token + + // Invalid or expired token + if (!token || token.use !== TOKEN_USAGE_SIGNIN_NEW_USER) { + return res.render(view, { + error: 'Invalid or expired link' + }) + } + + var userOptions = { + private: true, + password: false + } + + User.insert(req.connectParams, userOptions, function (err, user) { + if (err) { + return res.render(view, { + error: err, + token: token._id, + email: token.sub.email + }) + } + OneTimeToken.revoke(token._id) + // don't care if token revocation fails as they expire anyhow. + req.user = user + authenticator.login(req, user) + next() + }) +} + function signinRenderErrorInvalidEmail (req, res, next) { if (typeof req.connectParams.email === 'undefined') { return res.render('signin', { @@ -111,6 +220,18 @@ function signinRenderErrorInvalidEmail (req, res, next) { next() } +function issueToken (req, tokenOptions, cb) { + var subObject = _.pick(req.connectParams, CONNECT_SUB_FIELDS) + var theTokenOptions = { + ttl: settings.providers.passwordless.tokenTTL || 60 * 15, + sub: subObject + } + theTokenOptions = _.merge(theTokenOptions, tokenOptions) + OneTimeToken.issue(theTokenOptions, function (err, token) { + cb(err, token, theTokenOptions) + }) +} + function sendMail (req, res, next) { User.getByEmail(req.connectParams.email, function (err, user) { if (err) { return next(err) } @@ -123,15 +244,16 @@ function sendMail (req, res, next) { // The email send is tailored to whether it is a new sign up or // a sign in of an existing user - var email = req.connectParams.email - var tokenOptions = { - ttl: settings.providers.passwordless.tokenTTL || 60 * 15, use: user ? TOKEN_USAGE_SIGNIN : TOKEN_USAGE_SIGNUP, - sub: user ? user._id : email + sub: {} + } + if (user) { + tokenOptions.sub.user = user._id } - OneTimeToken.issue(tokenOptions, function (err, token) { + issueToken(req, tokenOptions, function (err, token, tokenOptions) { if (err) { return next(err) } + var email = tokenOptions.sub.email var verifyURL = url.parse(settings.issuer) verifyURL.pathname = user ? 'signin/pwless' : 'signup/pwless' @@ -232,8 +354,9 @@ function routes (server) { }, sendMail ) - server.get('/signin/pwless', - oidc.selectConnectParams, + server.get('/signin/:provider', + consumeToken, + extractTokenSub, oidc.verifyClient, oidc.validateAuthorizationParams, oidc.determineProvider.setup({requireProvider: true}), @@ -243,10 +366,42 @@ function routes (server) { oidc.promptToAuthorize, oidc.authorize ) + server.get('/signup/:provider', + consumeToken, + extractTokenSub, + oidc.verifyClient, + oidc.validateAuthorizationParams, + oidc.determineProvider.setup({requireProvider: true}), + verifyPasswordlessEnabled, + verifyPasswordlessSignupToken + ) + server.post('/signup/:provider', + oidc.selectConnectParams, + peekToken, + extractTokenSub, + oidc.verifyClient, + oidc.validateAuthorizationParams, + oidc.determineProvider.setup({requireProvider: true}), + verifyPasswordlessEnabled, + verifyPasswordlessNewUserSigninToken, + oidc.determineUserScope, + oidc.promptToAuthorize, + oidc.authorize + ) } module.exports = { routes: routes, signin: postSigninMiddleware, - verifyEnabled: verifyPasswordlessEnabled + middleware: { + verifyEnabled: verifyPasswordlessEnabled, + peekToken: peekToken, + consumeToken: consumeToken, + extractTokenSub: extractTokenSub, + verifyToken: verifyPasswordlessSigninToken, + renderInvalidEmail: signinRenderErrorInvalidEmail, + sendMail: sendMail, + verifySignupToken: verifyPasswordlessSignupToken, + verifyNewUserSigninToken: verifyPasswordlessNewUserSigninToken + } } diff --git a/test/integration/routes/pwless_signin.coffee b/test/integration/routes/pwless_signin.coffee index e4ac7a19..9697be33 100644 --- a/test/integration/routes/pwless_signin.coffee +++ b/test/integration/routes/pwless_signin.coffee @@ -27,33 +27,37 @@ describe 'Passwordless signin post', -> settings = require '../../../boot/settings' mailer = require '../../../boot/mailer' - tsSettings = new TestSettings(settings, - _.pick(settings, ['response_types_supported'])) - - tsSettings.addSettings - issuer: 'https://test.issuer.com' - providers: - passwordless: - "tokenTTL-foo": 600 - - tsMailer = new TestSettings(mailer, - from: "from@example.com" - render: {} - sendMail: (tmpl, loc, opts, cb) -> - cb() - transport: {} - ) + tsSettings = {} + tsMailer = {} + + before -> + tsSettings = new TestSettings(settings, + _.pick(settings, ['response_types_supported'])) + + tsSettings.addSettings + issuer: 'https://test.issuer.com' + providers: + passwordless: + "tokenTTL-foo": 600 + + tsMailer = new TestSettings(mailer, + from: "from@example.com" + render: {} + sendMail: (tmpl, loc, opts, cb) -> + cb() + transport: {} + ) after -> tsMailer.restore() tsSettings.restore() - {err, res} = {} + {fields, err, res} = {} describe 'POST signin body passwordless', -> - describe 'success flow', -> + describe 'success flow new user', -> before (done) -> @@ -75,6 +79,8 @@ describe 'Passwordless signin post', -> # the exp, sub and use will be checked when verifying the link. # However this is not needed in this test, only the generated token id. + sinon.stub(mailer, 'sendMail').callsArgWith 3, null, null + fields = client_id: '4a2c1a31-150d-49e3-9946-2909220cdb16' redirect_uri: 'http://localhost:9000/callback_popup.html' @@ -94,10 +100,45 @@ describe 'Passwordless signin post', -> done() after -> + mailer.sendMail.restore() OneTimeToken.issue.restore() User.getByEmail.restore() Client.get.restore() + it 'should issue an expiring token', -> + OneTimeToken.issue.should.have.been.calledWith sinon.match({ + ttl: sinon.match.number + }) + + it 'should issue a token for passwordless sign-UP', -> + OneTimeToken.issue.should.have.been.calledWith sinon.match({ + use: 'pwless-signup' + }) + + it 'should send email to the user', -> + mailer.sendMail.should.have.been + .calledWith 'passwordlessSignup', sinon.match.object, sinon.match({ + to: fields.email + }) + + it 'should provide a subject', -> + mailer.sendMail.should.have.been + .calledWith 'passwordlessSignup', sinon.match.object, sinon.match({ + subject: sinon.match.string + }) + + it 'should render with the user email', -> + mailer.sendMail.should.have.been + .calledWith 'passwordlessSignup', sinon.match({ + email: fields.email + }) + + it 'should render with the verification url', -> + mailer.sendMail.should.have.been + .calledWith 'passwordlessSignup', sinon.match({ + verifyURL: sinon.match.string + }) + it 'should respond 200', -> res.statusCode.should.equal 200 @@ -109,3 +150,103 @@ describe 'Passwordless signin post', -> it 'should respond with html page containing the resend link', -> res.text.should.match (new RegExp("href=\"https://test.issuer.com/resend/passwordless\\?email=user%40test\\.com&", "g")) + + describe 'success flow existing user', -> + + before (done) -> + + sinon.stub(Client, 'get').callsArgWith(2, null, + { + client_name: 'unit test pwless_signin' + redirect_uris: [ + 'http://localhost:9000/callback_popup.html' + ] + trusted: true + response_types: ['id_token token'] + grant_types: ['implicit'] + }) + + sinon.stub(User, 'getByEmail').callsArgWith(1, null, + new User + email: 'doe@test.com' + _id: 'uuid-of-doe' + givenName: 'john' + familyName: 'doe-family' + ) + + sinon.stub(OneTimeToken, 'issue') + .callsArgWith(1, null, new OneTimeToken {}) + # the exp, sub and use will be checked when verifying the link. + # However this is not needed in this test, only the generated token id. + + sinon.stub(mailer, 'sendMail').callsArgWith 3, null, null + + fields = + client_id: '4a2c1a31-150d-49e3-9946-2909220cdb16' + redirect_uri: 'http://localhost:9000/callback_popup.html' + response_type: 'id_token token' + scope: 'openid profile' + nonce: 'KG4vsD0bfAjbEdCMurmiPxzEcpFGoguYGR7b3cj3AMs' + email: 'doe@test.com' + provider: 'passwordless' + + request + .post('/signin') + .set('referer', settings.issuer + '/signin') + .send(fields) + .end (error, response) -> + err = error + res = response + done() + + after -> + mailer.sendMail.restore() + OneTimeToken.issue.restore() + User.getByEmail.restore() + Client.get.restore() + + it 'should issue an expiring token', -> + OneTimeToken.issue.should.have.been.calledWith sinon.match({ + ttl: sinon.match.number + }) + + it 'should issue a token for passwordless sign-IN', -> + OneTimeToken.issue.should.have.been.calledWith sinon.match({ + use: 'pwless-signin' + }) + + it 'should send email to the user', -> + mailer.sendMail.should.have.been + .calledWith 'passwordlessSignin', sinon.match.object, sinon.match({ + to: fields.email + }) + + it 'should provide a subject', -> + mailer.sendMail.should.have.been + .calledWith 'passwordlessSignin', sinon.match.object, sinon.match({ + subject: sinon.match.string + }) + + it 'should render with the user email', -> + mailer.sendMail.should.have.been + .calledWith 'passwordlessSignin', sinon.match({ + email: fields.email + }) + + it 'should render with the verification url', -> + mailer.sendMail.should.have.been + .calledWith 'passwordlessSignin', sinon.match({ + verifyURL: sinon.match.string + }) + + it 'should respond 200', -> + res.statusCode.should.equal 200 + + it 'should respond with an html page', -> + res.headers['content-type'].should.contain 'text/html' + + it 'should respond with html page containing the sender', -> + res.text.should.contain 'from@example.com' + + it 'should respond with html page containing the resend link', -> + res.text.should.match (new RegExp("href=\"https://test.issuer.com/resend/passwordless\\?email=doe%40test\\.com&", "g")) diff --git a/test/integration/routes/pwless_signin_link.coffee b/test/integration/routes/pwless_signin_link.coffee new file mode 100644 index 00000000..c6f43dc4 --- /dev/null +++ b/test/integration/routes/pwless_signin_link.coffee @@ -0,0 +1,168 @@ +# Test dependencies +chai = require 'chai' +sinon = require 'sinon' +sinonChai = require 'sinon-chai' +supertest = require 'supertest' +expect = chai.expect + + +# Assertions +chai.use sinonChai +chai.should() + + +_ = require 'lodash' + +# Code under test +express = require 'express' +server = require '../../../server' +Client = require('../../../models/Client') +User = require('../../../models/User') +OneTimeToken = require '../../../models/OneTimeToken' +Scope = require('../../../models/Scope') +IDToken = require '../../../models/IDToken' +AccessToken = require '../../../models/AccessToken' + +TestSettings = require '../../lib/testSettings' + +request = supertest(server) + +describe 'Passwordless signin link activation', -> + + settings = require '../../../boot/settings' + + tsSettings = {} + + before -> + tsSettings = new TestSettings(settings, + _.pick(settings, ['response_types_supported', 'keys'])) + + tsSettings.addSettings + issuer: 'https://test.issuer.com' + providers: + passwordless: + "tokenTTL-foo": 600 + + after -> + tsSettings.restore() + + {err, res} = {} + + + describe 'success flow existing user sign-IN', -> + + session = {} + + tokenOptions = + exp: Math.round(Date.now() / 1000) + 3600 + use: 'pwless-signin' + sub: + email: 'peter@mary.com' + client_id: '4a2c1a31-150d-49e3-9946-2909220cdb16' + redirect_uri: 'http://localhost:9000/callback_popup.html' + response_type: 'id_token token' + scope: 'openid profile' + nonce: 'KG4vsD0bfAjbEdCMurmiPxzEcpFGoguYGR7b3cj3AMs' + user: 'peters-user-uuid' + + client = + client_name: 'unit test pwless_signin' + _id: '4a2c1a31-150d-49e3-9946-2909220cdb16' + redirect_uris: [ + 'http://localhost:9000/callback_popup.html' + ] + trusted: true + response_types: ['id_token token'] + grant_types: ['implicit'] + + {scope,scopes} = {} + + before (done) -> + + # If redis is not running req.session remains undefined + # causing an error in oidc/setSessionAmr.js which ultimately + # result in a internal server error by oidc/error.js + http.IncomingMessage.prototype.session = session + + sinon.stub(OneTimeToken, 'consume') + .callsArgWith(1, null, new OneTimeToken tokenOptions) + + sinon.stub(Client, 'get').callsArgWith(2, null, client) + + user = new User _id: tokenOptions.sub.user + + sinon.stub(User, 'patch').callsArgWith(2, null, user) + + scope = 'openid profile developer' + scopes = [ + new Scope name: 'openid' + new Scope name: 'profile' + new Scope name: 'developer' + ] + sinon.stub(Scope, 'determine').callsArgWith(2, null, scope, scopes) + + response = AccessToken.initialize().project('issue') + sinon.stub(AccessToken, 'issue').callsArgWith(1, null, response) + sinon.spy(IDToken.prototype, 'initializePayload') + + sinon.spy(express.response, 'redirect') + + query = + token: 'token-id-random-stuff' + + request + .get('/signin/passwordless') + .redirects(0) + .query(query) + .end (error, response) -> + err = error + res = response + done() + console.log('request next line') + # done() + + after -> + express.response.redirect.restore(); + AccessToken.issue.restore() + IDToken.prototype.initializePayload.restore() + Scope.determine.restore() + User.patch.restore() + OneTimeToken.consume.restore() + Client.get.restore() + delete http.IncomingMessage.prototype.session + + it 'should consume token with the id taken of the link query', -> + OneTimeToken.consume.should.have.been.calledWith 'token-id-random-stuff' + + it 'should respond with an http redirect', -> + res.statusCode.should.equal 302 + + it 'should redirect to the callback', -> + express.response.redirect.should.have.been.calledWith sinon.match( client.redirect_uris[0] ) + + it 'should provide a uri fragment', -> + express.response.redirect.should.have.been.calledWith sinon.match('#') + + it 'should provide access_token', -> + express.response.redirect.should.have.been.calledWith sinon.match('access_token=') + + it 'should provide token_type', -> + express.response.redirect.should.have.been.calledWith sinon.match('token_type=Bearer') + + it 'should provide expires_in', -> + express.response.redirect.should.have.been.calledWith sinon.match('expires_in=3600') + + it 'should provide id_token', -> + express.response.redirect.should.have.been.calledWith sinon.match('id_token=') + + # TODO: should there be a state here? + # it 'should provide state', -> + # express.response.redirect.should.have.been.calledWith # sinon.match req.connectParams.state + + it 'should provide session_state', -> + express.response.redirect.should.have.been.calledWith sinon.match('session_state=') + + it 'should include `amr` claim in id_token', -> + IDToken.prototype.initializePayload.should.have.been.calledWith( + sinon.match amr: session.amr + ) diff --git a/test/integration/routes/pwless_signup_get.coffee b/test/integration/routes/pwless_signup_get.coffee new file mode 100644 index 00000000..4065efa5 --- /dev/null +++ b/test/integration/routes/pwless_signup_get.coffee @@ -0,0 +1,113 @@ +# Test dependencies +chai = require 'chai' +sinon = require 'sinon' +sinonChai = require 'sinon-chai' +supertest = require 'supertest' +expect = chai.expect +qs = require 'qs' + + +# Assertions +chai.use sinonChai +chai.should() + +_ = require 'lodash' + +# Code under test +server = require '../../../server' +Client = require('../../../models/Client') +User = require('../../../models/User') +OneTimeToken = require '../../../models/OneTimeToken' + +TestSettings = require '../../lib/testSettings' + +request = supertest(server) + +describe 'Passwordless signup get route', -> + + settings = require '../../../boot/settings' + + tsSettings = new TestSettings(settings, + _.pick(settings, ['response_types_supported'])) + + tsSettings.addSettings + issuer: 'https://test.issuer.com' + providers: + passwordless: {} + + + after -> + tsSettings.restore() + + {err, res} = {} + + describe 'GET signup/passwordless', -> + + describe 'success flow', -> + + sub = + email: 'test@test.org' + redirect_uri: 'https://test.org/callback' + client_id: 'test-client-id' + response_type: 'token id_token' + scope: 'openid profile' + nonce: 'test-nonce' + unexpected: 'test-unexpected' + + signupToken = new OneTimeToken { + use: 'pwless-signup' + sub: sub + } + + newUserToken = new OneTimeToken { + _id: 'id-new-use-token' + use: 'pwless-new-user' + sub: sub + } + + + before (done) -> + + sinon.stub(OneTimeToken, 'consume') + .callsArgWith(1, null, signupToken) + + sinon.stub(Client, 'get').callsArgWith(2, null, + { + client_name: 'unit test pwless_resend' + redirect_uris: [ + 'https://test.org/callback' + ] + trusted: true + response_types: ['id_token token'] + grant_types: ['implicit'] + }) + + sinon.stub(OneTimeToken, 'issue') + .callsArgWith(1, null, newUserToken) + + query = + token: 'token-id' + + request + .get('/signup/passwordless?' + qs.stringify(query)) + .end (error, response) -> + err = error + res = response + done() + + after -> + OneTimeToken.issue.restore() + Client.get.restore() + OneTimeToken.consume.restore() + + it 'should respond 200', -> + res.statusCode.should.equal 200 + + it 'should respond with an html page', -> + res.headers['content-type'].should.contain 'text/html' + + it 'should respond with html page containing the sender', -> + res.text.should.contain 'test@test.org' + + it 'should respond with html page containing the token', -> + res.text.should.contain 'id-new-use-token' diff --git a/test/integration/routes/pwless_signup_post.coffee b/test/integration/routes/pwless_signup_post.coffee new file mode 100644 index 00000000..fde0b4d6 --- /dev/null +++ b/test/integration/routes/pwless_signup_post.coffee @@ -0,0 +1,180 @@ +# Test dependencies +chai = require 'chai' +sinon = require 'sinon' +sinonChai = require 'sinon-chai' +supertest = require 'supertest' +expect = chai.expect + + +# Assertions +chai.use sinonChai +chai.should() + + +_ = require 'lodash' + +# Code under test +express = require 'express' +server = require '../../../server' +Client = require('../../../models/Client') +User = require('../../../models/User') +OneTimeToken = require '../../../models/OneTimeToken' +Scope = require('../../../models/Scope') +IDToken = require '../../../models/IDToken' +AccessToken = require '../../../models/AccessToken' + +http = require 'http' + +TestSettings = require '../../lib/testSettings' + +request = supertest(server) + +describe 'Passwordless signup form submission', -> + + settings = require '../../../boot/settings' + + tsSettings = {} + + before -> + tsSettings = new TestSettings(settings, + _.pick(settings, ['response_types_supported', 'keys'])) + + tsSettings.addSettings + issuer: 'https://test.issuer.com' + providers: + passwordless: {} + + after -> + tsSettings.restore() + + {err, res} = {} + + + describe 'successful form submission creating user and sign-in', -> + + session = {} + + tokenOptions = + _id: 'token-id-random-stuff' + exp: Math.round(Date.now() / 1000) + 3600 + use: 'pwless-new-user' + sub: + email: 'peter@mary.com' + client_id: '4a2c1a31-150d-49e3-9946-2909220cdb16' + redirect_uri: 'http://localhost:9000/callback_popup.html' + response_type: 'id_token token' + scope: 'openid profile' + nonce: 'KG4vsD0bfAjbEdCMurmiPxzEcpFGoguYGR7b3cj3AMs' + + client = + client_name: 'unit test pwless' + _id: '4a2c1a31-150d-49e3-9946-2909220cdb16' + redirect_uris: [ + 'http://localhost:9000/callback_popup.html' + ] + trusted: true + response_types: ['id_token token'] + grant_types: ['implicit'] + + body = + token: tokenOptions._id + givenName: 'Peter' + familyName: 'Mary' + + {scope,scopes} = {} + + before (done) -> + + # If redis is not running req.session remains undefined + # causing an error in oidc/setSessionAmr.js which ultimately + # result in a internal server error by oidc/error.js + http.IncomingMessage.prototype.session = session + + sinon.stub(OneTimeToken, 'peek') + .callsArgWith(1, null, new OneTimeToken tokenOptions) + + sinon.stub(Client, 'get').callsArgWith(2, null, client) + + user = new User body + + sinon.stub(User, 'insert').callsArgWith(2, null, user) + + sinon.spy(OneTimeToken, 'revoke') + + scope = 'openid profile developer' + scopes = [ + new Scope name: 'openid' + new Scope name: 'profile' + new Scope name: 'developer' + ] + sinon.stub(Scope, 'determine').callsArgWith(2, null, scope, scopes) + + response = AccessToken.initialize().project('issue') + sinon.stub(AccessToken, 'issue').callsArgWith(1, null, response) + sinon.spy(IDToken.prototype, 'initializePayload') + sinon.spy(express.response, 'redirect') + + request + .post('/signup/passwordless') + .redirects(0) + .send(body) + .end (error, response) -> + err = error + res = response + done() + + after -> + express.response.redirect.restore(); + AccessToken.issue.restore() + IDToken.prototype.initializePayload.restore() + Scope.determine.restore() + OneTimeToken.revoke.restore() + User.insert.restore() + Client.get.restore() + OneTimeToken.peek.restore() + delete http.IncomingMessage.prototype.session + + it 'should peek token with the id taken of the form submission body token id', -> + OneTimeToken.peek.should.have.been.calledWith 'token-id-random-stuff' + + it 'should revoke token', -> + OneTimeToken.revoke.should.have.been.calledWith 'token-id-random-stuff' + + it 'User insert should have been called with form submission body', -> + User.insert.should.have.been.calledWith sinon.match { + givenName: 'Peter' + familyName: 'Mary' + } + + it 'should respond with an http redirect', -> + res.statusCode.should.equal 302 + + it 'should redirect to the callback', -> + express.response.redirect.should.have.been.calledWith sinon.match( client.redirect_uris[0] ) + + it 'should provide a uri fragment', -> + express.response.redirect.should.have.been.calledWith sinon.match('#') + + it 'should provide access_token', -> + express.response.redirect.should.have.been.calledWith sinon.match('access_token=') + + it 'should provide token_type', -> + express.response.redirect.should.have.been.calledWith sinon.match('token_type=Bearer') + + it 'should provide expires_in', -> + express.response.redirect.should.have.been.calledWith sinon.match('expires_in=3600') + + it 'should provide id_token', -> + express.response.redirect.should.have.been.calledWith sinon.match('id_token=') + + # TODO: should there be a state here? + # it 'should provide state', -> + # express.response.redirect.should.have.been.calledWith # sinon.match req.connectParams.state + + it 'should provide session_state', -> + express.response.redirect.should.have.been.calledWith sinon.match('session_state=') + + it 'should include `amr` claim in id_token', -> + IDToken.prototype.initializePayload.should.have.been.calledWith( + sinon.match amr: session.amr + ) diff --git a/test/unit/oidc/passwordlessConsumeToken.coffee b/test/unit/oidc/passwordlessConsumeToken.coffee new file mode 100644 index 00000000..3a1db36a --- /dev/null +++ b/test/unit/oidc/passwordlessConsumeToken.coffee @@ -0,0 +1,142 @@ +chai = require 'chai' +sinon = require 'sinon' +sinonChai = require 'sinon-chai' +expect = chai.expect + + +chai.use sinonChai +chai.should() + + +passwordless = require '../../../routes/passwordless' +passwordless = passwordless.middleware + +# InvalidRequestError = require '../../../errors/InvalidRequestError' + +settings = require '../../../boot/settings' +TestSettings = require '../../lib/testSettings' + +InvalidRequestError = require '../../../errors/InvalidRequestError' +OneTimeToken = require '../../../models/OneTimeToken' + + +describe 'Passwordless middleware tests', -> + + + {req,res,next,err, issuer} = {} + + tsSettings = {} + + + describe 'consume link token', -> + + before -> + tsSettings = new TestSettings(settings) + tsSettings.addSettings + issuer: 'https://test.issuer.com' + providers: + passwordless: {} + + after -> + tsSettings.restore() + + describe 'missing token is an error', -> + before -> + req = + query: {} + + next = sinon.spy() + + passwordless.consumeToken req, res, next + + it 'should continue', -> + next.should.have.been.called + + it 'should provide an invalid request error', -> + next.should.have.been.calledWith sinon.match.instanceOf(InvalidRequestError) + + describe 'valid token after stub OneTimeToken#consume call', -> + {err} = {} + {token} = {} + + before (done) -> + req = + query: + token: 'the-passwordless-token' + + next = sinon.spy (error) -> + err = error + done() + + token = {test: 'foo'} + sinon.stub(OneTimeToken, 'consume').callsArgWith(1, null, token) + + passwordless.consumeToken req, res, next + + after -> + OneTimeToken.consume.restore() + + it 'should continue', -> + next.should.have.been.called + + it 'should not provide an error', -> + expect(err).to.be.undefined + + it 'should set token on the request', -> + req.token.should.equal token + + describe 'token not found case in OneTimeToken#consume not found case results in next with no errors and req.token = null', -> + {err} = {} + + before (done) -> + req = + query: + token: 'the-passwordless-token' + + next = sinon.spy (error) -> + err = error + done() + + sinon.stub(OneTimeToken, 'consume').callsArgWith(1, null, null) + + passwordless.consumeToken req, res, next + + after -> + OneTimeToken.consume.restore() + + it 'should continue', -> + next.should.have.been.called + + it 'should not provide an error', -> + expect(err).to.be.undefined + + it 'should have null token on the request', -> + expect(req.token).to.be.null + + describe 'OneTimeToken#consume returns err reports error in next(err) call', -> + {err} = {} + + before (done) -> + req = + query: + token: 'the-passwordless-token' + + next = sinon.spy (error) -> + err = error + done() + + sinon.stub(OneTimeToken, 'consume').callsArgWith(1, new Error("fake A redis DB unit test error"), null) + + passwordless.consumeToken req, res, next + + after -> + OneTimeToken.consume.restore() + + it 'should continue', -> + next.should.have.been.called + + it 'should provide an error', -> + next.should.have.been.calledWith sinon.match.instanceOf(Error) + + it 'should have undefined token on the request', -> + expect(req.token).to.be.undefined diff --git a/test/unit/oidc/passwordlessExtractTokenSub.coffee b/test/unit/oidc/passwordlessExtractTokenSub.coffee new file mode 100644 index 00000000..48fddf89 --- /dev/null +++ b/test/unit/oidc/passwordlessExtractTokenSub.coffee @@ -0,0 +1,230 @@ +chai = require 'chai' +sinon = require 'sinon' +sinonChai = require 'sinon-chai' +expect = chai.expect + + +chai.use sinonChai +chai.should() + + +passwordless = require '../../../routes/passwordless' +passwordless = passwordless.middleware + +settings = require '../../../boot/settings' +TestSettings = require '../../lib/testSettings' + +describe 'Passwordless middleware tests', -> + + + {req,res,next,err, issuer} = {} + + tsSettings = {} + + describe 'extract sub info from token', -> + + before -> + tsSettings = new TestSettings(settings) + tsSettings.addSettings + issuer: 'https://test.issuer.com' + providers: + passwordless: {} + + after -> + tsSettings.restore() + + describe 'req.token is null', -> + before (done) -> + req = + token: null + + next = sinon.spy (error) -> + err = error + done() + + passwordless.extractTokenSub req, res, next + + it 'should continue', -> + next.should.have.been.called + + it 'should not provide an error', -> + expect(err).to.be.undefined + + it 'req.token should remain null', -> + expect(req.token).to.be.null + + describe 'req.token is undefined (this is unexpected)', -> + before (done) -> + req = + token: undefined + + next = sinon.spy (error) -> + err = error + done() + + passwordless.extractTokenSub req, res, next + + it 'should continue', -> + next.should.have.been.called + + it 'should not provide an error', -> + expect(err).to.be.undefined + + it 'req.token should remain undefined', -> + expect(req.token).to.be.undefined + + describe 'req.token has no sub ', -> + token = {id: 'foo'} + before (done) -> + req = + token: token + + next = sinon.spy (error) -> + err = error + done() + + passwordless.extractTokenSub req, res, next + + it 'should continue', -> + next.should.have.been.called + + it 'should not provide an error', -> + expect(err).to.be.undefined + + it 'req.token should remain unchanged', -> + req.token.should.equal(token) + + describe 'req.token with sub ', -> + token = {sub: '3.14'} + before (done) -> + req = + token: token + + next = sinon.spy (error) -> + err = error + done() + + passwordless.extractTokenSub req, res, next + + it 'should continue', -> + next.should.have.been.called + + it 'should not provide an error', -> + expect(err).to.be.undefined + + it 'req.connectParams should not be undefined', -> + req.connectParams.should.not.be.undefined + + describe 'req.token has expected connectParams and more in sub ', -> + sub = + email: 'test@test.org' + redirect_uri: 'https://test.org/callback' + client_id: 'test-client-id' + response_type: 'test-response-type' + scope: 'test-scope' + nonce: 'test-nonce' + unexpected: 'test-unexpected' + + token = {sub: sub} + + before (done) -> + req = + token: token + + next = sinon.spy (error) -> + err = error + done() + + passwordless.extractTokenSub req, res, next + + it 'should continue', -> + next.should.have.been.called + + it 'should not provide an error', -> + expect(err).to.be.undefined + + it 'req.connectParams to have property email', -> + expect(req.connectParams).property('email', 'test@test.org') + + it 'req.connectParams to have property redirect_uri', -> + expect(req.connectParams).property('redirect_uri', 'https://test.org/callback') + + it 'req.connectParams to have property client_id', -> + expect(req.connectParams).property('client_id', 'test-client-id') + + it 'req.connectParams to have property response_type', -> + expect(req.connectParams).property('response_type', 'test-response-type') + + it 'req.connectParams to have property scope', -> + expect(req.connectParams).property('scope', 'test-scope') + + it 'req.connectParams to have property nonce', -> + expect(req.connectParams).property('nonce', 'test-nonce') + + it 'req.connectParams to not have property unexpected', -> + expect(req.connectParams.unexpected).to.be.undefined + + describe 'req.token has sub with only some of the expected connectParams', -> + sub = + email: 'test@test.org' + redirect_uri: 'https://test.org/callback' + client_id: 'test-client-id' + response_type: 'test-response-type' + + token = {sub: sub} + + before (done) -> + req = + token: token + + next = sinon.spy (error) -> + err = error + done() + + passwordless.extractTokenSub req, res, next + + it 'should continue', -> + next.should.have.been.called + + it 'should not provide an error', -> + expect(err).to.be.undefined + + it 'req.connectParams to have property email', -> + expect(req.connectParams).property('email', 'test@test.org') + + it 'req.connectParams to have property redirect_uri', -> + expect(req.connectParams).property('redirect_uri', 'https://test.org/callback') + + it 'req.connectParams to have property response_type', -> + expect(req.connectParams).property('response_type', 'test-response-type') + + it 'req.connectParams to NOT have property scope', -> + expect(req.connectParams.scope).to.be.undefined + + it 'req.connectParams to NOT have property nonce', -> + expect(req.connectParams.nonce).to.be.undefined + + describe 'req.token has sub user', -> + sub = + user: 'test-user-id' + + token = {sub: sub} + + before (done) -> + req = + token: token + + next = sinon.spy (error) -> + err = error + done() + + passwordless.extractTokenSub req, res, next + + it 'should continue', -> + next.should.have.been.called + + it 'should not provide an error', -> + expect(err).to.be.undefined + + it 'req to have property user_id', -> + expect(req).property('user_id', 'test-user-id') diff --git a/test/unit/oidc/passwordlessPeekToken.coffee b/test/unit/oidc/passwordlessPeekToken.coffee new file mode 100644 index 00000000..5283bee7 --- /dev/null +++ b/test/unit/oidc/passwordlessPeekToken.coffee @@ -0,0 +1,147 @@ +chai = require 'chai' +sinon = require 'sinon' +sinonChai = require 'sinon-chai' +expect = chai.expect + + +chai.use sinonChai +chai.should() + + +passwordless = require '../../../routes/passwordless' +passwordless = passwordless.middleware + +# InvalidRequestError = require '../../../errors/InvalidRequestError' + +settings = require '../../../boot/settings' +TestSettings = require '../../lib/testSettings' + +InvalidRequestError = require '../../../errors/InvalidRequestError' +OneTimeToken = require '../../../models/OneTimeToken' + + +describe 'Passwordless middleware tests', -> + + + {req,res,next,err, issuer} = {} + + tsSettings = {} + + + describe 'peek link token', -> + + before -> + tsSettings = new TestSettings(settings) + tsSettings.addSettings + issuer: 'https://test.issuer.com' + providers: + passwordless: {} + + after -> + tsSettings.restore() + + describe 'missing token is an error', -> + before -> + req = + query: {} + + req.connectParams = req.query + + next = sinon.spy() + + passwordless.peekToken req, res, next + + it 'should continue', -> + next.should.have.been.called + + it 'should provide an invalid request error', -> + next.should.have.been.calledWith sinon.match.instanceOf(InvalidRequestError) + + describe 'valid token after stub OneTimeToken#peek call', -> + {err} = {} + {token} = {} + + before (done) -> + req = + body: + token: 'the-passwordless-token' + req.connectParams = req.body + + next = sinon.spy (error) -> + err = error + done() + + token = {test: 'foo'} + sinon.stub(OneTimeToken, 'peek').callsArgWith(1, null, token) + + passwordless.peekToken req, res, next + + after -> + OneTimeToken.peek.restore() + + it 'should continue', -> + next.should.have.been.called + + it 'should not provide an error', -> + expect(err).to.be.undefined + + it 'should set token on the request', -> + req.token.should.equal token + + describe 'token not found case in OneTimeToken#peek not found case results in next with no errors and req.token = null', -> + {err} = {} + + before (done) -> + req = + body: + token: 'the-passwordless-token' + req.connectParams = req.body + + next = sinon.spy (error) -> + err = error + done() + + sinon.stub(OneTimeToken, 'peek').callsArgWith(1, null, null) + + passwordless.peekToken req, res, next + + after -> + OneTimeToken.peek.restore() + + it 'should continue', -> + next.should.have.been.called + + it 'should not provide an error', -> + expect(err).to.be.undefined + + it 'should have null token on the request', -> + expect(req.token).to.be.null + + describe 'OneTimeToken#peek returns err reports error in next(err) call', -> + {err} = {} + + before (done) -> + req = + body: + token: 'the-passwordless-token' + req.connectParams = req.body + + next = sinon.spy (error) -> + err = error + done() + + sinon.stub(OneTimeToken, 'peek').callsArgWith(1, new Error("fake A redis DB unit test error"), null) + + passwordless.peekToken req, res, next + + after -> + OneTimeToken.peek.restore() + + it 'should continue', -> + next.should.have.been.called + + it 'should provide an error', -> + next.should.have.been.calledWith sinon.match.instanceOf(Error) + + it 'should have undefined token on the request', -> + expect(req.token).to.be.undefined diff --git a/test/unit/oidc/passwordlessRenderInvalidEmail.coffee b/test/unit/oidc/passwordlessRenderInvalidEmail.coffee new file mode 100644 index 00000000..b07fa14d --- /dev/null +++ b/test/unit/oidc/passwordlessRenderInvalidEmail.coffee @@ -0,0 +1,91 @@ +chai = require 'chai' +sinon = require 'sinon' +sinonChai = require 'sinon-chai' +expect = chai.expect + + +chai.use sinonChai +chai.should() + + +passwordless = require '../../../routes/passwordless' +passwordless = passwordless.middleware + +# InvalidRequestError = require '../../../errors/InvalidRequestError' + +settings = require '../../../boot/settings' +TestSettings = require '../../lib/testSettings' + +User = require '../../../models/User' + + +describe 'Passwordless middleware tests', -> + + + {req,res,next,err, issuer} = {} + + tsSettings = {} + + + describe 'render invalid email error', -> + + {view, view_info} = {} + + before -> + tsSettings = new TestSettings(settings) + tsSettings.addSettings + issuer: 'https://test.issuer.com' + providers: + passwordless: {} + + after -> + tsSettings.restore() + + describe 'req.connectParams.email is undefined', -> + before (done) -> + req = + connectParams: {} + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.renderInvalidEmail req, res, next + + it 'should not continue', -> + next.should.not.have.been.called + + it 'should render signin view with error', -> + res.render.should.have.been. + calledWith 'signin', + sinon.match({ + error: sinon.match.string + }) + + describe 'req.connectParams.email is something', -> + before (done) -> + req = + connectParams: + email: 'foo@test.org' + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.renderInvalidEmail req, res, next + + it 'should continue', -> + next.should.have.been.called + + it 'should not have an error', -> + expect(err).to.be.undefined diff --git a/test/unit/oidc/passwordlessVerifyEnablement.coffee b/test/unit/oidc/passwordlessVerifyEnablement.coffee new file mode 100644 index 00000000..3ecabd16 --- /dev/null +++ b/test/unit/oidc/passwordlessVerifyEnablement.coffee @@ -0,0 +1,67 @@ +chai = require 'chai' +sinon = require 'sinon' +sinonChai = require 'sinon-chai' +expect = chai.expect + + +chai.use sinonChai +chai.should() + + +passwordless = require '../../../routes/passwordless' +passwordless = passwordless.middleware + +settings = require '../../../boot/settings' +TestSettings = require '../../lib/testSettings' + +describe 'Passwordless middleware tests', -> + + + {req,res,next,err, issuer} = {} + + tsSettings = {} + + describe 'passwordless enablement', -> + + describe 'verify when enabled', -> + + before -> + tsSettings = new TestSettings(settings) + tsSettings.addSettings + issuer: 'https://test.issuer.com' + providers: + passwordless: {} + next = sinon.spy() + passwordless.verifyEnabled req, res, next + + after -> + tsSettings.restore() + + it 'should continue', -> + next.should.have.been.called + + it 'should not provide an error', -> + next.should.not.have.been.calledWith sinon.match.defined + + + describe 'verify when not enabled', -> + + before -> + tsSettings = new TestSettings(settings) + tsSettings.addSettings + issuer: 'https://test.issuer.com' + providers: {} + + next = sinon.spy() + passwordless.verifyEnabled req, res, next + + after -> + tsSettings.restore() + + it 'should continue', -> + next.should.have.been.called + + it 'should provide an error', -> + next.should.have.been.calledWith sinon.match.instanceOf(Error) + + # boot/settings will always establish a settings.providers object. diff --git a/test/unit/oidc/passwordlessVerifyNewUserSigninToken.coffee b/test/unit/oidc/passwordlessVerifyNewUserSigninToken.coffee new file mode 100644 index 00000000..2c16d6a7 --- /dev/null +++ b/test/unit/oidc/passwordlessVerifyNewUserSigninToken.coffee @@ -0,0 +1,207 @@ +chai = require 'chai' +sinon = require 'sinon' +sinonChai = require 'sinon-chai' +expect = chai.expect + + +chai.use sinonChai +chai.should() + + +passwordless = require '../../../routes/passwordless' +passwordless = passwordless.middleware + +# InvalidRequestError = require '../../../errors/InvalidRequestError' + +settings = require '../../../boot/settings' +TestSettings = require '../../lib/testSettings' + +User = require '../../../models/User' + + +describe 'Passwordless middleware tests', -> + + + {req,res,next,err, issuer} = {} + + tsSettings = {} + + + describe 'verify new user create account and sign-in token', -> + + {view, view_info} = {} + + before -> + tsSettings = new TestSettings(settings) + tsSettings.addSettings + issuer: 'https://test.issuer.com' + providers: + passwordless: {} + + after -> + tsSettings.restore() + + describe 'req.token is null', -> + before (done) -> + req = + token: null + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.verifyNewUserSigninToken req, res, next + + it 'should not continue', -> + next.should.not.have.been.called + + it 'should render view with error', -> + res.render.should.have.been. + calledWith 'passwordless/pwlessSignup', + sinon.match({ + error: sinon.match.string + }) + + describe 'req.token is undefined', -> + before (done) -> + req = {} + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.verifyNewUserSigninToken req, res, next + + it 'should not continue', -> + next.should.not.have.been.called + + it 'should render view with error', -> + res.render.should.have.been. + calledWith 'passwordless/pwlessSignup', + sinon.match({ + error: sinon.match.string + }) + + describe 'req.token.use is not pwless-new-user', -> + before (done) -> + req = + token: + use: 'foo' + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.verifyNewUserSigninToken req, res, next + + it 'should not continue', -> + next.should.not.have.been.called + + it 'should render view with error', -> + res.render.should.have.been. + calledWith 'passwordless/pwlessSignup', + sinon.match({ + error: sinon.match.string + }) + + + describe 'User insert fails with error', -> + before (done) -> + req = + session: {} + token: + _id: 'token-id' + use: 'pwless-new-user' + sub: + email: 'test@test.org' + req.connectParams = {} + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + sinon.stub(User, 'insert').callsArgWith(2, "Field foo must have format YYY/MM/DD") + + passwordless.verifyNewUserSigninToken req, res, next + + after -> + User.insert.restore() + + it 'should not continue', -> + next.should.not.have.been.called + + it 'should render view with error', -> + res.render.should.have.been. + calledWith 'passwordless/pwlessSignup', + sinon.match({ + error: sinon.match.string + token: sinon.match.string + email: sinon.match.string + }) + + + + describe 'User insertion works', -> + user = new User _id: 'uuid-test' + before (done) -> + req = + provider: + amr: 'test-amr' + user_id: 'test-user-id' + token: + use: 'pwless-new-user' + session: {} + + sinon.stub(User, 'insert').callsArgWith(2, null, user); + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.verifyNewUserSigninToken req, res, next + + after -> + User.insert.restore() + + it 'should continue', -> + next.should.have.been.called + + it 'should not have an error', -> + expect(err).to.be.undefined + + it 'req.user should be the user returned from User.insert call', -> + req.user.should.equal(user) + + it 'req.session should have the user_id', -> + req.session.user.should.equal('uuid-test') + + it 'req.session.amr should match the test seup', -> + req.session.amr.should.have.members(['test-amr']) + + it 'req.session.opbs exists (it should contain a random value)', -> + req.session.opbs.should.exist diff --git a/test/unit/oidc/passwordlessVerifySignupToken.coffee b/test/unit/oidc/passwordlessVerifySignupToken.coffee new file mode 100644 index 00000000..e84f6a3b --- /dev/null +++ b/test/unit/oidc/passwordlessVerifySignupToken.coffee @@ -0,0 +1,186 @@ +chai = require 'chai' +sinon = require 'sinon' +sinonChai = require 'sinon-chai' +expect = chai.expect + + +chai.use sinonChai +chai.should() + + +passwordless = require '../../../routes/passwordless' +passwordless = passwordless.middleware + +# InvalidRequestError = require '../../../errors/InvalidRequestError' + +settings = require '../../../boot/settings' +TestSettings = require '../../lib/testSettings' + +OneTimeToken = require '../../../models/OneTimeToken' + + +describe 'Passwordless middleware tests', -> + + + {req,res,next,err, issuer} = {} + + tsSettings = {} + + + describe 'verify sign-up token', -> + + {view, view_info} = {} + + before -> + tsSettings = new TestSettings(settings) + tsSettings.addSettings + issuer: 'https://test.issuer.com' + providers: + passwordless: {} + + after -> + tsSettings.restore() + + describe 'req.token is null', -> + before (done) -> + req = + token: null + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.verifySignupToken req, res, next + + it 'should not continue', -> + next.should.not.have.been.called + + it 'should render view with error', -> + res.render.should.have.been. + calledWith 'passwordless/pwlessSigninLinkError', + sinon.match({ + error: sinon.match.string + }) + + describe 'req.token is undefined', -> + before (done) -> + req = {} + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.verifySignupToken req, res, next + + it 'should not continue', -> + next.should.not.have.been.called + + it 'should render view with error', -> + res.render.should.have.been. + calledWith 'passwordless/pwlessSigninLinkError', + sinon.match({ + error: sinon.match.string + }) + + describe 'req.token.use is not pwless-signup', -> + before (done) -> + req = + token: + use: 'foo' + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.verifySignupToken req, res, next + + it 'should not continue', -> + next.should.not.have.been.called + + it 'should render view with error', -> + res.render.should.have.been. + calledWith 'passwordless/pwlessSigninLinkError', + sinon.match({ + error: sinon.match.string + }) + + + describe 'error on DB call to issue new Token', -> + before (done) -> + req = + token: + use: 'pwless-signup' + + sinon.stub(OneTimeToken, 'issue').callsArgWith(1, new Error('database problem')) + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.verifySignupToken req, res, next + + after -> + OneTimeToken.issue.restore() + + it 'should continue', -> + next.should.have.been.called + + # Perhaps it would be better to render a more helpful message, but this is consistent with verifyEmail + it 'should provide an error', -> + next.should.have.been.calledWith sinon.match.instanceOf(Error) + + describe 'successful with a new signup token ', -> + before (done) -> + req = + user_id: 'test-user-id' + token: + use: 'pwless-signup' + sub: + email: 'test@test.org' + + sinon.stub(OneTimeToken, 'issue').callsArgWith(1, null, new OneTimeToken {} ) + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.verifySignupToken req, res, next + + after -> + OneTimeToken.issue.restore() + + it 'should NOT continue', -> + next.should.not.have.been.called + + it 'should render view with email and token', -> + res.render.should.have.been. + calledWith 'passwordless/pwlessSignup', + sinon.match({ + email: sinon.match.string, + token: sinon.match.string + }) diff --git a/test/unit/oidc/passwordlessVerifyToken.coffee b/test/unit/oidc/passwordlessVerifyToken.coffee new file mode 100644 index 00000000..e5d762ce --- /dev/null +++ b/test/unit/oidc/passwordlessVerifyToken.coffee @@ -0,0 +1,284 @@ +chai = require 'chai' +sinon = require 'sinon' +sinonChai = require 'sinon-chai' +expect = chai.expect + + +chai.use sinonChai +chai.should() + + +passwordless = require '../../../routes/passwordless' +passwordless = passwordless.middleware + +# InvalidRequestError = require '../../../errors/InvalidRequestError' + +settings = require '../../../boot/settings' +TestSettings = require '../../lib/testSettings' + +User = require '../../../models/User' + + +describe 'Passwordless middleware tests', -> + + + {req,res,next,err, issuer} = {} + + tsSettings = {} + + + describe 'verify sign-in token', -> + + {view, view_info} = {} + + before -> + tsSettings = new TestSettings(settings) + tsSettings.addSettings + issuer: 'https://test.issuer.com' + providers: + passwordless: {} + + after -> + tsSettings.restore() + + describe 'req.token is null', -> + before (done) -> + req = + token: null + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.verifyToken req, res, next + + it 'should not continue', -> + next.should.not.have.been.called + + it 'should render view with error', -> + res.render.should.have.been. + calledWith 'passwordless/pwlessSigninLinkError', + sinon.match({ + error: sinon.match.string + }) + + describe 'req.token is undefined', -> + before (done) -> + req = {} + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.verifyToken req, res, next + + it 'should not continue', -> + next.should.not.have.been.called + + it 'should render view with error', -> + res.render.should.have.been. + calledWith 'passwordless/pwlessSigninLinkError', + sinon.match({ + error: sinon.match.string + }) + + describe 'req.token.use is not pwless-signin', -> + before (done) -> + req = + token: + use: 'foo' + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.verifyToken req, res, next + + it 'should not continue', -> + next.should.not.have.been.called + + it 'should render view with error', -> + res.render.should.have.been. + calledWith 'passwordless/pwlessSigninLinkError', + sinon.match({ + error: sinon.match.string + }) + + + describe 'req.user_id is undefined', -> + before (done) -> + req = + token: + use: 'pwless-signin' + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.verifyToken req, res, next + + it 'should not continue', -> + next.should.not.have.been.called + + it 'should render view with error', -> + res.render.should.have.been. + calledWith 'passwordless/pwlessSigninLinkError', + sinon.match({ + error: sinon.match.string + }) + + describe 'req.user_id is only whitespaces', -> + before (done) -> + req = + user_id: ' ' + token: + use: 'pwless-signin' + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.verifyToken req, res, next + + it 'should not continue', -> + next.should.not.have.been.called + + it 'should render view with error', -> + res.render.should.have.been. + calledWith 'passwordless/pwlessSigninLinkError', + sinon.match({ + error: sinon.match.string + }) + + describe 'error on DB call', -> + before (done) -> + req = + user_id: 'test-user-id' + token: + use: 'pwless-signin' + + sinon.stub(User, 'patch').callsArgWith(2, new Error('database problem')) + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.verifyToken req, res, next + + after -> + User.patch.restore() + + it 'should continue', -> + next.should.have.been.called + + # Perhaps it would be better to render a more helpful message, but this is consistent with verifyEmail + it 'should provide an error', -> + next.should.have.been.calledWith sinon.match.instanceOf(Error) + + describe 'with an unknown user ', -> + before (done) -> + req = + user_id: 'test-user-id' + token: + use: 'pwless-signin' + + sinon.stub(User, 'patch').callsArgWith(2, null, null) + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.verifyToken req, res, next + + after -> + User.patch.restore() + + it 'should NOT continue', -> + next.should.not.have.been.called + + it 'should render view with error', -> + res.render.should.have.been. + calledWith 'passwordless/pwlessSigninLinkError', + sinon.match({ + error: sinon.match.string + }) + + describe 'for known user', -> + user = new User _id: 'uuid-test' + before (done) -> + req = + provider: + amr: 'test-amr' + user_id: 'test-user-id' + token: + use: 'pwless-signin' + session: {} + + sinon.stub(User, 'patch').callsArgWith(2, null, user) + + res = render: sinon.spy (vw, vw_info) -> + view = vw + view_info = view_info + done() + + next = sinon.spy (error) -> + err = error + done() + + passwordless.verifyToken req, res, next + + after -> + User.patch.restore() + + it 'should continue', -> + next.should.have.been.called + + it 'should not have an error', -> + expect(err).to.be.undefined + + it 'req.user should be the user returned from DB call', -> + req.user.should.equal(user) + + it 'req.session should have the user_id', -> + req.session.user.should.equal('uuid-test') + + it 'req.session.amr should match the test seup', -> + req.session.amr.should.have.members(['test-amr']) + + it 'req.session.opbs exists (it should contain a random value)', -> + req.session.opbs.should.exist diff --git a/views/passwordless/pwlessSigninLinkVerified.jade b/views/passwordless/pwlessSigninLinkError.jade similarity index 79% rename from views/passwordless/pwlessSigninLinkVerified.jade rename to views/passwordless/pwlessSigninLinkError.jade index 312c0970..6628cd11 100644 --- a/views/passwordless/pwlessSigninLinkVerified.jade +++ b/views/passwordless/pwlessSigninLinkError.jade @@ -15,7 +15,5 @@ html(lang="en") if error .error= error else + //- this case should never happen for passwordless.. div Your e-mail link has now been verified! - if signin - div You may now continue to sign in to #{signin.client.client_name}. - a.button(href=signin.url) Continue to sign in diff --git a/views/passwordless/pwlessSignup.jade b/views/passwordless/pwlessSignup.jade new file mode 100644 index 00000000..92b1cce7 --- /dev/null +++ b/views/passwordless/pwlessSignup.jade @@ -0,0 +1,23 @@ +doctype html +html(lang="en") + head + title Sign up | Anvil Connect + + link(rel='stylesheet', href='//maxcdn.bootstrapcdn.com/font-awesome/4.1.0/css/font-awesome.min.css') + link(rel='stylesheet', href='/stylesheets/app.css') + link(rel='stylesheet', href='//fonts.googleapis.com/css?family=Roboto:400,100,100italic,400italic,700,700italic|Raleway:400,100,600,300|Playfair+Display+SC:900,400') + body + + .anvilform + img.logo(src='/images/anvil.svg', alt='Anvil Connect') + + form.panel(method='POST') + p= error + input(type='hidden', name='token', value=token) + hr.textsep.signup + p + | Create a new account for email #{email}. + | This form must be submitted within one day. + input(type='text', name='givenName', placeholder='First Name') + input(type='text', name='familyName', placeholder='Last Name') + input.callout.full(type='submit', value='Sign up') From 6dd519e786e1de80404d040001c1788e01077e74 Mon Sep 17 00:00:00 2001 From: Henrich Kraemer Date: Wed, 7 Oct 2015 22:24:11 +0200 Subject: [PATCH 6/9] Fix issues found during manual tests and cleanup - passwordless new user to have a verified email status. - resend link to have all required parameters including nonce - comment in hogen should not be in html - Improved comments - New module render/renderSignin.js to allow res.render('signin') call with proper context. - test to assert that render signin shows formError not error --- email/passwordlessSignin.hogan | 5 +- providers/passwordless.js | 6 +- render/renderSignin.js | 36 ++++ routes/passwordless.js | 164 +++++++++++------- routes/signin.js | 41 +---- test/integration/routes/pwless_signin.coffee | 34 +++- .../routes/pwless_signin_link.coffee | 5 + .../routes/pwless_signup_post.coffee | 2 +- .../passwordlessRenderInvalidEmail.coffee | 2 +- ...asswordlessVerifyNewUserSigninToken.coffee | 4 + 10 files changed, 191 insertions(+), 108 deletions(-) create mode 100644 render/renderSignin.js diff --git a/email/passwordlessSignin.hogan b/email/passwordlessSignin.hogan index a23f4f1a..5aa95e18 100644 --- a/email/passwordlessSignin.hogan +++ b/email/passwordlessSignin.hogan @@ -17,11 +17,10 @@

Follow the link below by clicking the button if you want to sign in to {{providerName}}.

- -

Sign in to {{providerName}}

-

If you don't see a link above, paste the following URL into your browser: {{pwlessURL}}

+

If you don't see a link above, paste the following URL into your browser: {{verifyURL}}

This e-mail was addressed to {{email}}

diff --git a/providers/passwordless.js b/providers/passwordless.js index dd7bc4bc..f61d5e9d 100644 --- a/providers/passwordless.js +++ b/providers/passwordless.js @@ -7,13 +7,13 @@ module.exports = function (config) { return { id: 'passwordless', - name: 'Email only', + name: 'Sign in or create account with your email', protocol: 'Passwordless', - amr: 'pwd', // TODO: is there a proper amr value for passwordless? + amr: 'email', // TODO: this is not a standard value, see https://tools.ietf.org/html/draft-jones-oauth-amr-values-01#section-2 tokenTTL: 60 * 15, fields: [ { name: 'email', type: 'email' } ], - usernameField: 'email' // TODO: write test that this is used. + usernameField: 'email' // TODO: The usernameField appears nowhere to be referenced?. This was copied from the Password provider. } } diff --git a/render/renderSignin.js b/render/renderSignin.js new file mode 100644 index 00000000..6a94f140 --- /dev/null +++ b/render/renderSignin.js @@ -0,0 +1,36 @@ +/** + * Module dependencies + */ +var _ = require('lodash') +var qs = require('qs') + +var settings = require('../boot/settings') +var mailer = require('../boot/mailer') +var providers = require('../providers') + +var providerInfo = {} +var providerNames = Object.keys(providers) +for (var i = 0; i < providerNames.length; i++) { + providerInfo[providerNames[i]] = providers[providerNames[i]] +} +var visibleProviders = {} +// Only render providers that are not marked as hidden +Object.keys(settings.providers).forEach(function (providerID) { + if (!settings.providers[providerID].hidden) { + visibleProviders[providerID] = settings.providers[providerID] + } +}) + +module.exports = function (res, params, options) { + var locals = { + params: qs.stringify(params), + request: params, + providers: visibleProviders, + providerInfo: providerInfo, + mailSupport: !!(mailer.transport) + } + if (typeof options === 'object') { + _.assign(locals, options) + } + res.render('signin', locals) +} diff --git a/routes/passwordless.js b/routes/passwordless.js index 81da9d0f..a325a72d 100644 --- a/routes/passwordless.js +++ b/routes/passwordless.js @@ -5,6 +5,7 @@ var express = require('express') var _ = require('lodash') var url = require('url') + var settings = require('../boot/settings') var mailer = require('../boot/mailer') var authenticator = require('../lib/authenticator') @@ -13,40 +14,80 @@ var User = require('../models/User') var OneTimeToken = require('../models/OneTimeToken') var InvalidRequestError = require('../errors/InvalidRequestError') var PasswordlessDisabledError = require('../errors/PasswordlessDisabledError') +var renderSignin = require('../render/renderSignin') /** - * Passwordless - * - * Both new and existing users start at the signin page - * - * Common flow to signin or signup, starting at /signin : - - * 1. User provides email - * 2. Validate input as valid email, display error if not. - * 3. Check whether user account exists. - * 4. Send link in either case with a one time token expiring - * after 15 minutes by default. - * 4.1 If user account exists email tells user to click and confirm - * that they want to sign in. - * Link to route /signin/pwless/... - * 4.2 If new user email tells user to click the link if that want - * to create a new user account. - * Link to route /signup/pwless/... - * - * New user passwordless signup flow: (/signup/pwless) - * - * 1. Verify token exists and is for pwless. - * 2. Direct user to new passwordless user view, where user can enter additional details such as last name, first name. - * 3. Create user based on form information, revoke token. - * 4. Send welcome mail. - * 5. Authorize user - * - * Existing user passwordless sigin flow: (/signin/pwless) - * - * 1. Verify token exists and is for pwless. - * 2. Update user email verified status with current time. - * 3. Authorize user. - */ +Passwordless + +FLOW: Sign in and signup + +POST route: /sigin + In passwordless sign up starts exactly like sign in. + There is a common flow to signin or signup, starting at /signin : + + 1. User selects 'Sign in or sign up with your email' method, provides email + and submits. + + 2. Standard post /signin middleware is performed, such as + verifyClient, validateAuthorizationParams an more. + + 3. Passwordless configuration validations are done: + verifyPasswordlessEnabled + verifyMailerConfigured + + 4. The input email is validated syntactically and errors are rendered + to the sign in page. + + 5. Check whether there is a user account for that email. + Send link in either case with an id of a one time token + expiring after 15 minutes by default. + The token also captures parameters such as the email, + redirect_uri, client_id, response_type, scope, nonce. + + 5.1.A. Existing user: + If user account exists email tells user to click and confirm + that they want to sign in. + Link to route /signin/passwordless?token= + 5.1.B. New user: + If new user email tells user to click the link if that want + to create a new user account. + Link to route /signup/passwordless?token= + + 5.2 Show page telling user to check their mail and that + a link has been send. This page also allows to resend the email. + +FLOW: New user sign up flow: + +GET route: /signup/passwordless + 1. Verify token exists and is for passwordless signup. + 2. verifyClient, validateAuthorizationParams, + 3. verifyPasswordlessEnabled + 4. Issue a new token for subsequent account creation when form is + submitted. This token has a longer expiration (1 day default). + 5. Renders signup form 'passwordless/pwlessSignup' for user to enter + given_name and family_name. + +POST route: + 1. Verify token exists and is for passwordless create new account. + 2. verifyClient, validateAuthorizationParams, + 3. verifyPasswordlessEnabled + 4. Create new user based on form data and email. + 5. Revoke token. + [ Not done : Send welcome mail] + 6. login user + 7. Authorize user + +FLOW: Existing user sign in flow: + +GET route: /signin/passwordless + 1. Verify token exists and is for passwordless signup. + 2. verifyClient, validateAuthorizationParams, + 3. verifyPasswordlessEnabled + 4. update users verified email date. + 5. login user + 6. authorize user + +*/ var TOKEN_USAGE_SIGNIN = 'pwless-signin' var TOKEN_USAGE_SIGNUP = 'pwless-signup' @@ -195,7 +236,14 @@ function verifyPasswordlessNewUserSigninToken (req, res, next) { password: false } - User.insert(req.connectParams, userOptions, function (err, user) { + var userData = { + dateEmailVerified: Date.now(), + emailVerified: true + } + + _.assign(userData, req.connectParams) + + User.insert(userData, userOptions, function (err, user) { if (err) { return res.render(view, { error: err, @@ -203,18 +251,19 @@ function verifyPasswordlessNewUserSigninToken (req, res, next) { email: token.sub.email }) } - OneTimeToken.revoke(token._id) - // don't care if token revocation fails as they expire anyhow. - req.user = user - authenticator.login(req, user) - next() + OneTimeToken.revoke(token._id, function () { + // don't care if token revocation fails as they expire anyhow. + req.user = user + authenticator.login(req, user) + next() + }) }) } function signinRenderErrorInvalidEmail (req, res, next) { if (typeof req.connectParams.email === 'undefined') { - return res.render('signin', { - error: 'Please enter a valid e-mail address.' + return renderSignin(res, req.connectParams, { + formError: 'Please enter a valid e-mail address.' }) } next() @@ -236,7 +285,7 @@ function sendMail (req, res, next) { User.getByEmail(req.connectParams.email, function (err, user) { if (err) { return next(err) } - // TODO: There may be differences in processing delays + // The runtime of User.getByEmail may reveal // whether user has an account or not. This could potentially // be used for an attack. @@ -256,21 +305,21 @@ function sendMail (req, res, next) { var email = tokenOptions.sub.email var verifyURL = url.parse(settings.issuer) - verifyURL.pathname = user ? 'signin/pwless' : 'signup/pwless' + verifyURL.pathname = user ? 'signin/passwordless' : 'signup/passwordless' verifyURL.query = { token: token._id } + var mailOptions = { + email: email, + verifyURL: url.format(verifyURL), + providerName: req.client.client_name + } + var template = user ? 'passwordlessSignin' : 'passwordlessSignup' var subject = user - ? 'Sign in to ' + req.client.client_name - : 'Create your account on ' + req.client.client_name + ? 'Sign in to ' + mailOptions.providerName + : 'Create your account on ' + mailOptions.providerName - // TODO: test is {{providerName}} available in template? - // TODO: see also usage of #{signin.client.client_name} in verifyEmail.jade, oidc.verifyClient - - mailer.sendMail(template, { - email: email, - verifyURL: url.format(verifyURL) - }, { + mailer.sendMail(template, mailOptions, { to: email, subject: subject }, function (err, responseStatus) { @@ -292,15 +341,8 @@ function renderSentMail (req, res, next) { var resendURL = url.parse(settings.issuer) resendURL.pathname = 'resend/passwordless' - resendURL.query = { - email: req.connectParams.email - } - if (req.connectParams) { - resendURL.query.redirect_uri = req.connectParams.redirect_uri - resendURL.query.client_id = req.connectParams.client_id - resendURL.query.response_type = req.connectParams.response_type - resendURL.query.scope = req.connectParams.scope - } + resendURL.query = {} + _.assign(resendURL.query, _.pick(req.connectParams, CONNECT_SUB_FIELDS)) var locals = { from: mailer.from, resendURL: url.format(resendURL) @@ -336,7 +378,7 @@ function postSigninMiddleware () { return passwordlessSignin } -// signin/pwless, signup/pwless, +// routes for provider=passwordless function routes (server) { server.get('/resend/:provider', oidc.selectConnectParams, diff --git a/routes/signin.js b/routes/signin.js index 238c683f..4cd7ab58 100644 --- a/routes/signin.js +++ b/routes/signin.js @@ -4,26 +4,9 @@ var express = require('express') var oidc = require('../oidc') -var settings = require('../boot/settings') -var mailer = require('../boot/mailer') var authenticator = require('../lib/authenticator') var passwordless = require('./passwordless') -var qs = require('qs') -var providers = require('../providers') - -var providerInfo = {} -var providerNames = Object.keys(providers) -for (var i = 0; i < providerNames.length; i++) { - providerInfo[providerNames[i]] = providers[providerNames[i]] -} -var visibleProviders = {} -// Only render providers that are not marked as hidden -Object.keys(settings.providers).forEach(function (providerID) { - if (!settings.providers[providerID].hidden) { - visibleProviders[providerID] = settings.providers[providerID] - } -}) - +var renderSignin = require('../render/renderSignin') /** * Signin Endpoint */ @@ -38,13 +21,7 @@ module.exports = function (server) { oidc.verifyClient, oidc.validateAuthorizationParams, function (req, res, next) { - res.render('signin', { - params: qs.stringify(req.query), - request: req.query, - providers: visibleProviders, - providerInfo: providerInfo, - mailSupport: !!(mailer.transport) - }) + renderSignin(res, req.query) }) /** @@ -73,21 +50,11 @@ module.exports = function (server) { function (req, res, next) { authenticator.dispatch(req.body.provider, req, res, next, function (err, user, info) { if (err) { - res.render('signin', { - params: qs.stringify(req.body), - request: req.body, - providers: visibleProviders, - providerInfo: providerInfo, - mailSupport: !!(mailer.transport), + renderSignin(res, req.body, { error: err.message }) } else if (!user) { - res.render('signin', { - params: qs.stringify(req.body), - request: req.body, - providers: visibleProviders, - providerInfo: providerInfo, - mailSupport: !!(mailer.transport), + renderSignin(res, req.body, { formError: info.message }) } else { diff --git a/test/integration/routes/pwless_signin.coffee b/test/integration/routes/pwless_signin.coffee index 9697be33..56cb57df 100644 --- a/test/integration/routes/pwless_signin.coffee +++ b/test/integration/routes/pwless_signin.coffee @@ -148,9 +148,24 @@ describe 'Passwordless signin post', -> it 'should respond with html page containing the sender', -> res.text.should.contain 'from@example.com' - it 'should respond with html page containing the resend link', -> + it 'should respond with html page containing a resend link', -> res.text.should.match (new RegExp("href=\"https://test.issuer.com/resend/passwordless\\?email=user%40test\\.com&", "g")) + it 'resend link should contain redirect_uri', -> + res.text.should.match (new RegExp("href=\"https://test.issuer.com/resend/passwordless\\?.*redirect_uri=", "g")) + + it 'resend link should contain client_id', -> + res.text.should.match (new RegExp("href=\"https://test.issuer.com/resend/passwordless\\?.*client_id=", "g")) + + it 'resend link should contain response_type', -> + res.text.should.match (new RegExp("href=\"https://test.issuer.com/resend/passwordless\\?.*response_type=", "g")) + + it 'resend link should contain scope', -> + res.text.should.match (new RegExp("href=\"https://test.issuer.com/resend/passwordless\\?.*scope=", "g")) + + it 'resend link should contain nonce', -> + res.text.should.match (new RegExp("href=\"https://test.issuer.com/resend/passwordless\\?.*nonce=", "g")) + describe 'success flow existing user', -> before (done) -> @@ -248,5 +263,20 @@ describe 'Passwordless signin post', -> it 'should respond with html page containing the sender', -> res.text.should.contain 'from@example.com' - it 'should respond with html page containing the resend link', -> + it 'should respond with html page containing a resend link', -> res.text.should.match (new RegExp("href=\"https://test.issuer.com/resend/passwordless\\?email=doe%40test\\.com&", "g")) + + it 'resend link should contain redirect_uri', -> + res.text.should.match (new RegExp("href=\"https://test.issuer.com/resend/passwordless\\?.*redirect_uri=", "g")) + + it 'resend link should contain client_id', -> + res.text.should.match (new RegExp("href=\"https://test.issuer.com/resend/passwordless\\?.*client_id=", "g")) + + it 'resend link should contain response_type', -> + res.text.should.match (new RegExp("href=\"https://test.issuer.com/resend/passwordless\\?.*response_type=", "g")) + + it 'resend link should contain scope', -> + res.text.should.match (new RegExp("href=\"https://test.issuer.com/resend/passwordless\\?.*scope=", "g")) + + it 'resend link should contain nonce', -> + res.text.should.match (new RegExp("href=\"https://test.issuer.com/resend/passwordless\\?.*nonce=", "g")) diff --git a/test/integration/routes/pwless_signin_link.coffee b/test/integration/routes/pwless_signin_link.coffee index c6f43dc4..30572de3 100644 --- a/test/integration/routes/pwless_signin_link.coffee +++ b/test/integration/routes/pwless_signin_link.coffee @@ -166,3 +166,8 @@ describe 'Passwordless signin link activation', -> IDToken.prototype.initializePayload.should.have.been.calledWith( sinon.match amr: session.amr ) + + it 'should include `amr` claim `email` in id_token', -> + IDToken.prototype.initializePayload.should.have.been.calledWith( + sinon.match amr: ['email'] + ) diff --git a/test/integration/routes/pwless_signup_post.coffee b/test/integration/routes/pwless_signup_post.coffee index fde0b4d6..6a99bf0a 100644 --- a/test/integration/routes/pwless_signup_post.coffee +++ b/test/integration/routes/pwless_signup_post.coffee @@ -99,7 +99,7 @@ describe 'Passwordless signup form submission', -> sinon.stub(User, 'insert').callsArgWith(2, null, user) - sinon.spy(OneTimeToken, 'revoke') + sinon.stub(OneTimeToken, 'revoke').callsArgWith(1, null) scope = 'openid profile developer' scopes = [ diff --git a/test/unit/oidc/passwordlessRenderInvalidEmail.coffee b/test/unit/oidc/passwordlessRenderInvalidEmail.coffee index b07fa14d..ebd31896 100644 --- a/test/unit/oidc/passwordlessRenderInvalidEmail.coffee +++ b/test/unit/oidc/passwordlessRenderInvalidEmail.coffee @@ -64,7 +64,7 @@ describe 'Passwordless middleware tests', -> res.render.should.have.been. calledWith 'signin', sinon.match({ - error: sinon.match.string + formError: sinon.match.string }) describe 'req.connectParams.email is something', -> diff --git a/test/unit/oidc/passwordlessVerifyNewUserSigninToken.coffee b/test/unit/oidc/passwordlessVerifyNewUserSigninToken.coffee index 2c16d6a7..5ef4f3b6 100644 --- a/test/unit/oidc/passwordlessVerifyNewUserSigninToken.coffee +++ b/test/unit/oidc/passwordlessVerifyNewUserSigninToken.coffee @@ -17,6 +17,7 @@ settings = require '../../../boot/settings' TestSettings = require '../../lib/testSettings' User = require '../../../models/User' +OneTimeToken = require '../../../models/OneTimeToken' describe 'Passwordless middleware tests', -> @@ -174,6 +175,8 @@ describe 'Passwordless middleware tests', -> sinon.stub(User, 'insert').callsArgWith(2, null, user); + sinon.stub(OneTimeToken, 'revoke').callsArgWith(1, null); + res = render: sinon.spy (vw, vw_info) -> view = vw view_info = view_info @@ -186,6 +189,7 @@ describe 'Passwordless middleware tests', -> passwordless.verifyNewUserSigninToken req, res, next after -> + OneTimeToken.revoke.restore() User.insert.restore() it 'should continue', -> From 2d02bbe89acfe6087ec315d8a380a910f85eccb6 Mon Sep 17 00:00:00 2001 From: Henrich Kraemer Date: Mon, 12 Oct 2015 08:54:03 +0200 Subject: [PATCH 7/9] Avoid internal server error when redis is not running. When redis is not running express-session will not find a session store ready and as a consequence not establish a req.session object. That led to an error when running the integration test: igelmac:connect2 dev$ DEBUG=express-session node_modules/.bin/mocha --timeout 0 --compilers coffee:coffee-script/register ./test/integration/routes/pwless_signin_link.coffee --reporter spec Passwordless signin link activation success flow existing user sign-IN express-session store is disconnected +0ms 21:28:41 access: GET /signin/passwordless?token=token-id-random-stuff 500 23ms ... error: { [Error: cannot GET /signin/passwordless?token=token-id-random-stuff (500)] status: 500, text: 'Internal Server Error

Cannot set property \'amr\' of undefined', method: 'GET', path: '/signin/passwordless?token=token-id-random-stuff' }, This fix simply establishes an empty req.session object. It appears that alternatives such as requiring to run redis for end to end integration testing might be better. Conflicts: test/integration/routes/pwless_signin_link.coffee --- test/integration/routes/pwless_signin_link.coffee | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/integration/routes/pwless_signin_link.coffee b/test/integration/routes/pwless_signin_link.coffee index 30572de3..a98ac56b 100644 --- a/test/integration/routes/pwless_signin_link.coffee +++ b/test/integration/routes/pwless_signin_link.coffee @@ -23,6 +23,8 @@ Scope = require('../../../models/Scope') IDToken = require '../../../models/IDToken' AccessToken = require '../../../models/AccessToken' +http = require 'http' + TestSettings = require '../../lib/testSettings' request = supertest(server) @@ -43,7 +45,13 @@ describe 'Passwordless signin link activation', -> passwordless: "tokenTTL-foo": 600 + # If redis is not running req.session remains undefined + # causing an error in oidc/setSessionAmr.js which ultimately + # result in a internal server error by oidc/error.js + http.IncomingMessage.prototype.session = {} + after -> + delete http.IncomingMessage.prototype.session tsSettings.restore() {err, res} = {} From 293eba79987c18ce2d0e95d192e8b32f0324a4ec Mon Sep 17 00:00:00 2001 From: Henrich Kraemer Date: Mon, 19 Oct 2015 18:16:33 +0200 Subject: [PATCH 8/9] Use render/renderSignin in routes/connect.jsi --- routes/connect.js | 27 +++------------------------ 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/routes/connect.js b/routes/connect.js index 10a1b380..b4a1d3d9 100644 --- a/routes/connect.js +++ b/routes/connect.js @@ -4,24 +4,9 @@ var settings = require('../boot/settings') var oidc = require('../oidc') -var mailer = require('../boot/mailer') var authenticator = require('../lib/authenticator') -var qs = require('qs') var NotFoundError = require('../errors/NotFoundError') -var providers = require('../providers') - -var providerInfo = {} -var providerNames = Object.keys(providers) -for (var i = 0; i < providerNames.length; i++) { - providerInfo[providerNames[i]] = providers[providerNames[i]] -} -var visibleProviders = {} -// Only render providers that are not marked as hidden -Object.keys(settings.providers).forEach(function (providerID) { - if (!settings.providers[providerID].hidden) { - visibleProviders[providerID] = settings.providers[providerID] - } -}) +var renderSignin = require('../render/renderSignin') /** * Third Party Provider Authorization Endpoints @@ -71,15 +56,9 @@ module.exports = function (server) { // render the signin screen with an error if (!user) { - res.render('signin', { - params: qs.stringify(req.connectParams), - request: req.body, - error: info.message, - providers: visibleProviders, - providerInfo: providerInfo, - mailSupport: !!(mailer.transport) + renderSignin(res, req.body, { + error: info.message }) - // login the user } else { authenticator.login(req, user) From 27dcabe044b6194ade713ab8c8264888f2bc1edd Mon Sep 17 00:00:00 2001 From: Henrich Kraemer Date: Fri, 27 Nov 2015 10:51:10 +0100 Subject: [PATCH 9/9] TestSessings must be in before hook. Otherwise these are called before execting the actual tests which interferes with other tests. This did not fail when unit and integration tests were run separately. --- test/integration/routes/pwless_resend.coffee | 32 +++++++++++-------- .../routes/pwless_signup_get.coffee | 15 +++++---- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/test/integration/routes/pwless_resend.coffee b/test/integration/routes/pwless_resend.coffee index 6ffd6a6f..6408c601 100644 --- a/test/integration/routes/pwless_resend.coffee +++ b/test/integration/routes/pwless_resend.coffee @@ -28,22 +28,26 @@ describe 'Passwordless resend email route', -> settings = require '../../../boot/settings' mailer = require '../../../boot/mailer' - tsSettings = new TestSettings(settings, + tsSettings = {} + tsMailer = {} + + before -> + tsSettings = new TestSettings(settings, _.pick(settings, ['response_types_supported'])) - tsSettings.addSettings - issuer: 'https://test.issuer.com' - providers: - passwordless: - "tokenTTL-foo": 600 - - tsMailer = new TestSettings(mailer, - from: "from@example.com" - render: {} - sendMail: (tmpl, loc, opts, cb) -> - cb() - transport: {} - ) + tsSettings.addSettings + issuer: 'https://test.issuer.com' + providers: + passwordless: + "tokenTTL-foo": 600 + + tsMailer = new TestSettings(mailer, + from: "from@example.com" + render: {} + sendMail: (tmpl, loc, opts, cb) -> + cb() + transport: {} + ) after -> tsMailer.restore() diff --git a/test/integration/routes/pwless_signup_get.coffee b/test/integration/routes/pwless_signup_get.coffee index 4065efa5..5a1a22bf 100644 --- a/test/integration/routes/pwless_signup_get.coffee +++ b/test/integration/routes/pwless_signup_get.coffee @@ -27,13 +27,16 @@ describe 'Passwordless signup get route', -> settings = require '../../../boot/settings' - tsSettings = new TestSettings(settings, - _.pick(settings, ['response_types_supported'])) + tsSettings = {} - tsSettings.addSettings - issuer: 'https://test.issuer.com' - providers: - passwordless: {} + before -> + tsSettings = new TestSettings(settings, + _.pick(settings, ['response_types_supported'])) + + tsSettings.addSettings + issuer: 'https://test.issuer.com' + providers: + passwordless: {} after ->