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

bugfix(#512): converted user to API-compatible shape for AM user creation method #513

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
19 changes: 9 additions & 10 deletions lib/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ function toExternalUser(user) {
*/
function toInternalUser(user) {
for (var prop in user) {
if ( prop === 'roleTenantFilter' && user[prop] !== null ) {
if ( prop === 'roleTenantFilter' && user[prop] !== null && typeof(user[prop]) !== 'string' ) {
Copy link
Author

Choose a reason for hiding this comment

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

Added for backward compatibility, as the current workaround is to use API-compatible string when calling AM user creation method, e.g. roleTenantFilter: 'ECOM_USER:abcd_dev,abcd_stg'

// transform to string
var scopeFilters = [];
var groups = user[prop];
Expand Down Expand Up @@ -272,8 +272,8 @@ function createUser(orgID, user, token, callback) {
userState : "ENABLED"
});

// the payload
options['body'] = mergedUser;
// the payload, transform into internal payload (API properties and internal JSON structure)
options['body'] = toInternalUser(mergedUser);

// do the request
request(options, function (err, res, body) {
Expand Down Expand Up @@ -928,18 +928,17 @@ module.exports.cli = {
* @param {Function} callback callback function to call when called through the Javascript API
*/
create : function(org, user, mail, firstName, lastName, asJson, token, callback) {
// TODO: create deep clone not to mutate user argument
var userToCreate = Object.assign({}, user)
// respect user, login, firstName and lastName (if passed)
if (typeof(user) === 'undefined' || user === null) {
user = {};
}
if (typeof(mail) !== 'undefined' && mail !== null) {
user['mail'] = mail;
userToCreate['mail'] = mail;
}
if (typeof(firstName) !== 'undefined' && firstName !== null) {
user['firstName'] = firstName;
userToCreate['firstName'] = firstName;
}
if (typeof(lastName) !== 'undefined' && lastName !== null) {
user['lastName'] = lastName;
userToCreate['lastName'] = lastName;
}

libOrg.getOrg(org, token, function(err, foundOrg) {
Expand All @@ -956,7 +955,7 @@ module.exports.cli = {
}
return;
}
createUser(foundOrg['id'], user, token, function(err, newUser) {
createUser(foundOrg['id'], userToCreate, token, function(err, newUser) {
if (err) {
if (typeof callback !== 'undefined') {
callback(err, undefined);
Expand Down
59 changes: 42 additions & 17 deletions test/unit/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ var jsonStub = testbase.jsonLogStub;
var infoStub = testbase.infoLogStub;

describe('Tests for lib/user.js', function() {

var organization = 'myorg';
var user = proxyquire('../../lib/user', {
'request': requestStub,
'./auth': {
Expand All @@ -28,7 +28,7 @@ describe('Tests for lib/user.js', function() {
},
'./org': {
'getOrg' : function (id, undefined, callback) {
callback(undefined, { id : 'myorg' });
callback(undefined, { id : organization });
}
}
});
Expand All @@ -37,25 +37,25 @@ describe('Tests for lib/user.js', function() {
firstName : 'John',
lastName : 'Doe',
displayName : 'John Doe',
userState : 'ok',
userState : 'ENABLED',
roles : ['admin','expert','ECOM_ADMIN','ECOM_USER'],
roleTenantFilter : 'expert:here,there;ECOM_ADMIN:zzzz_stg;ECOM_USER:zzzz_prd',
primaryOrganization : 'doe org',
primaryOrganization : organization,
mail : 'john@doe.org',
organizations : ['doe org','other org'],
organizations : [organization],
externalNames : 'foo',
links : 'coding'
};
var cleanUserObj = {
firstName : 'John',
lastName : 'Doe',
displayName : 'John Doe',
userState : 'ok',
userState : 'ENABLED',
roles : ['admin','expert','ECOM_ADMIN','ECOM_USER'],
roleTenantFilter : {expert : ['here', 'there'],'bm-admin':['zzzz_stg'],'bm-user':['zzzz_prd']},
primaryOrganization : 'doe org',
primaryOrganization : organization,
mail : 'john@doe.org',
organizations : ['doe org','other org']
organizations : [organization]
};

var localUserObj = {
Expand All @@ -71,13 +71,37 @@ describe('Tests for lib/user.js', function() {
});

it('makes a post request', function() {
user.cli.create('myorg', undefined, 'john@doe.org', 'John', 'Doe', true);
user.cli.create(organization, undefined, 'john@doe.org', 'John', 'Doe', true);

const reqArgs = requestStub.getCall(0).args[0];
expect(reqArgs.uri).to.equal('https://am.host/dw/rest/v1/users');
expect(reqArgs.method).to.equal('POST');
});

it('makes request to create user with correct parameters', function() {
var user = proxyquire('../../lib/user', {
'request': function (opts, callback) {
callback(undefined, {statusCode: 200}, opts.body);
},
'./auth': {
'getToken' : () => 'mytoken',
'getAMHost' : () => 'am.host'
},
'./log': {
'json' : jsonStub
},
'./org': {
'getOrg' : function (id, undefined, callback) {
callback(undefined, { id : organization });
}
}
});
user.cli.create(organization, cleanUserObj, 'john@doe.org', 'John', 'Doe', true);

const logArgs = jsonStub.getCall(0).args;
expect(logArgs[0]).to.eql(cleanUserObj);
});

it('returns the created user', function() {
var user = proxyquire('../../lib/user', {
'request': function (opts, callback) {
Expand All @@ -96,13 +120,14 @@ describe('Tests for lib/user.js', function() {
}
}
});
user.cli.create('myorg', undefined, 'john@doe.org', 'John', 'Doe', true);
user.cli.create(organization, undefined, 'john@doe.org', 'John', 'Doe', true);

const logArgs = jsonStub.getCall(0).args;
expect(logArgs[0]).to.eql(cleanUserObj);
});
});


describe('cli.list function', function() {

beforeEach(function() {
Expand All @@ -122,11 +147,11 @@ describe('Tests for lib/user.js', function() {
},
'./org': {
'getOrg' : function (id, undefined, callback) {
callback(undefined, { id : 'myorg' });
callback(undefined, { id : organization });
}
}
});
user.cli.list('myorg', null, null, null, true, undefined);
user.cli.list(organization, null, null, null, true, undefined);

const reqArgs = requestStub.getCall(0).args[0];
expect(reqArgs.uri).to.equal('https://am.host/dw/rest/v1/users?page=0&size=25');
Expand All @@ -147,11 +172,11 @@ describe('Tests for lib/user.js', function() {
},
'./org': {
'getOrg' : function (id, undefined, callback) {
callback(undefined, { id : 'myorg' });
callback(undefined, { id : organization });
}
}
});
user.cli.list('myorg', null, null, null, true, undefined);
user.cli.list(organization, null, null, null, true, undefined);

const logArgs = jsonStub.getCall(0).args;
expect(logArgs[0]).to.eql([{id:1,firstName:'John'},{id:2,firstName:'Jane'}]);
Expand All @@ -171,11 +196,11 @@ describe('Tests for lib/user.js', function() {
},
'./org': {
'getOrg' : function (id, undefined, callback) {
callback(undefined, { id : 'myorg' });
callback(undefined, { id : organization });
}
}
});
user.cli.list('myorg', null, 'john@doe.org', null, true, undefined);
user.cli.list(organization, null, 'john@doe.org', null, true, undefined);

const logArgs = jsonStub.getCall(0).args;
expect(logArgs[0]).to.eql({id:1,firstName:'John'});
Expand Down Expand Up @@ -211,7 +236,7 @@ describe('Tests for lib/user.js', function() {
},
'./org': {
'getOrg' : function (id, undefined, callback) {
callback(undefined, { id : 'myorg' });
callback(undefined, { id : organization });
}
}
});
Expand Down