Merge branch 'develop' into feature/#688-auto-fix-linting

geli-lms · May 4, 2018 · c44f9f9 · c44f9f9
2 parents 055f722 + 9d79517
commit c44f9f9
Show file tree

Hide file tree

Showing 16 changed files with 101 additions and 114 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,9 +13,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Added new course & user API unit tests. [#654](https://github.com/h-da/geli/issues/654) [#691](https://github.com/h-da/geli/issues/691)
 - Added details of courseAdmin and teacher to course detail view. on click profiles are shown.[#598] (https://github.com/h-da/geli/issues/598)
 - Added small auto linting scripts to package.json [#688](https://github.com/h-da/geli/issues/688)
+- Added changed size of drop down arrows for better usability. [#686] (https://github.com/h-da/geli/issues/686)
 
 ### Changed
 - Refactored or slightly altered various course & user related APIs. [#654](https://github.com/h-da/geli/issues/654) [#691](https://github.com/h-da/geli/issues/691)
+- Removed firstname from resend activation feature and change button positioning. [#711] (https://github.com/h-da/geli/issues/711)
+- Refactored register and resend activation to use geli email validator with top level domain check. [#713] (https://github.com/h-da/geli/issues/713)
 
 ### Fixed
 - Fixed wasteful course data usage via specialized course model interfaces. [#654](https://github.com/h-da/geli/issues/654)
@@ -31,7 +34,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Fixed `tsconfig.spec.ts` for `ng test`. [#656](https://github.com/h-da/geli/pull/656)
 
 ### Security
-- Fixed numerous severe user related security issues. [#691](https://github.com/h-da/geli/issues/691)
+- Fixed numerous severe user related security issues. [#691](https://github.com/h-da/geli/issues/691) [#709](https://github.com/h-da/geli/pull/709)
 - Fixed multiple severe course related security issues. [#594](https://github.com/h-da/geli/issues/594) [#653](https://github.com/h-da/geli/issues/653) [#691](https://github.com/h-da/geli/issues/691)
 - Updated the dependencies for security. [#661](https://github.com/h-da/geli/issues/661)
 

diff --git a/api/src/controllers/AuthController.ts b/api/src/controllers/AuthController.ts
@@ -142,7 +142,6 @@ export class AuthController {
    * @apiName ActivationResend
    * @apiGroup Auth
    *
-   * @apiParam {string} firstname firstname of user which activation should be resend.
    * @apiParam {string} lastname lastname of user which activation should be resend.
    * @apiParam {string} uid matriculation number of user which activation should be resend.
    * @apiParam {string} email email the new activation should be sent to.
@@ -156,12 +155,11 @@ export class AuthController {
    */
   @Post('/activationresend')
   @OnUndefined(204)
-  async activationResend (@BodyParam('firstname') firstname: string,
-                                      @BodyParam('lastname') lastname: string,
+  async activationResend (@BodyParam('lastname') lastname: string,
                                       @BodyParam('uid') uid: string,
                                       @BodyParam('email') email: string,
                                       @Res() response: Response) {
-        const user = await User.findOne({'profile.firstName': firstname, 'profile.lastName': lastname, uid: uid, role: 'student'});
+        const user = await User.findOne({'profile.lastName': lastname, uid: uid, role: 'student'});
 
         if (!user) {
           throw new BadRequestError(errorCodes.errorCodes.user.userNotFound.code);

diff --git a/api/src/models/User.ts b/api/src/models/User.ts
@@ -220,7 +220,7 @@ userSchema.methods.forUser = function (otherUser: IUser): IUserSubSafe | IUserSu
 // (Or when the currentUser is an admin or targets itself.)
 const editLevels: {[key: string]: number} = {
   student: 0,
-  teacher: 1,
+  teacher: 0,
   admin: 2,
 };
 

diff --git a/api/test/integration/auth.ts b/api/test/integration/auth.ts
@@ -159,8 +159,7 @@ describe('Auth', () => {
 
       const res = await chai.request(app)
         .post(`${BASE_URL}/activationresend`)
-        .send({'firstname': resendActivationUser.profile.firstName,
-          'lastname': resendActivationUser.profile.lastName,
+        .send({'lastname': resendActivationUser.profile.lastName,
           'uid': resendActivationUser.uid,
           'email': resendActivationUser.email })
         .catch(err => err.response);
@@ -176,8 +175,7 @@ describe('Auth', () => {
 
       const res = await chai.request(app)
         .post(`${BASE_URL}/activationresend`)
-        .send({'firstname': resendActivationUser.profile.firstName,
-          'lastname': resendActivationUser.profile.lastName,
+        .send({'lastname': resendActivationUser.profile.lastName,
           'uid': resendActivationUser.uid,
           'email': resendActivationUser.email })
         .catch(err => err.response);
@@ -196,8 +194,7 @@ describe('Auth', () => {
 
       const res = await chai.request(app)
         .post(`${BASE_URL}/activationresend`)
-        .send({'firstname': resendActivationUser.profile.firstName,
-          'lastname': resendActivationUser.profile.lastName,
+        .send({'lastname': resendActivationUser.profile.lastName,
           'uid': resendActivationUser.uid,
           'email': resendActivationUser.email })
         .catch(err => err.response);
@@ -218,8 +215,7 @@ describe('Auth', () => {
 
       const res = await chai.request(app)
         .post(`${BASE_URL}/activationresend`)
-        .send({'firstname': resendActivationUser.profile.firstName,
-          'lastname': resendActivationUser.profile.lastName,
+        .send({'lastname': resendActivationUser.profile.lastName,
           'uid': resendActivationUser.uid,
           'email': resendActivationUser.email })
         .catch(err => err.response);
@@ -236,8 +232,7 @@ describe('Auth', () => {
 
       const res = await chai.request(app)
         .post(`${BASE_URL}/activationresend`)
-        .send({'firstname': resendActivationUser.profile.firstName,
-          'lastname': resendActivationUser.profile.lastName,
+        .send({'lastname': resendActivationUser.profile.lastName,
           'uid': resendActivationUser.uid,
           'email': resendActivationUser.email })
         .catch(err => err.response);

diff --git a/api/test/integration/user.ts b/api/test/integration/user.ts
@@ -156,67 +156,61 @@ describe('User', () => {
   });
 
   describe(`PUT ${BASE_URL}`, () => {
+    function requestUserUpdate(currentUser: IUser, updatedUser: IUser) {
+      return chai.request(app)
+        .put(`${BASE_URL}/${updatedUser._id}`)
+        .set('Authorization', `JWT ${JwtUtils.generateToken(currentUser)}`)
+        .send(updatedUser);
+    }
+
+    function requestUserUpdateAndCatch(currentUser: IUser, updatedUser: IUser) {
+      return requestUserUpdate(currentUser, updatedUser).catch(err => err.response);
+    }
+
+    function assertFailure(res: request.Response, status: number, name: string, message: string) {
+      res.status.should.be.equal(status);
+      res.body.name.should.be.equal(name);
+      res.body.message.should.be.equal(message);
+    }
+
     it('should fail with bad request (revoke own admin privileges)', async () => {
       const admin = await FixtureUtils.getRandomAdmin();
-
       const updatedUser = admin;
       updatedUser.role = 'teacher';
-      const res = await chai.request(app)
-        .put(`${BASE_URL}/${admin._id}`)
-        .set('Authorization', `JWT ${JwtUtils.generateToken(admin)}`)
-        .send(updatedUser)
-        .catch(err => err.response);
 
-      res.status.should.be.equal(400);
-      res.body.name.should.be.equal('BadRequestError');
-      res.body.message.should.be.equal(errorCodes.user.cantChangeOwnRole.text);
+      const res = await requestUserUpdateAndCatch(admin, updatedUser);
+      assertFailure(res, 400, 'BadRequestError', errorCodes.user.cantChangeOwnRole.text);
     });
 
     it('should fail with bad request (email already in use)', async () => {
       const admin = await FixtureUtils.getRandomAdmin();
       const updatedUser = await FixtureUtils.getRandomStudent();
       updatedUser.email = admin.email;
 
-      const res = await chai.request(app)
-        .put(`${BASE_URL}/${updatedUser._id}`)
-        .set('Authorization', `JWT ${JwtUtils.generateToken(admin)}`)
-        .send(updatedUser)
-        .catch(err => err.response);
-
-      res.status.should.be.equal(400);
-      res.body.name.should.be.equal('BadRequestError');
-      res.body.message.should.be.equal(errorCodes.user.emailAlreadyInUse.text);
+      const res = await requestUserUpdateAndCatch(admin, updatedUser);
+      assertFailure(res, 400, 'BadRequestError', errorCodes.user.emailAlreadyInUse.text);
     });
 
+    // This test is disabled because there currently is no role beneath 'admin' that is allowed to edit other users.
+    // Reactivate and adjust this test if such a role should become available in the future.
+    // (Previously teachers had permission to change some parts of any student's profile.)
+    /*
     it('should fail changing other user\'s uid with wrong authorization (not admin)', async () => {
       const teacher = await FixtureUtils.getRandomTeacher();
       const updatedUser = await FixtureUtils.getRandomStudent();
       updatedUser.uid = '987456';
 
-      const res = await chai.request(app)
-        .put(`${BASE_URL}/${updatedUser._id}`)
-        .set('Authorization', `JWT ${JwtUtils.generateToken(teacher)}`)
-        .send(updatedUser)
-        .catch(err => err.response);
-
-      res.status.should.be.equal(403);
-      res.body.name.should.be.equal('ForbiddenError');
-      res.body.message.should.be.equal(errorCodes.user.onlyAdminsCanChangeUids.text);
+      const res = await requestUserUpdateAndCatch(teacher, updatedUser);
+      assertFailure(res, 403, 'ForbiddenError', errorCodes.user.onlyAdminsCanChangeUids.text);
     });
+    */
 
     it('should fail changing other user\'s name with wrong authorization (low edit level)', async () => {
       const [student, updatedUser] = await FixtureUtils.getRandomStudents(2, 2);
       updatedUser.profile.firstName = 'TEST';
 
-      const res = await chai.request(app)
-        .put(`${BASE_URL}/${updatedUser._id}`)
-        .set('Authorization', `JWT ${JwtUtils.generateToken(student)}`)
-        .send(updatedUser)
-        .catch(err => err.response);
-
-      res.status.should.be.equal(403);
-      res.body.name.should.be.equal('ForbiddenError');
-      res.body.message.should.be.equal(errorCodes.user.cantChangeUserWithHigherRole.text);
+      const res = await requestUserUpdateAndCatch(student, updatedUser);
+      assertFailure(res, 403, 'ForbiddenError', errorCodes.user.cantChangeUserWithHigherRole.text);
     });
 
     it('should update user base data without password', async () => {
@@ -227,11 +221,7 @@ describe('User', () => {
       updatedUser.profile.lastName = 'User';
       updatedUser.email = 'student2@updated.local';
 
-      const res = await chai.request(app)
-        .put(`${BASE_URL}/${student._id}`)
-        .set('Authorization', `JWT ${JwtUtils.generateToken(student)}`)
-        .send(updatedUser);
-
+      const res = await requestUserUpdate(student, updatedUser);
       res.status.should.be.equal(200);
       res.body.profile.firstName.should.be.equal('Updated');
       res.body.profile.lastName.should.be.equal('User');
@@ -246,11 +236,7 @@ describe('User', () => {
       updatedUser.profile.lastName = 'User';
       updatedUser.email = 'student1@updated.local';
 
-      const res = await chai.request(app)
-        .put(`${BASE_URL}/${student._id}`)
-        .set('Authorization', `JWT ${JwtUtils.generateToken(student)}`)
-        .send(updatedUser);
-
+      const res = await requestUserUpdate(student, updatedUser);
       res.status.should.be.equal(200);
       res.body.profile.firstName.should.be.equal('Updated');
       res.body.profile.lastName.should.be.equal('User');
@@ -265,11 +251,7 @@ describe('User', () => {
       updatedUser.profile.lastName = 'User';
       updatedUser.email = 'student@updated.local';
 
-      const res = await chai.request(app)
-        .put(`${BASE_URL}/${student._id}`)
-        .set('Authorization', `JWT ${JwtUtils.generateToken(student)}`)
-        .send(updatedUser);
-
+      const res = await requestUserUpdate(student, updatedUser);
       res.status.should.be.equal(200);
       res.body.profile.firstName.should.be.equal('Updated');
       res.body.profile.lastName.should.be.equal('User');
@@ -287,11 +269,7 @@ describe('User', () => {
       updatedUser.profile.lastName = 'User';
       updatedUser.email = 'student@updated.local';
 
-      const res = await chai.request(app)
-        .put(`${BASE_URL}/${student._id}`)
-        .set('Authorization', `JWT ${JwtUtils.generateToken(admin)}`)
-        .send(updatedUser);
-
+      const res = await requestUserUpdate(admin, updatedUser);
       res.status.should.be.equal(200);
       res.body.uid.should.be.equal(origUid);
       res.body.profile.firstName.should.be.equal('Updated');

diff --git a/app/webFrontend/src/app/auth/activation-resend/activation-resend.component.html b/app/webFrontend/src/app/auth/activation-resend/activation-resend.component.html
@@ -18,20 +18,6 @@
   <mat-card-content>
     <form name="form" (ngSubmit)="resendActivation()" [formGroup]="resendActivationForm">
       <div class="form-group" formGroupName="profile">
-        <mat-form-field>
-          <input matInput formControlName="firstName"
-                 [placeholder]="'common.profile.firstName'|translate"/>
-          <div *ngIf="resendActivationForm.get('profile').get('firstName').touched">
-            <small *ngIf="resendActivationForm.get('profile').get('firstName').hasError('required')"
-                   class="text-danger">
-              {{'common.validation.required'|translate}}
-            </small>
-            <small *ngIf="resendActivationForm.get('profile').get('firstName').hasError('minlength')"
-                   class="text-danger" translate [translateParams]="{min:2}">
-              common.validation.minLength
-            </small>
-          </div>
-        </mat-form-field>
         <mat-form-field>
           <input matInput formControlName="lastName"
                  [placeholder]="'common.profile.lastName'|translate"/>
@@ -72,7 +58,7 @@
             </small>
             <small *ngIf="mailError" class="text-danger">{{mailError}}</small>
             <small
-              *ngIf="resendActivationForm.controls.email.hasError('email') && !resendActivationForm.controls.email.hasError('required')"
+              *ngIf="resendActivationForm.controls.email.hasError('emailValidator') && !resendActivationForm.controls.email.hasError('required')"
               class="text-danger">
               {{'common.validation.invalid'|translate}}
             </small>
@@ -86,9 +72,12 @@
       <button mat-raised-button color="primary" type="submit"
               [disabled]="loading || !resendActivationForm?.valid">{{'common.send'|translate}}
       </button>
-      <button mat-raised-button type="button" routerLink="/register" routerLinkActive="active">
+    </form>
+    <div class="bottom-buttons">
+      <button mat-button type="button" routerLink="/register" routerLinkActive="active">
         {{'auth.resendActivation.back'|translate}}
       </button>
-    </form>
+    </div>
+
   </mat-card-content>
 </mat-card>
diff --git a/app/webFrontend/src/app/auth/activation-resend/activation-resend.component.scss b/app/webFrontend/src/app/auth/activation-resend/activation-resend.component.scss
@@ -19,3 +19,7 @@
 #resendActivationFormError{
   margin-bottom: 20px;
 }
+
+.bottom-buttons{
+  margin-top: 20px;
+}
diff --git a/app/webFrontend/src/app/auth/activation-resend/activation-resend.component.ts b/app/webFrontend/src/app/auth/activation-resend/activation-resend.component.ts
@@ -6,6 +6,7 @@ import {ShowProgressService} from '../../shared/services/show-progress.service';
 import {MatSnackBar} from '@angular/material';
 import {errorCodes} from '../../../../../../api/src/config/errorCodes';
 import {TitleService} from '../../shared/services/title.service';
+import {emailValidator} from '../../shared/validators/validators';
 
 @Component({
   selector: 'app-activation-resend',
@@ -22,7 +23,6 @@ export class ActivationResendComponent implements OnInit {
 
   private trimFormFields() {
 
-    this.resendActivationForm.value.profile.firstName = this.resendActivationForm.value.profile.firstName.trim();
     this.resendActivationForm.value.profile.lastName = this.resendActivationForm.value.profile.lastName.trim();
     this.resendActivationForm.value.uid = this.resendActivationForm.value.uid.trim();
     this.resendActivationForm.value.email = this.resendActivationForm.value.email.trim();
@@ -61,8 +61,8 @@ export class ActivationResendComponent implements OnInit {
     this.resendActivationForm.value.email = this.resendActivationForm.value.email.replace(/\s/g, '').toLowerCase();
     this.trimFormFields();
     try {
-     await this.authenticationService.resendActivation(this.resendActivationForm.value.profile.firstName,
-        this.resendActivationForm.value.profile.lastName, this.resendActivationForm.value.uid, this.resendActivationForm.value.email);
+     await this.authenticationService.resendActivation(this.resendActivationForm.value.profile.lastName,
+       this.resendActivationForm.value.uid, this.resendActivationForm.value.email);
      this.resendActivationDone = true;
     } catch (err) {
       this.handleError(err);
@@ -103,11 +103,10 @@ export class ActivationResendComponent implements OnInit {
   generateForm() {
     this.resendActivationForm = this.formBuilder.group({
       profile: this.formBuilder.group({
-        firstName: ['', Validators.compose([Validators.required, Validators.minLength(2)])],
         lastName: ['', Validators.compose([Validators.required, Validators.minLength(2)])],
       }),
       uid: ['', Validators.compose([Validators.required, this.validateMatriculationNumber.bind(this)])],
-      email: ['', Validators.compose([Validators.required, Validators.email])]
+      email: ['', Validators.compose([Validators.required, emailValidator])]
     });
   }
 

diff --git a/app/webFrontend/src/app/auth/register/register.component.html b/app/webFrontend/src/app/auth/register/register.component.html
@@ -65,11 +65,11 @@
           </small>
           <small *ngIf="mailError" class="text-danger">{{mailError}}</small>
           <small
-            *ngIf="registerForm.controls.email.hasError('teacherEmailError') && !registerForm.controls.email.hasError('email') && !registerForm.controls.email.hasError('required')"
+            *ngIf="registerForm.controls.email.hasError('teacherEmailError') && !registerForm.controls.email.hasError('emailValidator') && !registerForm.controls.email.hasError('required')"
             class="text-danger">{{'common.validation.instituteMail'|translate}}
           </small>
           <small
-            *ngIf="registerForm.controls.email.hasError('email') && !registerForm.controls.email.hasError('teacherMailError') && !registerForm.controls.email.hasError('required')"
+            *ngIf="registerForm.controls.email.hasError('emailValidator') && !registerForm.controls.email.hasError('teacherMailError') && !registerForm.controls.email.hasError('required')"
             class="text-danger">
             {{'common.validation.invalid'|translate}}
           </small>
@@ -93,17 +93,19 @@
       <button mat-raised-button color="primary" type="submit"
               [disabled]="loading || !registerForm?.valid">{{'common.register'|translate}}
       </button>
-      <button *ngIf="role == 'student'" mat-raised-button type="button" routerLink="/activation-resend" routerLinkActive="active">
+    </form>
+    <div class="bottom-buttons">
+      <button *ngIf="role == 'student'" mat-button type="button" routerLink="/activation-resend" routerLinkActive="active">
         {{'auth.registration.resendActivationButton'|translate}}
       </button>
-    </form>
 
-    <button mat-button *ngIf="role==='student'" (click)="changeRole('teacher')" class="pull-right">
-      {{'auth.registration.teacherRegistrationButton'|translate}}
-    </button>
+      <button mat-button *ngIf="role==='student'" (click)="changeRole('teacher')" class="pull-right">
+        {{'auth.registration.teacherRegistrationButton'|translate}}
+      </button>
 
-    <button mat-button *ngIf="role!=='student'" (click)="changeRole('student')" class="pull-right">
-      {{'common.back'|translate}}
-    </button>
+      <button mat-button *ngIf="role!=='student'" (click)="changeRole('student')" class="pull-right">
+        {{'common.back'|translate}}
+      </button>
+    </div>
   </mat-card-content>
 </mat-card>
diff --git a/app/webFrontend/src/app/auth/register/register.component.scss b/app/webFrontend/src/app/auth/register/register.component.scss
@@ -23,7 +23,7 @@
     }
   }
 
-  .pull-right{
-    margin-top: 7px;
+  .bottom-buttons{
+    margin-top: 20px;
   }
 }