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

Fix SSO screen not showing in some cases #1415

Merged
merged 2 commits into from
Jun 21, 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
8 changes: 7 additions & 1 deletion src/__tests__/connection/database/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,15 @@ describe('databaseUsernameValue', () => {
describe('for database connection with username required', () => {
const model = getModel('user@contoso.com', 'user', true);

it('should get the username', () => {
it('should get the username when `emailFirst` is not set', () => {
expect(databaseUsernameValue(model)).toEqual('user');
});
it('should get the username when `emailFirst` is false', () => {
expect(databaseUsernameValue(model, { emailFirst: false })).toEqual('user');
});
it('should get the email when `emailFirst` is true', () => {
expect(databaseUsernameValue(model, { emailFirst: true })).toEqual('user@contoso.com');
});

describe('and only email address is filled in', () => {
const model = getModel('user@contoso.com', null, true);
Expand Down
5 changes: 4 additions & 1 deletion src/__tests__/connection/database/reset_password.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ describe('ResetPasswordScreen', () => {
jest.resetModules();

jest.mock('connection/database/index', () => ({
databaseUsernameValue: () => 'foo@test.com'
databaseUsernameValue: (model, options) => {
expect(options.emailFirst).toBe(true);
return 'foo@test.com';
}
}));

jest.mock('connection/enterprise', () => ({
Expand Down
30 changes: 26 additions & 4 deletions src/__tests__/engine/classic/sign_up_screen.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,24 @@ jest.mock('connection/enterprise/single_sign_on_notice', () =>
mockComponent('single_sign_on_notice')
);

const getComponent = () => {
const getScreen = () => {
const SignUpScreen = require('engine/classic/sign_up_screen').default;
const screen = new SignUpScreen();
return screen.render();
return new SignUpScreen();
};

const getComponent = () => {
return getScreen().render();
};

describe('SignUpScreen', () => {
beforeEach(() => {
jest.resetModules();

jest.mock('connection/database/index', () => ({
databaseUsernameValue: (model, options) => {
expect(options.emailFirst).toBe(true);
return 'foo@bar.com';
},
termsAccepted: () => true,
hasScreen: () => false,
mustAcceptTerms: () => false
Expand All @@ -34,7 +41,10 @@ describe('SignUpScreen', () => {
}));
jest.mock('engine/classic', () => ({
hasOnlyClassicConnections: () => false,
isSSOEnabled: () => false,
isSSOEnabled: (model, options) => {
expect(options.emailFirst).toBe(true);
return false;
},
useBigSocialButtons: () => false
}));
jest.mock('core/signed_in_confirmation', () => ({
Expand All @@ -48,6 +58,10 @@ describe('SignUpScreen', () => {
renderOptionSelection: () => false
}));

jest.mock('connection/enterprise', () => ({
isHRDDomain: () => false
}));

jest.mock('connection/enterprise/actions', () => ({
logIn: jest.fn()
}));
Expand Down Expand Up @@ -113,4 +127,12 @@ describe('SignUpScreen', () => {
expectComponent(<Component {...defaultProps} />).toMatchSnapshot();
});
});
describe('on Submit, uses `options.emailFirst=true` and', () => {
it('calls signup', () => {
const screen = getScreen();
screen.submitHandler()();
const { mock } = require('connection/database/actions').signUp;
expect(mock.calls.length).toBe(1);
});
});
});
6 changes: 4 additions & 2 deletions src/connection/database/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -353,12 +353,14 @@ export function databaseLogInWithEmail(m) {
return databaseUsernameStyle(m) === 'email';
}

export function databaseUsernameValue(m) {
export function databaseUsernameValue(m, options = {}) {
const isEmailOnly = databaseLogInWithEmail(m);
if (isEmailOnly) {
return getFieldValue(m, 'email');
}

if (options.emailFirst) {
return getFieldValue(m, 'email') || getFieldValue(m, 'username');
}
return getFieldValue(m, 'username') || getFieldValue(m, 'email');
}

Expand Down
2 changes: 1 addition & 1 deletion src/connection/database/reset_password.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export default class ResetPassword extends Screen {
isSubmitDisabled(m) {
const tryingToResetPasswordWithEnterpriseEmail = isEnterpriseDomain(
m,
databaseUsernameValue(m)
databaseUsernameValue(m, { emailFirst: true })
);
if (tryingToResetPasswordWithEnterpriseEmail) {
swap(
Expand Down
4 changes: 2 additions & 2 deletions src/engine/classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ import { hasError, isDone, isSuccess } from '../sync';
import { getFieldValue } from '../field/index';
import { swap, updateEntity } from '../store/index';

export function isSSOEnabled(m) {
return matchesEnterpriseConnection(m, databaseUsernameValue(m));
export function isSSOEnabled(m, options) {
return matchesEnterpriseConnection(m, databaseUsernameValue(m, options));
}

export function matchesEnterpriseConnection(m, usernameValue) {
Expand Down
9 changes: 5 additions & 4 deletions src/engine/classic/sign_up_screen.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import LoginSignUpTabs from '../../connection/database/login_sign_up_tabs';
import SingleSignOnNotice from '../../connection/enterprise/single_sign_on_notice';

const Component = ({ i18n, model }) => {
const sso = isSSOEnabled(model) && hasScreen(model, 'login');
const sso = isSSOEnabled(model, { emailFirst: true }) && hasScreen(model, 'login');
const ssoNotice = sso && <SingleSignOnNotice>{i18n.str('ssoEnabled')}</SingleSignOnNotice>;

const tabs = !sso &&
Expand Down Expand Up @@ -87,10 +87,11 @@ export default class SignUp extends Screen {

submitHandler(m) {
if (hasOnlyClassicConnections(m, 'social')) return null;
if (isHRDDomain(m, databaseUsernameValue(m))) {
return id => startHRD(id, databaseUsernameValue(m));
const username = databaseUsernameValue(m, { emailFirst: true });
if (isHRDDomain(m, username)) {
return id => startHRD(id, username);
}
if (isSSOEnabled(m)) return enterpriseLogIn;
if (isSSOEnabled(m, { emailFirst: true })) return enterpriseLogIn;
return signUp;
}

Expand Down