From 1667b75789cccb311cfc3b0d232533d79f80c189 Mon Sep 17 00:00:00 2001 From: Robin Date: Sun, 28 Aug 2016 18:31:07 +0200 Subject: [PATCH 1/4] add support for setting registration fields from account schema --- ...richIntegrationFromRemoteConfigStrategy.js | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/lib/strategy/EnrichIntegrationFromRemoteConfigStrategy.js b/lib/strategy/EnrichIntegrationFromRemoteConfigStrategy.js index 2e65161..e5231c1 100644 --- a/lib/strategy/EnrichIntegrationFromRemoteConfigStrategy.js +++ b/lib/strategy/EnrichIntegrationFromRemoteConfigStrategy.js @@ -206,12 +206,47 @@ EnrichIntegrationFromRemoteConfigStrategy.prototype._enrichWithDirectoryPolicies config.passwordPolicy = strength; - callback(null, null); + callback(null, directory); })); })); })); }; +EnrichIntegrationFromRemoteConfigStrategy.prototype._enrichWithDirectoryAccountModel = function (client, config, directory, callback) { + if (!directory) { + return callback(null, null); + } + + // Returns the callback immediately if there is an error. + // Continues processing if there isn't. + var stopIfError = function (process) { + return function (err, result) { + if (err) { + callback(err); + } else { + process(result); + } + } + }; + + var webConfig = config.web; + + // Map the registration fields defined in the account schema. + directory.getAccountSchema(stopIfError(function (accountSchema) { + accountSchema.getFields(stopIfError(function (fields) { + fields.each(function (field, next) { + if (field.name in webConfig.register.form.fields) { + webConfig.register.form.fields[field.name].required = field.required; + } + + next(); + }, function (err) { + callback(err, null); + }); + })); + })); +}; + EnrichIntegrationFromRemoteConfigStrategy.prototype.process = function (config, callback) { var tasks = []; @@ -228,7 +263,8 @@ EnrichIntegrationFromRemoteConfigStrategy.prototype.process = function (config, this._enrichWithOAuthPolicy.bind(this), this._enrichWithSocialProviders.bind(this, config), this._resolveDirectoryHref.bind(this), - this._enrichWithDirectoryPolicies.bind(this, client, config) + this._enrichWithDirectoryPolicies.bind(this, client, config), + this._enrichWithDirectoryAccountModel.bind(this, client, config) ]); } From 41c94fe78835b9563efac55c74a5a35ee66868cd Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 26 Sep 2016 18:45:47 -0700 Subject: [PATCH 2/4] Updating per latest changes to framework spec * Fields are null by default * Warnings if there are configuration mis-matches * Directory schema requirements override local disabling of requirements * Stub out tests --- ...richIntegrationFromRemoteConfigStrategy.js | 19 +++++++++++++++++-- lib/strings.js | 2 ++ ...IntegrationFromRemoteConfigStrategyTest.js | 7 +++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/lib/strategy/EnrichIntegrationFromRemoteConfigStrategy.js b/lib/strategy/EnrichIntegrationFromRemoteConfigStrategy.js index e5231c1..c3a5a6b 100644 --- a/lib/strategy/EnrichIntegrationFromRemoteConfigStrategy.js +++ b/lib/strategy/EnrichIntegrationFromRemoteConfigStrategy.js @@ -3,6 +3,7 @@ var async = require('async'); var extend = require('../helpers/clone-extend').extend; var strings = require('../strings'); +var util = require('util'); /** * Retrieves Stormpath settings from the API service, and ensures the local @@ -226,7 +227,7 @@ EnrichIntegrationFromRemoteConfigStrategy.prototype._enrichWithDirectoryAccountM } else { process(result); } - } + }; }; var webConfig = config.web; @@ -235,8 +236,22 @@ EnrichIntegrationFromRemoteConfigStrategy.prototype._enrichWithDirectoryAccountM directory.getAccountSchema(stopIfError(function (accountSchema) { accountSchema.getFields(stopIfError(function (fields) { fields.each(function (field, next) { + + var fieldToConfigure = webConfig.register.form.fields[field.name]; + if (field.name in webConfig.register.form.fields) { - webConfig.register.form.fields[field.name].required = field.required; + + var localValue = webConfig.register.form.fields[field.name].required; + var remoteValue = field.required; + + if (localValue === false && remoteValue === true){ + console.warn(util.format(strings.ACCOUNT_SCHEMA_REMOTE_FIELD_REQUIRED_WARNING,directory.name,field.name)); + fieldToConfigure.required = true; + } + + if (localValue === true && remoteValue === false){ + console.warn(util.format(strings.ACCOUNT_SCHEMA_LOCAL_FIELD_REQUIRED_WARNING,directory.name,field.name)); + } } next(); diff --git a/lib/strings.js b/lib/strings.js index 2d448ec..aa3261d 100644 --- a/lib/strings.js +++ b/lib/strings.js @@ -1,6 +1,8 @@ module.exports = { APP_HREF_NOT_FOUND: 'The provided application could not be found.\n\nThe provided application href was: %href%\n', APP_NAME_NOT_FOUND: 'The provided application could not be found.\n\nThe provided application name was: %name%\n', + ACCOUNT_SCHEMA_REMOTE_FIELD_REQUIRED_WARNING: 'The directory \'%s\' requires the \'%s\' account field. Local configuration has attempted to disable this requirement. The directory schema configuration will override the local configuration.', + ACCOUNT_SCHEMA_LOCAL_FIELD_REQUIRED_WARNING: 'The directory \'%s\' does not require the \'%s\' account field. Local configuration has required this field. Field enforcment will only happen locally.', CONFLICTING_AUTO_LOGIN_AND_EMAIL_VERIFICATION_CONFIG: 'Invalid configuration: stormpath.web.register.autoLogin is true, but the default account store of the specified application has the email verification workflow enabled. Auto login is only possible if email verification is disabled. Please disable this workflow on this application\'s default account store.', NO_ACCOUNT_STORES_MAPPED: 'No account stores are mapped to the specified application. Account stores are required for login and registration.', NO_DEFAULT_ACCOUNT_STORE_MAPPED: 'No default account store is mapped to the specified application. A default account store is required for registration.', diff --git a/test/strategy/EnrichIntegrationFromRemoteConfigStrategyTest.js b/test/strategy/EnrichIntegrationFromRemoteConfigStrategyTest.js index ef19f1e..e694d21 100644 --- a/test/strategy/EnrichIntegrationFromRemoteConfigStrategyTest.js +++ b/test/strategy/EnrichIntegrationFromRemoteConfigStrategyTest.js @@ -333,4 +333,11 @@ describe('EnrichIntegrationFromRemoteConfigStrategy', function () { }); }); }); + + describe.skip('when parsing the directory account schema', function() { + it('fields should still be required by remote config, even if disabled locally', function(done){ + }); + it('fields should still be required by remote config, even if disabled locally', function(done){ + }); + }); }); From 52865df7e19c17e4559ec4f8acb79103cab69e03 Mon Sep 17 00:00:00 2001 From: Robin Date: Thu, 3 Nov 2016 15:40:45 +0100 Subject: [PATCH 3/4] fix so that if registration required field is not set, it defaults to the remote value --- ...richIntegrationFromRemoteConfigStrategy.js | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/lib/strategy/EnrichIntegrationFromRemoteConfigStrategy.js b/lib/strategy/EnrichIntegrationFromRemoteConfigStrategy.js index c3a5a6b..1d9ad38 100644 --- a/lib/strategy/EnrichIntegrationFromRemoteConfigStrategy.js +++ b/lib/strategy/EnrichIntegrationFromRemoteConfigStrategy.js @@ -235,15 +235,19 @@ EnrichIntegrationFromRemoteConfigStrategy.prototype._enrichWithDirectoryAccountM // Map the registration fields defined in the account schema. directory.getAccountSchema(stopIfError(function (accountSchema) { accountSchema.getFields(stopIfError(function (fields) { - fields.each(function (field, next) { - - var fieldToConfigure = webConfig.register.form.fields[field.name]; + var registrationFields = webConfig.register.form.fields; - if (field.name in webConfig.register.form.fields) { + fields.each(function (field, next) { + var fieldToConfigure = registrationFields[field.name]; - var localValue = webConfig.register.form.fields[field.name].required; + if (fieldToConfigure) { + var localValue = registrationFields[field.name].required; var remoteValue = field.required; + if (localValue === null) { + fieldToConfigure.required = remoteValue; + } + if (localValue === false && remoteValue === true){ console.warn(util.format(strings.ACCOUNT_SCHEMA_REMOTE_FIELD_REQUIRED_WARNING,directory.name,field.name)); fieldToConfigure.required = true; @@ -256,6 +260,13 @@ EnrichIntegrationFromRemoteConfigStrategy.prototype._enrichWithDirectoryAccountM next(); }, function (err) { + // If a field is not configured, then don't require it. + for (var key in registrationFields) { + if (typeof registrationFields[key].required !== 'boolean') { + registrationFields[key].required = false; + } + } + callback(err, null); }); })); From 1cd049d1a397da5b4ffa878ecf3d96831a5b1d4d Mon Sep 17 00:00:00 2001 From: Robin Date: Thu, 3 Nov 2016 15:40:45 +0100 Subject: [PATCH 4/4] fix so that if registration required field is not set, it defaults to the remote value --- ...IntegrationFromRemoteConfigStrategyTest.js | 90 ++++++++++++++++++- 1 file changed, 87 insertions(+), 3 deletions(-) diff --git a/test/strategy/EnrichIntegrationFromRemoteConfigStrategyTest.js b/test/strategy/EnrichIntegrationFromRemoteConfigStrategyTest.js index e694d21..7f5bd57 100644 --- a/test/strategy/EnrichIntegrationFromRemoteConfigStrategyTest.js +++ b/test/strategy/EnrichIntegrationFromRemoteConfigStrategyTest.js @@ -334,10 +334,94 @@ describe('EnrichIntegrationFromRemoteConfigStrategy', function () { }); }); - describe.skip('when parsing the directory account schema', function() { - it('fields should still be required by remote config, even if disabled locally', function(done){ + describe('._enrichWithDirectoryAccountModel(client, config, directory, callback)', function() { + var testMethod; + var mockClient; + var mockConfig; + var mockDirectory; + var mockFields; + var sinonSandbox; + + beforeEach(function () { + sinonSandbox = sinon.sandbox.create(); + + testMethod = EnrichIntegrationFromRemoteConfigStrategy.prototype._enrichWithDirectoryAccountModel; + + mockClient = {}; + + mockConfig = { + web: { + register: { + form: { + fields: { + givenName: { + enabled: true, + required: false + }, + surname: { + enabled: true, + required: false + } + } + } + } + } + }; + + mockFields = { + items: [{ + name: 'givenName', + required: true + }, { + name: 'surname', + required: true + }], + each: function (iterator, onComplete) { + var self = this; + this.items.forEach(function (item, index) { + iterator(item, function () { + if (onComplete && index === self.items.length - 1) { + onComplete(); + } + }); + }); + } + }; + + mockDirectory = { + getAccountSchema: function (callback) { + callback(null, { + getFields: function (callback) { + callback(null, mockFields); + } + }); + } + }; + }); + + afterEach(function () { + sinonSandbox.restore(); }); - it('fields should still be required by remote config, even if disabled locally', function(done){ + + describe('when givenName and surname registration fields in config are not required', function () { + describe('but remote accountSchema has fields set to required', function () { + it('should warn and set fields in config as required', function (done) { + var testFormFields = mockConfig.web.register.form.fields; + var consoleMock = sinonSandbox.mock(console); + + consoleMock.expects('warn').twice(); + + testMethod(mockClient, mockConfig, mockDirectory, function (err) { + assert.equal(err, null); + assert.equal(testFormFields.givenName.required, true); + assert.equal(testFormFields.surname.required, true); + + consoleMock.verify(); + + done(); + }); + }); + }); }); }); });