Skip to content
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.

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

Support new minimum password length parameter #1472

Merged
merged 4 commits into from
Aug 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
},
"dependencies": {
"auth0-js": "^9.7.3",
"auth0-password-policies": "auth0/auth0-password-policies#v1.0.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd trust more on password-sheriff than this repo. Sheriff already defines them here: https://github.com/auth0/password-sheriff/blob/4a17c9cba614ed8b75affc7c53ff0d4ba31a5b30/index.js. If this is not possible, then please copy that file and keep it here on this repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

our change password widget also uses this repo. This is fine

"blueimp-md5": "2.3.1",
"fbjs": "^0.3.1",
"immutable": "^3.7.3",
Expand Down
61 changes: 61 additions & 0 deletions src/__tests__/core/client/__snapshots__/index.test.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`core/client/index initClient loads password policy 'excellent' correctly with a password_complexity_options option 1`] = `
Object {
"minLength": 4,
}
`;

exports[`core/client/index initClient loads password policy 'excellent' correctly without a password_complexity_options option 1`] = `
Object {
"minLength": 10,
}
`;

exports[`core/client/index initClient loads password policy 'fair' correctly with a password_complexity_options option 1`] = `
Object {
"minLength": 4,
}
`;

exports[`core/client/index initClient loads password policy 'fair' correctly without a password_complexity_options option 1`] = `
Object {
"minLength": 8,
}
`;

exports[`core/client/index initClient loads password policy 'good' correctly with a password_complexity_options option 1`] = `
Object {
"minLength": 4,
}
`;

exports[`core/client/index initClient loads password policy 'good' correctly without a password_complexity_options option 1`] = `
Object {
"minLength": 8,
}
`;

exports[`core/client/index initClient loads password policy 'low' correctly with a password_complexity_options option 1`] = `
Object {
"minLength": 4,
}
`;

exports[`core/client/index initClient loads password policy 'low' correctly without a password_complexity_options option 1`] = `
Object {
"minLength": 6,
}
`;

exports[`core/client/index initClient loads password policy 'none' correctly with a password_complexity_options option 1`] = `
Object {
"minLength": 4,
}
`;

exports[`core/client/index initClient loads password policy 'none' correctly without a password_complexity_options option 1`] = `
Object {
"minLength": 1,
}
`;
44 changes: 44 additions & 0 deletions src/__tests__/core/client/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import Immutable from 'immutable';
import { initClient } from '../../../core/client';

describe('core/client/index', () => {
describe('initClient', () => {
['none', 'low', 'fair', 'good', 'excellent'].forEach(policy => {
it(`loads password policy '${policy}' correctly without a password_complexity_options option`, () => {
const client = {
strategies: [
{
name: 'auth0',
connections: [
{
name: 'Username-Password-Authentication',
passwordPolicy: policy
}
]
}
]
};
const result = initClient(Immutable.fromJS({}), client).toJS();
expect(result.client.connections.database[0].passwordPolicy.length).toMatchSnapshot();
});
it(`loads password policy '${policy}' correctly with a password_complexity_options option`, () => {
const client = {
strategies: [
{
name: 'auth0',
connections: [
{
name: 'Username-Password-Authentication',
passwordPolicy: policy,
password_complexity_options: { min_length: 4 }
}
]
}
]
};
const result = initClient(Immutable.fromJS({}), client).toJS();
expect(result.client.connections.database[0].passwordPolicy.length).toMatchSnapshot();
});
});
});
});
26 changes: 26 additions & 0 deletions src/__tests__/field/password.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import Immutable from 'immutable';

