From 809167fa353ed678f68815e55e5635c64a7e470b Mon Sep 17 00:00:00 2001 From: sergio-dc Date: Sat, 10 Oct 2020 20:01:09 -0500 Subject: [PATCH 1/8] Updating enrollment model This change was made keep the same structure between the migration file and the model --- src/models/enrollment.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/models/enrollment.js b/src/models/enrollment.js index cc2af5c5..8bd63f1c 100644 --- a/src/models/enrollment.js +++ b/src/models/enrollment.js @@ -4,7 +4,17 @@ const { Student } = require("./student"); const Course = require("./course"); const Enrollment = sequelize.define('enrollments', -{ +{ + studentId: { + type: Sequelize.INTEGER, + allowNull: false, + references: { model: 'students', key: 'id' } + }, + courseId: { + type: Sequelize.INTEGER, + allowNull: false, + references: { model: 'courses', key: 'id' } + }, calification: { type: Sequelize.DOUBLE, allowNull: false From 625e45e2f8e14dd8f5d46f545cecd69d19bf8931 Mon Sep 17 00:00:00 2001 From: sergio-dc Date: Sat, 10 Oct 2020 21:20:56 -0500 Subject: [PATCH 2/8] Fix enrollment model and update student model --- src/models/enrollment.js | 6 ------ src/models/student.js | 17 +++++++++++++---- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/models/enrollment.js b/src/models/enrollment.js index 8bd63f1c..c89c856f 100644 --- a/src/models/enrollment.js +++ b/src/models/enrollment.js @@ -1,7 +1,5 @@ const Sequelize = require("sequelize"); const {sequelize} = require("../../config/db/mysql"); -const { Student } = require("./student"); -const Course = require("./course"); const Enrollment = sequelize.define('enrollments', { @@ -23,9 +21,5 @@ const Enrollment = sequelize.define('enrollments', { timestamps: false } ); - -Enrollment.belongsTo(Course); -Enrollment.belongsTo(Student); - module.exports = Enrollment diff --git a/src/models/student.js b/src/models/student.js index fae70463..0b612abc 100644 --- a/src/models/student.js +++ b/src/models/student.js @@ -1,6 +1,5 @@ const Sequelize = require("sequelize"); const { sequelize } = require("../../config/db/mysql"); -const Course = require("./course"); const Student = sequelize.define("students", { id: { @@ -9,6 +8,19 @@ const Student = sequelize.define("students", { primaryKey: true, autoIncrement:true }, + userId: { + type: Sequelize.INTEGER, + allowNull: false, + references: { model: 'users', key: 'id' } + }, + createdAt: { + allowNull: false, + type: Sequelize.DATE + }, + updatedAt: { + allowNull: false, + type: Sequelize.DATE + }, deletedAt: { type: Sequelize.DATE, allowNull: true, @@ -17,7 +29,4 @@ const Student = sequelize.define("students", { paranoid:true }); - -Student.hasMany(Course); - module.exports = Student; From 2e22bef5c74a0aad8804edc1ebf09873c4be66b5 Mon Sep 17 00:00:00 2001 From: sergio-dc Date: Wed, 14 Oct 2020 20:11:57 -0500 Subject: [PATCH 3/8] \#1 Enrollment test_case added Verifies whether the student can enroll to a course if it exists, the test case does not contemplate doing calls with JWT --- tests/rest/enrollment.js | 52 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 tests/rest/enrollment.js diff --git a/tests/rest/enrollment.js b/tests/rest/enrollment.js new file mode 100644 index 00000000..725e0652 --- /dev/null +++ b/tests/rest/enrollment.js @@ -0,0 +1,52 @@ +const chai = require("chai"); +const chaiHttp = require("chai-http"); +const faker = require("faker"); +const { expect } = require("chai"); + +const app = require("../../src/index"); +const { generateValidToken } = require("../../src/services/generateToken"); +const StudentService = require('../../src/services/studentService'); +const EnrollmentController = require('../../src/controllers/enrollmentController'); +const User = require("../../src/models/user"); +const Student = require('../../src/models/student'); +const Enrollment = require('../../src/models/enrollment'); +const { sequelize } = require("../../config/db/mysql"); +const { exec } = require('child_process'); +const e = require("express"); +const LOG = require('debug')('error'); +const server = require('../../src/index'); + + +//1. Para que un estudiante pueda enrolarse al curso este debe existir y haber iniciado sesión +//2. Un estudiante no puede enrolarse al mismo curso más de 1 vez +//3. Un estudiante tendrá accesos al curso cuando el profesor lo haya autorizado +//4. Un estudiante NO puede desenrolarse de un curso +//5. Un profesor puede desenrolar de un curso a un estudiante +//6. Solo los estudiantes pueden visualizar los cursos? + +chai.use(chaiHttp); +describe('Enrollment Tests', () => { + describe('A student can enroll to a course', () => { + const existentCourseId = 100; + let student, token; + before(async function(){ + student = await StudentService.createStudent({firstname: 'Rigoberto', lastname: 'Pérez', email: 'ed2@gmail.com', password: 'cisco123'}); + //token = generateValidToken(student); + }); + + it.only('should return true if the course exists', (done) => { + chai.request(app) + .get("/api/v1/enrollment") + .query({studentId: student.id, courseId: existentCourseId}) + //.set( 'Authorization', `Bearer ${token}` ) + .end(function(err, res) { + if (err) done(err); + expect(res).to.have.property('ok', true); + expect(res).to.have.status(201); + + done(); + }); + }); + }); +}); + From c415c922786b5c91a2b355b64fb9f55cd7219c41 Mon Sep 17 00:00:00 2001 From: sergio-dc Date: Wed, 14 Oct 2020 20:37:02 -0500 Subject: [PATCH 4/8] Fix Enrollment model To avoid 'Enrollments' table having a primary_key --- src/models/enrollment.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/models/enrollment.js b/src/models/enrollment.js index c89c856f..ddfb270b 100644 --- a/src/models/enrollment.js +++ b/src/models/enrollment.js @@ -21,5 +21,7 @@ const Enrollment = sequelize.define('enrollments', { timestamps: false } ); +Enrollment.removeAttribute('id');//Remove the default field named 'id' + module.exports = Enrollment From 1fdf71d09ddbcd7029138875a23002f5485fb3a8 Mon Sep 17 00:00:00 2001 From: sergio-dc Date: Wed, 14 Oct 2020 20:50:48 -0500 Subject: [PATCH 5/8] Feature added: A student can enroll to a course At this moment this feature works in a simple manner. --- src/api/routes/enrollment.js | 8 ++++++ src/api/routes/index.js | 1 + src/controllers/enrollmentController.js | 35 +++++++++++++++++++++++++ src/services/studentService.js | 2 +- 4 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 src/api/routes/enrollment.js create mode 100644 src/controllers/enrollmentController.js diff --git a/src/api/routes/enrollment.js b/src/api/routes/enrollment.js new file mode 100644 index 00000000..cbe792ba --- /dev/null +++ b/src/api/routes/enrollment.js @@ -0,0 +1,8 @@ +const { Router } = require('express'); +const EnrollmentController = require("../../controllers/enrollmentController"); + +const router = Router(); + +router.get('/', EnrollmentController.enrollStudentInACourse); + +module.exports = router; \ No newline at end of file diff --git a/src/api/routes/index.js b/src/api/routes/index.js index 56374bd9..a54b2982 100644 --- a/src/api/routes/index.js +++ b/src/api/routes/index.js @@ -14,6 +14,7 @@ module.exports = function(app) { { path: "/answer", file: "./answers" }, { path: "/question", file: "./questions" }, { path: "/exam", file: "./exams" }, + { path: "/enrollment", file: './enrollment'} ]; routes.forEach(route => { diff --git a/src/controllers/enrollmentController.js b/src/controllers/enrollmentController.js new file mode 100644 index 00000000..7bcacbde --- /dev/null +++ b/src/controllers/enrollmentController.js @@ -0,0 +1,35 @@ +const e = require("debug")("error:data"); +const Enrollment = require('../models/enrollment'); + +class EnrollmentController { + + + static async enrollStudentInACourse(req, res, next) { + LOG(`StudentId: ${req.query.studentId}`); + LOG(`courseId: ${req.query.courseId}`); + const studentId = req.query.studentId; + const courseId = req.query.courseId; + + try { + const enrollment = await Enrollment.create({ + studentId, + courseId, + calification: 85 + }); + return res.status(201).json({ + ok: true, + message: `You have enrolled to course with id ${courseId}`, + }); + }catch(err) { + e(err); + return res.status(400).json({ + ok: false, + message: `Something wrong happen enrolling in a course`, + err + }); + } + + } +} + +module.exports = EnrollmentController; \ No newline at end of file diff --git a/src/services/studentService.js b/src/services/studentService.js index e92db90a..1b3e577b 100644 --- a/src/services/studentService.js +++ b/src/services/studentService.js @@ -22,7 +22,7 @@ class StudentService { } catch (err) { e(err); - return new Error('An error has ocurred'); + return new Error(err); } } From c72047bbc24d667eed94e1931cf36639d34d8303 Mon Sep 17 00:00:00 2001 From: sergio-dc Date: Wed, 14 Oct 2020 22:08:04 -0500 Subject: [PATCH 6/8] Improving #1 Enrollment test_case All the transactions did by test_case to the tables will be cleaned and restored --- src/controllers/enrollmentController.js | 2 +- tests/rest/enrollment.js | 71 ++++++++++++++++++++++++- 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/src/controllers/enrollmentController.js b/src/controllers/enrollmentController.js index 7bcacbde..b5d9ffdf 100644 --- a/src/controllers/enrollmentController.js +++ b/src/controllers/enrollmentController.js @@ -1,4 +1,4 @@ -const e = require("debug")("error:data"); +const LOG = require("debug")("error:data"); const Enrollment = require('../models/enrollment'); class EnrollmentController { diff --git a/tests/rest/enrollment.js b/tests/rest/enrollment.js index 725e0652..d8f06356 100644 --- a/tests/rest/enrollment.js +++ b/tests/rest/enrollment.js @@ -27,7 +27,7 @@ const server = require('../../src/index'); chai.use(chaiHttp); describe('Enrollment Tests', () => { describe('A student can enroll to a course', () => { - const existentCourseId = 100; + const existentCourseId = 5; let student, token; before(async function(){ student = await StudentService.createStudent({firstname: 'Rigoberto', lastname: 'Pérez', email: 'ed2@gmail.com', password: 'cisco123'}); @@ -46,7 +46,74 @@ describe('Enrollment Tests', () => { done(); }); - }); + }); + + after(function(){ + const starterPromise = Promise.resolve(); + const asyncThingsToDo = [ + 'DELETE_ENROLLMENT_ROWS', + 'DELETE_STUDENT_ROWS', + 'DELETE_USER_ROWS', + 'ALTER_AUTOINCREMENT_STUDENT', + 'ALTER_AUTOINCREMENT_USER', + 'CLOSE_DB_CONNECTION', + 'CLOSE_SERVER_CONNECTION' + ]; + + + asyncThingsToDo.reduce(async (previous, task) => { + try { + const log = await previous; + LOG(log); + } catch(err) { + LOG(err) + } + return cleanTables(task); + },starterPromise); + + }); + + function cleanTables(task) { + switch(task) { + case 'DELETE_ENROLLMENT_ROWS': + LOG('Inside DELETE_ENROLLMENT_ROWS'); + return Enrollment.destroy({ + where: { + studentId: student.id, + courseId + }, + force: true + }).then(num => `Enrollment rows deleted ${num}`); + case 'DELETE_STUDENT_ROWS': + LOG('Inside DELETE_STUDENT_ROWS'); + return Student.destroy({ + where: { + id: student.id + }, + force: true + }).then(num => `Student rows deleted ${num}`); + case 'DELETE_USER_ROWS': + LOG('Inside DELETE_USER_ROWS'); + return User.destroy({ + where: { + id: student.userId + }, + force: true + }).then(num => `User rows deleted ${num}`); + case 'ALTER_AUTOINCREMENT_STUDENT': + LOG('Inside ALTER_AUTOINCREMENT_STUDENT'); + return sequelize.query(`ALTER TABLE students AUTO_INCREMENT ${student.id}`) + //.then(result => result); + case 'ALTER_AUTOINCREMENT_USER': + LOG('Inside ALTER_AUTOINCREMENT_USER'); + return sequelize.query(`ALTER TABLE users AUTO_INCREMENT ${student.userId}`) + //.then(result => result); + case 'CLOSE_DB_CONNECTION': + return sequelize.close(); + case 'CLOSE_SERVER_CONNECTION': + return server.close(); + } + } }); }); From 717b07073e66c15b9f2e21da0eeace3bf67e7a05 Mon Sep 17 00:00:00 2001 From: sergio-dc Date: Thu, 15 Oct 2020 22:33:51 -0500 Subject: [PATCH 7/8] Fix Enrollment test case #1 typo --- tests/rest/enrollment.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/rest/enrollment.js b/tests/rest/enrollment.js index d8f06356..65081a84 100644 --- a/tests/rest/enrollment.js +++ b/tests/rest/enrollment.js @@ -31,14 +31,14 @@ describe('Enrollment Tests', () => { let student, token; before(async function(){ student = await StudentService.createStudent({firstname: 'Rigoberto', lastname: 'Pérez', email: 'ed2@gmail.com', password: 'cisco123'}); - //token = generateValidToken(student); + token = generateValidToken(student); }); it.only('should return true if the course exists', (done) => { chai.request(app) .get("/api/v1/enrollment") .query({studentId: student.id, courseId: existentCourseId}) - //.set( 'Authorization', `Bearer ${token}` ) + .set( 'Authorization', `Bearer ${token}` ) .end(function(err, res) { if (err) done(err); expect(res).to.have.property('ok', true); @@ -80,7 +80,7 @@ describe('Enrollment Tests', () => { return Enrollment.destroy({ where: { studentId: student.id, - courseId + courseId: existentCourseId }, force: true }).then(num => `Enrollment rows deleted ${num}`); From 084ee592df267ebf24cb5a4054115986de6e8fa8 Mon Sep 17 00:00:00 2001 From: sergio-dc Date: Sat, 17 Oct 2020 13:13:21 -0500 Subject: [PATCH 8/8] Adding a layer of security to the get enrollment endpoint with JWT Adding several debug code lines --- src/api/middlewares/is-auth.js | 42 ++++++++++++++++++++++++++++++---- src/api/routes/enrollment.js | 3 ++- tests/rest/enrollment.js | 21 ++++++++--------- 3 files changed, 48 insertions(+), 18 deletions(-) diff --git a/src/api/middlewares/is-auth.js b/src/api/middlewares/is-auth.js index 7c32a62e..6e5e8e5a 100644 --- a/src/api/middlewares/is-auth.js +++ b/src/api/middlewares/is-auth.js @@ -1,15 +1,27 @@ //require("dotenv").config(); const jwt = require("jsonwebtoken"); const Professor = require("../../models/professor"); +const Student = require('../../models/student'); +const log = require('debug')('is-auth'); +const logVerifyToken = log.extend('verifyToken'); +const logVerifyStudent = log.extend('verifyStudent'); exports.verifyToken=(req,res,next)=>{ - - let query = req.header('authorization').split(' '); - - let token = query[1]; //Authenticate - + //The exception thrown by the split method is handled sending a meessage to client + let token; + try { + let query = req.header('authorization').split(' '); + token = query[1]; //Authenticate + }catch(err) { + logVerifyToken(err); + return res.status(503).json({ + ok: false, + err + }); + } jwt.verify(token,process.env.JWT_SECRET,(error,decoded)=>{ //decoded is the payload + logVerifyToken(error); if(error){ return res.status(500).json({ ok:false, @@ -37,4 +49,24 @@ exports.verifyProfesssor = async (req,res,next) => { error:'El usuario no es profesor' }); } +} + +exports.verifyStudent = async (req, res, next) => { + let id=req.id; + try { + const student= await Student.findOne({ + where:{ + userId:id + } + }); + logVerifyStudent(`Student row: ${student}`) + next(); + } catch(error){ + logVerifyStudent(`Error: ${error}`) + return res.status(500).json({ + ok:false, + message: 'User is not a student', + error + }); + } } \ No newline at end of file diff --git a/src/api/routes/enrollment.js b/src/api/routes/enrollment.js index cbe792ba..8278f1ba 100644 --- a/src/api/routes/enrollment.js +++ b/src/api/routes/enrollment.js @@ -1,8 +1,9 @@ const { Router } = require('express'); +const auth = require('../middlewares/is-auth'); const EnrollmentController = require("../../controllers/enrollmentController"); const router = Router(); -router.get('/', EnrollmentController.enrollStudentInACourse); +router.get('/', [auth.verifyToken, auth.verifyStudent],EnrollmentController.enrollStudentInACourse); module.exports = router; \ No newline at end of file diff --git a/tests/rest/enrollment.js b/tests/rest/enrollment.js index 65081a84..a7667c4b 100644 --- a/tests/rest/enrollment.js +++ b/tests/rest/enrollment.js @@ -1,19 +1,16 @@ const chai = require("chai"); const chaiHttp = require("chai-http"); -const faker = require("faker"); const { expect } = require("chai"); const app = require("../../src/index"); const { generateValidToken } = require("../../src/services/generateToken"); const StudentService = require('../../src/services/studentService'); -const EnrollmentController = require('../../src/controllers/enrollmentController'); const User = require("../../src/models/user"); const Student = require('../../src/models/student'); const Enrollment = require('../../src/models/enrollment'); const { sequelize } = require("../../config/db/mysql"); -const { exec } = require('child_process'); -const e = require("express"); -const LOG = require('debug')('error'); +const log = require('debug')('enrollmenTest'); +const logCleanTables = log.extend('cleanTables'); const server = require('../../src/index'); @@ -64,9 +61,9 @@ describe('Enrollment Tests', () => { asyncThingsToDo.reduce(async (previous, task) => { try { const log = await previous; - LOG(log); + logCleanTables(log); } catch(err) { - LOG(err) + logCleanTables(err) } return cleanTables(task); },starterPromise); @@ -76,7 +73,7 @@ describe('Enrollment Tests', () => { function cleanTables(task) { switch(task) { case 'DELETE_ENROLLMENT_ROWS': - LOG('Inside DELETE_ENROLLMENT_ROWS'); + logCleanTables('Inside DELETE_ENROLLMENT_ROWS'); return Enrollment.destroy({ where: { studentId: student.id, @@ -85,7 +82,7 @@ describe('Enrollment Tests', () => { force: true }).then(num => `Enrollment rows deleted ${num}`); case 'DELETE_STUDENT_ROWS': - LOG('Inside DELETE_STUDENT_ROWS'); + logCleanTables('Inside DELETE_STUDENT_ROWS'); return Student.destroy({ where: { id: student.id @@ -93,7 +90,7 @@ describe('Enrollment Tests', () => { force: true }).then(num => `Student rows deleted ${num}`); case 'DELETE_USER_ROWS': - LOG('Inside DELETE_USER_ROWS'); + logCleanTables('Inside DELETE_USER_ROWS'); return User.destroy({ where: { id: student.userId @@ -101,11 +98,11 @@ describe('Enrollment Tests', () => { force: true }).then(num => `User rows deleted ${num}`); case 'ALTER_AUTOINCREMENT_STUDENT': - LOG('Inside ALTER_AUTOINCREMENT_STUDENT'); + logCleanTables('Inside ALTER_AUTOINCREMENT_STUDENT'); return sequelize.query(`ALTER TABLE students AUTO_INCREMENT ${student.id}`) //.then(result => result); case 'ALTER_AUTOINCREMENT_USER': - LOG('Inside ALTER_AUTOINCREMENT_USER'); + logCleanTables('Inside ALTER_AUTOINCREMENT_USER'); return sequelize.query(`ALTER TABLE users AUTO_INCREMENT ${student.userId}`) //.then(result => result); case 'CLOSE_DB_CONNECTION':