Skip to content

Fixed input sanitization for admin route #370

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
petruki merged 2 commits into from
Jan 29, 2023
Merged

Fixed input sanitization for admin route #370

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
63179b8
Fixed input sanitization for admin route
petruki Jan 29, 2023
16dbd6a
Fixed tests after jest update - Bumped libs
petruki Jan 29, 2023
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
12 changes: 6 additions & 6 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,20 @@
],
"license": "MIT",
"dependencies": {
"axios": "^1.2.3",
"axios": "^1.2.6",
"bcryptjs": "^2.4.3",
"cors": "^2.8.5",
"express": "^4.18.2",
"express-basic-auth": "^1.2.1",
"express-graphql": "^0.12.0",
"express-validator": "^6.14.2",
"express-validator": "^6.14.3",
"graphql": "^15.8.0",
"graphql-tag": "^2.12.6",
"helmet": "^6.0.1",
"jsonwebtoken": "^9.0.0",
"moment": "^2.29.4",
"mongodb": "^4.13.0",
"mongoose": "^6.8.4",
"mongoose": "^6.9.0",
"pino": "^8.8.0",
"pino-pretty": "^9.1.1",
"swagger-ui-express": "^4.6.0",
Expand All @@ -52,11 +52,11 @@
"@babel/node": "^7.20.7",
"@babel/preset-env": "^7.20.2",
"@babel/register": "^7.18.9",
"babel-jest": "^29.3.1",
"babel-jest": "^29.4.1",
"babel-polyfill": "^6.26.0",
"env-cmd": "^10.1.0",
"eslint": "^8.31.0",
"jest": "^29.3.1",
"eslint": "^8.33.0",
"jest": "^29.4.1",
"jest-sonar-reporter": "^2.0.0",
"node-notifier": "^10.0.1",
"nodemon": "^2.0.20",
Expand Down
4 changes: 3 additions & 1 deletion src/routers/admin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,9 @@ router.get('/admin/collaboration', auth, async (req, res) => {
res.send(Array.from(new Set(domains)));
});

router.get('/admin/:id', auth, async (req, res) => {
router.get('/admin/:id', auth, [
check('id').isMongoId()
], validate, async (req, res) => {
try {
const admin = await Services.getAdminById(req.params.id);
res.send(admin);
Expand Down
61 changes: 38 additions & 23 deletions tests/admin.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('Testing Admin insertion', () => {
}).expect(201);

// DB validation - document created
const admin = await Admin.findById(response.body.admin._id).lean();
const admin = await Admin.findById(response.body.admin._id).lean().exec();
expect(admin).not.toBeNull();

//used at: ADMIN_SUITE - Should confirm access to a new Admin
Expand Down Expand Up @@ -96,7 +96,7 @@ describe('Testing Admin insertion', () => {

test('ADMIN_SUITE - Should NOT login before access confirmation sent via Email', async () => {
// given
let admin = await Admin.findById(signedupUser).lean();
let admin = await Admin.findById(signedupUser).lean().exec();
expect(admin).not.toBeNull();
expect(admin.active).toEqual(false);

Expand All @@ -111,7 +111,7 @@ describe('Testing Admin insertion', () => {

test('ADMIN_SUITE - Should confirm access to a new Admin', async () => {
// given
let admin = await Admin.findById(signedupUser).lean();
let admin = await Admin.findById(signedupUser).lean().exec();
expect(admin).not.toBeNull();
expect(admin.active).toEqual(false);

Expand All @@ -121,13 +121,13 @@ describe('Testing Admin insertion', () => {
.send().expect(201);

// DB validation - document updated
admin = await Admin.findById(signedupUser).lean();
admin = await Admin.findById(signedupUser).lean().exec();
expect(admin.active).toEqual(true);
});

test('ADMIN_SUITE - Should login after access confirmation', async () => {
// given
let admin = await Admin.findById(signedupUser).lean();
let admin = await Admin.findById(signedupUser).lean().exec();
expect(admin).not.toBeNull();
expect(admin.active).toEqual(true);

Expand Down Expand Up @@ -155,7 +155,7 @@ describe('Testing Admin insertion', () => {
axiosPostStub.returns(Promise.resolve(mockedRecaptchaResponse));

// test
let admin = await Admin.findOne({ email: 'new_admin@mail.com', active: true }).lean();
let admin = await Admin.findOne({ email: 'new_admin@mail.com', active: true }).lean().exec();
expect(admin).not.toBeNull();
expect(admin.code).toBeNull();

Expand All @@ -166,7 +166,7 @@ describe('Testing Admin insertion', () => {
}).expect(200);

// DB validation - document obtained
admin = await Admin.findOne({ email: 'new_admin@mail.com', active: true }).lean();
admin = await Admin.findOne({ email: 'new_admin@mail.com', active: true }).lean().exec();
expect(admin).not.toBeNull();
expect(admin.code).not.toBeNull();

Expand All @@ -183,7 +183,7 @@ describe('Testing Admin insertion', () => {
axiosPostStub.returns(Promise.resolve(mockedRecaptchaResponse));

// test
let admin = await Admin.findOne({ email: 'new_admin@mail.com', active: true }).lean();
let admin = await Admin.findOne({ email: 'new_admin@mail.com', active: true }).lean().exec();
expect(admin).not.toBeNull();
expect(admin.code).not.toBeNull();

Expand Down Expand Up @@ -302,7 +302,7 @@ describe('Testing Admin insertion', () => {
.send().expect(201);

// DB validation - document created
const admin = await Admin.findById(response.body.admin._id).lean();
const admin = await Admin.findById(response.body.admin._id).lean().exec();
expect(admin).not.toBeNull();
expect(admin._gitid).toEqual('123456789');

Expand All @@ -326,7 +326,7 @@ describe('Testing Admin insertion', () => {
}
};

var bodyFormData = new URLSearchParams();
const bodyFormData = new URLSearchParams();
bodyFormData.set('grant_type', 'authorization_code');
bodyFormData.set('code', 'BITBUCKET_CODE');

Expand All @@ -340,7 +340,7 @@ describe('Testing Admin insertion', () => {
.send().expect(201);

// DB validation - document created
const admin = await Admin.findById(response.body.admin._id).lean();
const admin = await Admin.findById(response.body.admin._id).lean().exec();
expect(admin).not.toBeNull();
expect(admin._bitbucketid).toEqual('123456789');

Expand Down Expand Up @@ -410,7 +410,7 @@ describe('Testing Admin insertion', () => {
// given
const mockedTokenData = { data: { access_token: 'MOCKED_TOKEN' } };

var bodyFormData = new URLSearchParams();
const bodyFormData = new URLSearchParams();
bodyFormData.set('grant_type', 'authorization_code');
bodyFormData.set('code', 'BITBUCKET_CODE');

Expand Down Expand Up @@ -507,7 +507,7 @@ describe('Testing Admin insertion', () => {
expect(refreshToken).not.toBeNull();

//DB validation
let admin = await Admin.findById(adminAccount._id).lean();
let admin = await Admin.findById(adminAccount._id).lean().exec();
expect(admin.token).toEqual(Admin.extractTokenPart(token));

await new Promise(resolve => setTimeout(resolve, 1000));
Expand Down Expand Up @@ -648,7 +648,7 @@ describe('Testing Admin login and fetch', () => {
password: adminAccount.password
}).expect(200);

const admin = await Admin.findById(adminAccountId).lean();
const admin = await Admin.findById(adminAccountId).lean().exec();
const token = response.body.jwt.token;
expect(Admin.extractTokenPart(token)).toBe(admin.token);
});
Expand Down Expand Up @@ -736,7 +736,22 @@ describe('Testing Admin login and fetch', () => {
.get('/admin/INVALID_ID')
.set('Authorization', `Bearer ${responseLogin.body.jwt.token}`)
.send()
.expect(400);
.expect(422);
});

test('ADMIN_SUITE - Should NOT get admin profile given unexisting Admin ID', async () => {
const responseLogin = await request(app)
.post('/admin/login')
.send({
email: adminMasterAccount.email,
password: adminMasterAccount.password
}).expect(200);

await request(app)
.get(`/admin/${new mongoose.Types.ObjectId()}`)
.set('Authorization', `Bearer ${responseLogin.body.jwt.token}`)
.send()
.expect(404);
});

test('ADMIN_SUITE - Should not get profile for unauthenticated admin', async () => {
Expand All @@ -761,7 +776,7 @@ describe('Testing Admin login and fetch', () => {
name: 'Updated Name'
})
.expect(200);
let admin = await Admin.findById(adminMasterAccountId).lean();
let admin = await Admin.findById(adminMasterAccountId).lean().exec();
expect(admin.name).toEqual('Updated Name');

// Validating regular Admin credential
Expand All @@ -780,7 +795,7 @@ describe('Testing Admin login and fetch', () => {
})
.expect(200);

admin = await Admin.findById(adminAccountId).lean();
admin = await Admin.findById(adminAccountId).lean().exec();
expect(admin.name).toEqual('Updated Name');
});

Expand Down Expand Up @@ -815,7 +830,7 @@ describe('Testing Admin login and fetch', () => {
.send()
.expect(200);

const admin = await Admin.findById(adminMasterAccountId).lean();
const admin = await Admin.findById(adminMasterAccountId).lean().exec();
expect(admin.token).toBeNull();
});
});
Expand Down Expand Up @@ -859,7 +874,7 @@ describe('Testing Admin logout', () => {
.send()
.expect(200);

const admin = await Admin.findById(adminMasterAccountId).lean();
const admin = await Admin.findById(adminMasterAccountId).lean().exec();
expect(admin).toBeNull();
});
});
Expand Down Expand Up @@ -970,7 +985,7 @@ describe('Testing Admin collaboration endpoint', () => {
}).expect(200);

//verify
let teams = await Team.find({ members: adminMasterAccountId }).lean();
let teams = await Team.find({ members: adminMasterAccountId }).lean().exec();
teams.forEach(team => {
expect(team.members[0]).toEqual(adminMasterAccountId);
});
Expand All @@ -981,7 +996,7 @@ describe('Testing Admin collaboration endpoint', () => {
.set('Authorization', `Bearer ${responseLogin.body.jwt.token}`)
.send().expect(200);

teams = await Team.find({ members: adminMasterAccountId }).lean();
teams = await Team.find({ members: adminMasterAccountId }).lean().exec();
teams.forEach(team => {
expect(team.members[0]).toBeNull();
});
Expand Down Expand Up @@ -1037,7 +1052,7 @@ describe('Testing Admin collaboration endpoint', () => {
}).expect(200);

//verify
let teams = await Team.find({ members: adminMasterAccountId }).lean();
let teams = await Team.find({ members: adminMasterAccountId }).lean().exec();
teams.forEach(team => {
expect(team.members[0]).toEqual(adminMasterAccountId);
});
Expand All @@ -1054,7 +1069,7 @@ describe('Testing Admin collaboration endpoint', () => {
.set('Authorization', `Bearer ${responseLogin.body.jwt.token}`)
.send().expect(200);

teams = await Team.find({ members: adminMasterAccountId });
teams = await Team.find({ members: adminMasterAccountId }).exec();
teams.forEach(team => {
expect(team.members[0]).toBeNull();
});
Expand Down
4 changes: 2 additions & 2 deletions tests/slack.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ describe('Slack Settings', () => {
}).expect(200);

slackDb = await Services.getSlack({ id: slack._id });
expect(response.body).toMatchObject(slackDb.settings);
expect(response.body).toEqual(slackDb.settings);
expect(slackDb.settings.ignored_environments).toEqual(
expect.arrayContaining(['dev', 'dev1']),
);
Expand All @@ -503,7 +503,7 @@ describe('Slack Settings', () => {
}).expect(200);

slackDb = await Services.getSlack({ id: slack._id });
expect(response.body).toMatchObject(slackDb.settings);
expect(response.body).toEqual(slackDb.settings);
expect(slackDb.settings.frozen_environments).toEqual(
expect.arrayContaining(['staging', 'staging1']),
);
Expand Down