describe('field/password', () => {
let passwordField;
beforeEach(() => {
jest.resetModules();
jest.mock('password-sheriff/lib/policy');
passwordField = require('field/password');
});
describe('validatePassword()', () => {
it(`returns true when there is no policy`, () => {
const value = passwordField.validatePassword('the-password');
expect(value).toBe(true);
});
it(`validates password correctly when there is a policy`, () => {
const model = {
toJS: jest.fn()
};
passwordField.validatePassword('the-password', model);
const { mock } = require('password-sheriff/lib/policy').prototype.check;
expect(mock.calls.length).toBe(1);
expect(mock.calls[0][0]).toBe('the-password');
expect(model.toJS).toHaveBeenCalledTimes(1);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`PasswordStrength validatePassword() validates password correctly with invalid password 1`] = `"<div class=\\"auth0-lock-password-strength animated fadeIn\\"><ul><li class=\\"\\"><span>At least 20 characters in length</span><!-- react-empty: 5 --></li></ul></div>"`;
25 changes: 25 additions & 0 deletions src/__tests__/ui/input/password/password_strength.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import React from 'react';
import { mount } from 'enzyme';
import PasswordStrength from '../../../../ui/input/password/password_strength';

describe('PasswordStrength', () => {
beforeEach(() => {
jest.resetModules();
});
describe('validatePassword()', () => {
it(`validates password correctly with invalid password`, () => {
const policy = {
toJS: () => ({
length: {
minLength: 20
}
})
};
const messages = { foo: 'the-message' };
const wrapper = mount(
<PasswordStrength policy={policy} password="the-password" messages={messages} />
);
expect(wrapper.html()).toMatchSnapshot();
});
});
});
10 changes: 9 additions & 1 deletion src/core/client/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import Immutable, { List, Map } from 'immutable';
import passwordPolicies from 'auth0-password-policies';

import { dataFns } from '../../utils/data_utils';
// TODO: this module should depend from social stuff
import { STRATEGIES as SOCIAL_STRATEGIES } from '../../connection/social/index';
Expand Down Expand Up @@ -112,7 +114,13 @@ function formatClientConnection(connectionType, strategyName, connection) {
};

if (connectionType === 'database') {
result.passwordPolicy = connection.passwordPolicy || 'none';
result.passwordPolicy = passwordPolicies[connection.passwordPolicy || 'none'];
if (
connection.password_complexity_options &&
connection.password_complexity_options.min_length
) {
result.passwordPolicy.length.minLength = connection.password_complexity_options.min_length;
}
result.allowSignup = typeof connection.showSignup === 'boolean' ? connection.showSignup : true;
result.allowForgot = typeof connection.showForgot === 'boolean' ? connection.showForgot : true;
result.requireUsername =
Expand Down
7 changes: 5 additions & 2 deletions src/field/password.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import createPolicy from 'password-sheriff';
import PasswordPolicy from 'password-sheriff/lib/policy';
import { setField } from './index';

export function validatePassword(password, policy) {
return createPolicy(policy).check(password);
if (!policy) {
return true;
}
return new PasswordPolicy(policy.toJS()).check(password);
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be safer, e.g. checking that if policy == undefined then none policy is still applied? https://github.com/auth0/password-sheriff/blob/master/index.js#L57 Now it will work since none is just length > 0 but if that changes this code will need to be changed as well. Let's reference this to the none policy instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

policy is only undefined when it's a login, which we don't validate passwords.

}

export function setPassword(m, password, policy) {
Expand Down
2 changes: 1 addition & 1 deletion src/field/password/password_pane.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ PasswordPane.propTypes = {
lock: PropTypes.object.isRequired,
onChange: PropTypes.func,
placeholder: PropTypes.string.isRequired,
policy: PropTypes.string,
policy: PropTypes.object,
strengthMessages: PropTypes.object,
hidden: PropTypes.bool
};
6 changes: 3 additions & 3 deletions src/ui/input/password/password_strength.jsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import PropTypes from 'prop-types';
import React from 'react';
import createPolicy from 'password-sheriff';
import PasswordPolicy from 'password-sheriff/lib/policy';
import util from 'util';

export default class PasswordStrength extends React.Component {
render() {
const { password, policy, messages } = this.props;
const analysis = createPolicy(policy).missing(password);
const analysis = new PasswordPolicy(policy.toJS()).missing(password);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the same applies here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this only runs on signup, so we're fine

// TODO: add a component for fadeIn / fadeOut animations?
const className =
'auth0-lock-password-strength animated ' + (!analysis.verified ? 'fadeIn' : 'fadeOut');
Expand Down Expand Up @@ -39,7 +39,7 @@ export default class PasswordStrength extends React.Component {
PasswordStrength.propTypes = {
messages: PropTypes.object.isRequired,
password: PropTypes.string.isRequired,
policy: PropTypes.oneOf(['none', 'low', 'fair', 'good', 'excellent']).isRequired
policy: PropTypes.object.isRequired
};

PasswordStrength.defaultProps = {
Expand Down
2 changes: 1 addition & 1 deletion src/ui/input/password_input.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export default class PasswordInput extends React.Component {
isValid: PropTypes.bool.isRequired,
onChange: PropTypes.func.isRequired,
placeholder: PropTypes.string,
policy: PropTypes.string,
policy: PropTypes.object,
strengthMessages: PropTypes.object,
value: PropTypes.string.isRequired,
showPassword: PropTypes.bool.isRequired,
Expand Down
6 changes: 6 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,12 @@ auth0-js@^9.7.3:
url-join "^1.1.0"
winchan "^0.2.0"

auth0-password-policies@auth0/auth0-password-policies#v1.0.1:
version "1.0.1"
resolved "https://codeload.github.com/auth0/auth0-password-policies/tar.gz/0b3da9b369b122a8b294d496143abfd33311e6b2"
dependencies:
password-sheriff "^1.1.0"

autoprefixer-stylus@^0.9.4:
version "0.9.4"
resolved "https://registry.yarnpkg.com/autoprefixer-stylus/-/autoprefixer-stylus-0.9.4.tgz#760914695dce321c9980b490d81a4385c08d914d"
Expand Down