Skip to content

Feature: Validate conflicting autoLogin and verifyEmail flags #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 13, 2016
Merged

Feature: Validate conflicting autoLogin and verifyEmail flags #15

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5984128
add check so that auto login and email verification isn't set at the …
typerandom Dec 21, 2015
b870c3b
config.web.changePassword should be enabled when config.web.forgotPas…
timothyej Jan 13, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions lib/strategy/EnrichIntegrationFromRemoteConfigStrategy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ EnrichIntegrationFromRemoteConfigStrategy.prototype._enrichWithSocialProviders =
extend(remoteProvider, { enabled: true });
extend(localProvider, remoteProvider);
}

next();
});
} else {
Expand All @@ -114,10 +115,10 @@ EnrichIntegrationFromRemoteConfigStrategy.prototype._enrichWithSocialProviders =

// Finds and returns an Application's default Account Store (Directory)
// object. If one doesn't exist, nothing will be returned.
EnrichIntegrationFromRemoteConfigStrategy.prototype._resolveDirectoryHref = function (application, callback) {
EnrichIntegrationFromRemoteConfigStrategy.prototype._resolveDirectoryHref = function (app, callback) {
var outerScope = this;

application.getAccountStoreMappings(function(err, mappings) {
app.getAccountStoreMappings(function(err, mappings) {
if (err) {
return callback(err);
}
Expand Down Expand Up @@ -172,19 +173,29 @@ EnrichIntegrationFromRemoteConfigStrategy.prototype._enrichWithDirectoryPolicies

// Enrich config with with directory policies.
client.getDirectory(directoryHref, { expand: 'passwordPolicy,accountCreationPolicy' }, stopIfError(function (directory) {
var resetEmailStatusEnabled = isEnabled(directory.passwordPolicy.resetEmailStatus);

// Enrich config with account policies.
extend(config, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change intentional? That config.web.changePassword.enabled is not set to whatever config.web.forgotPassword.enabled is set to anymore?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@typerandom?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it is not. Thanks for noticing! Will fix that :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed it for you, and I added some tests for it too :) Please review!

web: {
forgotPassword: {
enabled: config.web.changePassword.enabled = isEnabled(directory.passwordPolicy.resetEmailStatus)
enabled: resetEmailStatusEnabled
},
changePassword: {
enabled: resetEmailStatusEnabled
},
verifyEmail: {
enabled: isEnabled(directory.accountCreationPolicy.verificationEmailStatus)
}
}
});

// Enrich config with password policies
// Validate that auto login and email verification aren't enabled at the same time.
if (config.web.register.autoLogin && config.web.verifyEmail.enabled) {
return callback(new Error(strings.CONFLICTING_AUTO_LOGIN_AND_EMAIL_VERIFICATION_CONFIG));
}

// Enrich config with password policies.
directory.getPasswordPolicy(stopIfError(function (policy) {
policy.getStrength(stopIfError(function (strength) {
// Remove the href property from the Strength Resource, we don't want
Expand Down
1 change: 1 addition & 0 deletions lib/strings.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

135 changes: 134 additions & 1 deletion test/strategy/EnrichIntegrationFromRemoteConfigStrategyTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,16 @@ var assert = common.assert;
var sinon = common.sinon;
var strings = common.strings;

var Directory = require('stormpath/lib/resource/Directory');
var Application = require('stormpath/lib/resource/Application');
var Collection = require('stormpath/lib/resource/CollectionResource');
var AccountStoreMapping = require('stormpath/lib/resource/AccountStoreMapping');

var strategy = require('../../lib/strategy');
var EnrichIntegrationFromRemoteConfigStrategy = strategy.EnrichIntegrationFromRemoteConfigStrategy;

var sandbox;

describe('EnrichIntegrationFromRemoteConfigStrategy', function () {
var client, testStrategy;

Expand All @@ -22,12 +26,14 @@ describe('EnrichIntegrationFromRemoteConfigStrategy', function () {
};

var mockDirectory = {
href: 'https://stormpath.mock/directories/stormpath',
accountCreationPolicy: {
verificationEmailStatus: 'ENABLED'
},
passwordPolicy: {
resetEmailStatus: 'ENABLED'
}
},
getPasswordPolicy: function () {}
};

function makeMockConfig(mockApplication) {
Expand All @@ -36,6 +42,15 @@ describe('EnrichIntegrationFromRemoteConfigStrategy', function () {
web: {
register: {
enabled: true
},
forgotPassword: {
enabled: true
},
changePassword: {
enabled: true
},
verifyEmail: {
enabled: true
}
}
};
Expand All @@ -53,6 +68,17 @@ describe('EnrichIntegrationFromRemoteConfigStrategy', function () {
return mockApplication;
}

function makeMockDirectory() {
var directory = new Directory({ href: 'http://mock.api/directories/mock' });

sinon.stub(directory, 'getProvider')
.yields(null, {
providerId: 'mock'
});

return directory;
}

before(function (done) {
client = new stormpath.Client({
skipRemoteConfig: true
Expand All @@ -73,6 +99,14 @@ describe('EnrichIntegrationFromRemoteConfigStrategy', function () {
});
});

beforeEach(function () {
sandbox = sinon.sandbox.create();
});

afterEach(function () {
sandbox.restore();
});

describe('when resolving an application', function () {
it('should error if invalid', function (done) {
testStrategy.process({ application: { href: '123' } }, function (err) {
Expand Down Expand Up @@ -109,6 +143,35 @@ describe('EnrichIntegrationFromRemoteConfigStrategy', function () {
done();
});
});

it('should error if both web.register.autoLogin and config.web.verifyEmail.enabled is set at same time', function (done) {
var mockAccountStoreMapping = new AccountStoreMapping({ accountStore: makeMockDirectory() });
var mockAccountStoreMappings = new Collection({ size: 1, items: [mockAccountStoreMapping] }, null, AccountStoreMapping);

mockAccountStoreMappings.detect = function (iter, complete) {
complete(mockAccountStoreMapping);
};

mockAccountStoreMappings.each = function (iter, complete) {
iter(mockAccountStoreMapping, function () {
complete();
});
};

sinon.stub(mockAccountStoreMapping, 'getAccountStore')
.yields(null, mockAccountStoreMapping.accountStore);

var mockApplication = makeMockApplication(null, mockAccountStoreMappings);
var mockConfig = makeMockConfig(mockApplication);

mockConfig.web.register.autoLogin = true;

testStrategy.process(mockConfig, function (err) {
assert.isNotNull(err);
assert.equal(err.message, strings.CONFLICTING_AUTO_LOGIN_AND_EMAIL_VERIFICATION_CONFIG);
done();
});
});
});

describe('when enriching', function () {
Expand Down Expand Up @@ -138,4 +201,74 @@ describe('EnrichIntegrationFromRemoteConfigStrategy', function () {
});
});
});

describe('_enrichWithDirectoryPolicies method', function () {
var enrichWithDirectoryPolicies;
var mockConfig;

beforeEach(function () {
var mockAccountStoreMappings = new Collection({ size: 0 });
var mockApplication = makeMockApplication(null, mockAccountStoreMappings);

var policy = {
getStrength: function () {}
};

enrichWithDirectoryPolicies = testStrategy._enrichWithDirectoryPolicies;
mockConfig = makeMockConfig(mockApplication);

sandbox.stub(mockDirectory, 'getPasswordPolicy').yields(null, policy);
sandbox.stub(policy, 'getStrength').yields(null, {});
});

describe('directory.passwordPolicy.resetEmailStatus property', function () {
describe('when "ENABLED"', function () {
beforeEach(function () {
mockDirectory.passwordPolicy.resetEmailStatus = 'ENABLED';
});

it('should set config.web.forgotPassword.enabled to true', function (done) {
mockConfig.web.forgotPassword.enabled = false;

enrichWithDirectoryPolicies(client, mockConfig, mockDirectory.href, function () {
assert.equal(mockConfig.web.forgotPassword.enabled, true);
done();
});
});

it('should set config.web.changePassword.enabled to true', function (done) {
mockConfig.web.changePassword.enabled = false;

enrichWithDirectoryPolicies(client, mockConfig, mockDirectory.href, function () {
assert.equal(mockConfig.web.changePassword.enabled, true);
done();
});
});
});

describe('when undefined', function () {
beforeEach(function () {
mockDirectory.passwordPolicy.resetEmailStatus = undefined;
});

it('should set config.web.forgotPassword.enabled to false', function (done) {
mockConfig.web.forgotPassword.enabled = true;

enrichWithDirectoryPolicies(client, mockConfig, mockDirectory.href, function () {
assert.equal(mockConfig.web.forgotPassword.enabled, false);
done();
});
});

it('should set config.web.changePassword.enabled to false', function (done) {
mockConfig.web.changePassword.enabled = true;

enrichWithDirectoryPolicies(client, mockConfig, mockDirectory.href, function () {
assert.equal(mockConfig.web.changePassword.enabled, false);
done();
});
});
});
});
});
});