From 49c73cb302ef19dbc5b2ca65bd78c16076373d41 Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Fri, 8 Jun 2018 09:37:20 +0200 Subject: [PATCH 1/6] :bug: Validate input for update notification settings --- .../NotificationSettingsController.ts | 8 +++++-- .../TestNotificationSettingsController.ts | 24 ++++++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/api/src/controllers/NotificationSettingsController.ts b/api/src/controllers/NotificationSettingsController.ts index f66cb20d5..b0b8f1d21 100644 --- a/api/src/controllers/NotificationSettingsController.ts +++ b/api/src/controllers/NotificationSettingsController.ts @@ -1,6 +1,10 @@ import {Authorized, BadRequestError, Body, Get, JsonController, Param, Post, Put, UseBefore} from 'routing-controllers'; import passportJwtMiddleware from '../security/passportJwtMiddleware'; -import {API_NOTIFICATION_TYPE_ALL_CHANGES, INotificationSettingsModel, NotificationSettings} from '../models/NotificationSettings'; +import { + API_NOTIFICATION_TYPE_ALL_CHANGES, + INotificationSettingsModel, + NotificationSettings +} from '../models/NotificationSettings'; import {INotificationSettings} from '../../../shared/models/INotificationSettings'; @JsonController('/notificationSettings') @@ -81,7 +85,7 @@ export class NotificationSettingsController { @Authorized(['student', 'teacher', 'admin']) @Put('/:id') async updateNotificationSettings(@Param('id') id: string, @Body() notificationSettings: INotificationSettings) { - if (!notificationSettings) { + if (!notificationSettings.course || !notificationSettings.user) { throw new BadRequestError('notification needs fields course and user'); } const settings: INotificationSettingsModel = diff --git a/api/test/controllers/TestNotificationSettingsController.ts b/api/test/controllers/TestNotificationSettingsController.ts index 6f6a4d7c2..055263436 100644 --- a/api/test/controllers/TestNotificationSettingsController.ts +++ b/api/test/controllers/TestNotificationSettingsController.ts @@ -3,7 +3,11 @@ import {Server} from '../../src/server'; import {FixtureLoader} from '../../fixtures/FixtureLoader'; import {FixtureUtils} from '../../fixtures/FixtureUtils'; import {JwtUtils} from '../../src/security/JwtUtils'; -import {API_NOTIFICATION_TYPE_ALL_CHANGES, API_NOTIFICATION_TYPE_NONE, NotificationSettings} from '../../src/models/NotificationSettings'; +import { + API_NOTIFICATION_TYPE_ALL_CHANGES, + API_NOTIFICATION_TYPE_NONE, + NotificationSettings +} from '../../src/models/NotificationSettings'; import {User} from '../../src/models/User'; import {Course} from '../../src/models/Course'; import chaiHttp = require('chai-http'); @@ -102,6 +106,24 @@ describe('NotificationSettings', async () => { res.body.should.have.property('course'); res.body._id.should.be.a('string'); }); + + it('should fail when missing course or user', async () => { + const course = await FixtureUtils.getRandomCourse(); + const student = course.students[Math.floor(Math.random() * course.students.length)]; + + const settings = await new NotificationSettings({ + 'user': student, + 'course': course, + 'notificationType': API_NOTIFICATION_TYPE_ALL_CHANGES, + 'emailNotification': false + }).save(); + + const res = await chai.request(app) + .put(`${BASE_URL}/${settings._id}`) + .set('Authorization', `JWT ${JwtUtils.generateToken(student)}`) + .send([]); + res.should.have.status(400); + }); }); }); From baf8af0eb6672838cf5cc24c8d8a453c3e76253e Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Fri, 8 Jun 2018 11:12:29 +0200 Subject: [PATCH 2/6] :memo: Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b72386f08..79c9c0b76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fixed broken Apidoc [#737](https://github.com/h-da/geli/issues/737) - Disabled `tutor` role. [#710](https://github.com/h-da/geli/issues/710) - Fixed notifications on hidden units. [#733](https://github.com/utetrapp/geli/issues/733) +- Validate user input for notication settings api. [#771](https://github.com/utetrapp/geli/issues/771) ## [[0.7.0](https://github.com/h-da/geli/releases/tag/v0.7.0)] - 2018-05-05 - SS 18 intermediate Release ### Added From 5eb49b035a660d03e4b53cbdd5c69e33ba43c52e Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Fri, 8 Jun 2018 12:12:47 +0200 Subject: [PATCH 3/6] :bug: try a fix for coveralls --- .travis/coveralls.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis/coveralls.sh b/.travis/coveralls.sh index 9d63b8d6d..6129f7d8d 100755 --- a/.travis/coveralls.sh +++ b/.travis/coveralls.sh @@ -31,7 +31,8 @@ echo "+ sending lcov file to coveralls" # since we are using typescript and remap our coverage data to the ts files we need to remove the "build" part of all paths # this could easily be done with some sed magic # search for "api/build/src" and replace it with "api/src" -sed "s/api\/build\/src/api\/src/g" api/coverage/lcov.info | $BIN_PATH_FULL/coveralls -v +sed -i "s/api\/build\/src/api\/src/g" api/coverage/lcov.info +cat api/coverage/lcov.info | $BIN_PATH_FULL/coveralls echo "+ INFO: Currently only the api-coverdata are generated and send" From 3c83e41ac4b571071a4af2984502bad66a20274b Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Fri, 8 Jun 2018 13:18:03 +0200 Subject: [PATCH 4/6] Verbose output for coveralls --- .travis/coveralls.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis/coveralls.sh b/.travis/coveralls.sh index 6129f7d8d..c79a24ad5 100755 --- a/.travis/coveralls.sh +++ b/.travis/coveralls.sh @@ -32,7 +32,7 @@ echo "+ sending lcov file to coveralls" # this could easily be done with some sed magic # search for "api/build/src" and replace it with "api/src" sed -i "s/api\/build\/src/api\/src/g" api/coverage/lcov.info -cat api/coverage/lcov.info | $BIN_PATH_FULL/coveralls +cat api/coverage/lcov.info | $BIN_PATH_FULL/coveralls -v echo "+ INFO: Currently only the api-coverdata are generated and send" From 3af6e4e7380db90fe939e32d0864c837848f1d75 Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Fri, 8 Jun 2018 14:21:46 +0200 Subject: [PATCH 5/6] :white_check_mark: Add test when add notification setting a second time --- .../TestNotificationSettingsController.ts | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/api/test/controllers/TestNotificationSettingsController.ts b/api/test/controllers/TestNotificationSettingsController.ts index 055263436..f8562a2e8 100644 --- a/api/test/controllers/TestNotificationSettingsController.ts +++ b/api/test/controllers/TestNotificationSettingsController.ts @@ -26,7 +26,7 @@ describe('NotificationSettings', async () => { describe(`POST ${BASE_URL}`, async () => { it('should create notification settings', async () => { const course = await FixtureUtils.getRandomCourse(); - const student = await User.findById(course.students[0]); + const student = course.students[0]; const newSettings = {user: student, course: course}; const res = await chai.request(app) @@ -44,6 +44,24 @@ describe('NotificationSettings', async () => { notificationSettings.user.toString().should.be.equal(newSettings.user._id.toString()); notificationSettings.course.toString().should.be.equal(newSettings.course._id.toString()); }); + + it('should fail when already exist', async () => { + const course = await FixtureUtils.getRandomCourse(); + const student = course.students[0]; + const newSettings = {user: student, course: course}; + + const res = await chai.request(app) + .post(BASE_URL) + .set('Authorization', `JWT ${JwtUtils.generateToken(student)}`) + .send(newSettings); + res.status.should.be.equals(200); + + const resFail = await chai.request(app) + .post(BASE_URL) + .set('Authorization', `JWT ${JwtUtils.generateToken(student)}`) + .send(newSettings); + resFail.status.should.be.equals(400); + }); }); describe(`GET ${BASE_URL} user :id`, () => { From 3957ded2d4c216a49fe10daa76ce005feadab8b8 Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Fri, 8 Jun 2018 14:25:35 +0200 Subject: [PATCH 6/6] :white_check_mark: Add test when add empty data send --- .../controllers/TestNotificationSettingsController.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/api/test/controllers/TestNotificationSettingsController.ts b/api/test/controllers/TestNotificationSettingsController.ts index f8562a2e8..54f242e3e 100644 --- a/api/test/controllers/TestNotificationSettingsController.ts +++ b/api/test/controllers/TestNotificationSettingsController.ts @@ -62,6 +62,17 @@ describe('NotificationSettings', async () => { .send(newSettings); resFail.status.should.be.equals(400); }); + + it('should fail when course or user missing', async () => { + const course = await FixtureUtils.getRandomCourse(); + const student = course.students[0]; + + const res = await chai.request(app) + .post(BASE_URL) + .set('Authorization', `JWT ${JwtUtils.generateToken(student)}`) + .send({}); + res.status.should.be.equals(400); + }); }); describe(`GET ${BASE_URL} user :id`, () => {