Skip to content

Commit

Permalink
Fixes issue affecting the auth providers (#3211)
Browse files Browse the repository at this point in the history
* Reproduces error for issue 3208

* Make sure we don't override a default adapter with the options when validation function are not provided
  • Loading branch information
flovilmart authored and Arthur Cinader committed Dec 8, 2016
1 parent 81ebbb8 commit 4f46ece
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 22 deletions.
33 changes: 33 additions & 0 deletions spec/AuthenticationAdapters.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,13 @@ describe('AuthenticationProviers', function() {
expect(typeof authenticatonHandler.getValidatorForProvider).toBe('function');
}

function validateAuthenticationAdapter(authAdapter) {
expect(authAdapter).not.toBeUndefined();
if (!authAdapter) { return; }
expect(typeof authAdapter.validateAuthData).toBe('function');
expect(typeof authAdapter.validateAppId).toBe('function');
}

it('properly loads custom adapter', (done) => {
var validAuthData = {
id: 'hello',
Expand Down Expand Up @@ -279,4 +286,30 @@ describe('AuthenticationProviers', function() {
done();
})
});

it('properly loads a default adapter with options', () => {
const options = {
facebook: {
appIds: ['a', 'b']
}
};
const { adapter, appIds, providerOptions } = authenticationLoader.loadAuthAdapter('facebook', options);
validateAuthenticationAdapter(adapter);
expect(appIds).toEqual(['a', 'b']);
expect(providerOptions).toEqual(options.facebook);
});

it('properly loads a custom adapter with options', () => {
const options = {
custom: {
validateAppId: () => {},
validateAuthData: () => {},
appIds: ['a', 'b']
}
};
const { adapter, appIds, providerOptions } = authenticationLoader.loadAuthAdapter('custom', options);
validateAuthenticationAdapter(adapter);
expect(appIds).toEqual(['a', 'b']);
expect(providerOptions).toEqual(options.custom);
});
});
60 changes: 38 additions & 22 deletions src/Adapters/Auth/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,36 @@ function authDataValidator(adapter, appIds, options) {
}
}

function loadAuthAdapter(provider, authOptions) {
const defaultAdapter = providers[provider];
const adapter = Object.assign({}, defaultAdapter);
const providerOptions = authOptions[provider];

if (!defaultAdapter && !providerOptions) {
return;
}

const appIds = providerOptions ? providerOptions.appIds : undefined;

// Try the configuration methods
if (providerOptions) {
const optionalAdapter = loadAdapter(providerOptions, undefined, providerOptions);
if (optionalAdapter) {
['validateAuthData', 'validateAppId'].forEach((key) => {
if (optionalAdapter[key]) {
adapter[key] = optionalAdapter[key];
}
});
}
}

if (!adapter.validateAuthData || !adapter.validateAppId) {
return;
}

return {adapter, appIds, providerOptions};
}

module.exports = function(authOptions = {}, enableAnonymousUsers = true) {
let _enableAnonymousUsers = enableAnonymousUsers;
const setEnableAnonymousUsers = function(enable) {
Expand All @@ -67,33 +97,19 @@ module.exports = function(authOptions = {}, enableAnonymousUsers = true) {
return;
}

const defaultAdapter = providers[provider];
let adapter = defaultAdapter;
const providerOptions = authOptions[provider];

if (!defaultAdapter && !providerOptions) {
return;
}

const appIds = providerOptions ? providerOptions.appIds : undefined;

// Try the configuration methods
if (providerOptions) {
const optionalAdapter = loadAdapter(providerOptions, undefined, providerOptions);
if (optionalAdapter) {
adapter = optionalAdapter;
}
}

if (!adapter.validateAuthData || !adapter.validateAppId) {
return;
}
const {
adapter,
appIds,
providerOptions
} = loadAuthAdapter(provider, authOptions);

return authDataValidator(adapter, appIds, providerOptions);
}

return Object.freeze({
getValidatorForProvider,
setEnableAnonymousUsers,
setEnableAnonymousUsers
})
}

module.exports.loadAuthAdapter = loadAuthAdapter;

0 comments on commit 4f46ece

Please sign in to comment.