Skip to content

Commit

Permalink
Users: Passwort reset by username for internal users (ls1intum#4625)
Browse files Browse the repository at this point in the history
  • Loading branch information
julian-christl authored Feb 3, 2022
1 parent e00dd7e commit 2f52056
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 87 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ public interface UserRepository extends JpaRepository<User, Long> {
@Query("select count(*) from User user where :#{#groupName} member of user.groups")
Long countByGroupsIsContaining(@Param("groupName") String groupName);

List<User> findAllByEmailIgnoreCase(@Param("email") String email);
@Query("select user from User user where lower(user.email) = lower(:#{#searchInput}) or lower(user.login) = lower(:#{#searchInput})")
List<User> findAllByEmailOrUsernameIgnoreCase(@Param("searchInput") String searchInput);

@EntityGraph(type = LOAD, attributePaths = { "groups", "authorities" })
@Query("select user from User user where :#{#groupName} member of user.groups")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,11 @@ public void changePassword(@RequestBody PasswordChangeDTO passwordChangeDto) {
/**
* {@code POST /account/reset-password/init} : Send an email to reset the password of the user.
*
* @param mail the mail of the user.
* @param mailUsername string containing either mail or username of the user.
*/
@PostMapping(path = "/account/reset-password/init")
public void requestPasswordReset(@RequestBody String mail) {
List<User> user = userRepository.findAllByEmailIgnoreCase(mail);
public void requestPasswordReset(@RequestBody String mailUsername) {
List<User> user = userRepository.findAllByEmailOrUsernameIgnoreCase(mailUsername);
if (!user.isEmpty()) {
List<User> internalUsers = user.stream().filter(User::isInternal).toList();
if (internalUsers.isEmpty()) {
Expand All @@ -220,9 +220,9 @@ else if (userService.prepareUserForPasswordReset(internalUsers.get(0))) {
}
}
else {
// Pretend the request has been successful to prevent checking which emails really exist
// Pretend the request has been successful to prevent checking which emails or usernames really exist
// but log that an invalid attempt has been made
log.warn("Password reset requested for non existing mail '{}'", mail);
log.warn("Password reset requested for non existing mail or username '{}'", mailUsername);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,42 +22,22 @@ <h1 jhiTranslate="reset.request.title">Reset your password</h1>
</div>

<div class="alert alert-warning">
<span jhiTranslate="reset.request.messages.info">Enter the email address you used to register.</span>
<span jhiTranslate="reset.request.messages.info">Enter the email address you used to register or your username.</span>
</div>

<form name="form" role="form" (ngSubmit)="requestReset()" [formGroup]="resetRequestForm">
<div class="form-group">
<label class="form-control-label" for="email" jhiTranslate="global.form.email">Email</label>
<input
type="email"
class="form-control"
id="email"
name="email"
placeholder="{{ 'global.form.email.placeholder' | artemisTranslate }}"
formControlName="email"
#email
/>

<div *ngIf="resetRequestForm.get('email')!.invalid && (resetRequestForm.get('email')!.dirty || resetRequestForm.get('email')!.touched)">
<small class="form-text text-danger" *ngIf="resetRequestForm.get('email')?.errors?.required" jhiTranslate="global.messages.validate.email.required">
Your email is required.
</small>

<small class="form-text text-danger" *ngIf="resetRequestForm.get('email')?.errors?.email" jhiTranslate="global.messages.validate.email.invalid">
Your email is invalid.
</small>

<small class="form-text text-danger" *ngIf="resetRequestForm.get('email')?.errors?.minlength" jhiTranslate="global.messages.validate.email.minlength">
Your email is required to be at least 5 characters.
</small>

<small class="form-text text-danger" *ngIf="resetRequestForm.get('email')?.errors?.maxlength" jhiTranslate="global.messages.validate.email.maxlength">
Your email cannot be longer than 100 characters.
</small>
</div>
</div>
<div class="form-group">
<label class="form-control-label" for="emailUsername" jhiTranslate="reset.request.form.emailUsername">Email/Username</label>
<input
type="text"
class="form-control"
id="emailUsername"
name="emailUsername"
placeholder="{{ 'reset.request.form.emailUsername' | artemisTranslate }}"
[(ngModel)]="emailUsernameValue"
#emailUsername
/>
</div>

<button type="submit" [disabled]="resetRequestForm.invalid" class="btn btn-primary" jhiTranslate="reset.request.form.button">Reset</button>
</form>
<button type="submit" class="btn btn-primary" jhiTranslate="reset.request.form.button" (click)="requestReset()">Reset</button>
</div>
</div>
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { AfterViewInit, Component, ElementRef, OnInit, ViewChild } from '@angular/core';
import { PasswordResetInitService } from './password-reset-init.service';
import { ProfileService } from 'app/shared/layouts/profiles/profile.service';
import { FormBuilder, Validators } from '@angular/forms';
import { FormBuilder } from '@angular/forms';
import { AlertService } from 'app/core/util/alert.service';
import { HttpErrorResponse } from '@angular/common/http';
import { onError } from 'app/shared/util/global.utils';
Expand All @@ -14,13 +14,9 @@ import { ExternalUserPasswordResetModalComponent } from 'app/account/password-re
templateUrl: './password-reset-init.component.html',
})
export class PasswordResetInitComponent implements OnInit, AfterViewInit {
@ViewChild('email', { static: false })
email?: ElementRef;

success = false;
resetRequestForm = this.fb.group({
email: ['', [Validators.required, Validators.minLength(5), Validators.maxLength(100), Validators.email]],
});
@ViewChild('emailUsername', { static: false })
emailUsernameElement?: ElementRef;
emailUsernameValue = '';
useExternal: boolean;
externalCredentialProvider: string;
externalPasswordResetLink?: string;
Expand Down Expand Up @@ -52,16 +48,19 @@ export class PasswordResetInitComponent implements OnInit, AfterViewInit {
}

ngAfterViewInit(): void {
if (this.email) {
this.email.nativeElement.focus();
if (this.emailUsernameElement) {
this.emailUsernameElement.nativeElement.focus();
}
}

requestReset(): void {
this.passwordResetInitService.save(this.resetRequestForm.get(['email'])!.value).subscribe({
if (!this.emailUsernameValue) {
this.alertService.error('reset.request.messages.info');
return;
}
this.passwordResetInitService.save(this.emailUsernameValue).subscribe({
next: () => {
this.alertService.success('reset.request.messages.success');
this.success = true;
},
error: (err: HttpErrorResponse) => {
if (this.useExternal && err?.error?.errorKey === 'externalUser') {
Expand Down
8 changes: 4 additions & 4 deletions src/main/webapp/i18n/de/reset.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
"request": {
"title": "Passwort zurücksetzen",
"form": {
"button": "Passwort zurücksetzen"
"button": "Passwort zurücksetzen",
"emailUsername": "E-Mail/Benutzername"
},
"messages": {
"info": "Gebe die E-Mail-Adresse ein, welche du bei der Registrierung verwendest hast.",
"success": "Eine E-Mail mit weiteren Instruktionen für das Zurücksetzen des Passworts wurde gesendet.",
"notfound": "<strong>Diese E-Mail Adresse existiert nicht!</strong> Überprüfe deine E-Mail-Adresse und versuche es nochmal."
"info": "Gib die E-Mail-Adresse ein, die du bei der Registrierung verwendest hast oder deinen Benutzernamen.",
"success": "Eine E-Mail mit weiteren Instruktionen für das Zurücksetzen des Passworts wurde gesendet. Aus Sicherheitsgründen erscheint dieser Hinweis auch wenn der angegebene Account nicht existiert. Überprüfe deine Eingabe, wenn du keine E-Mail erhältst."
},
"external": {
"explanation": "Deine Zugangsdaten sind nicht in Artemis sondern in {{provider}} gespeichert.",
Expand Down
8 changes: 4 additions & 4 deletions src/main/webapp/i18n/en/reset.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
"request": {
"title": "Reset your password",
"form": {
"button": "Reset password"
"button": "Reset password",
"emailUsername": "Email/Username"
},
"messages": {
"info": "Enter the email address you used to register",
"success": "Check your email for details on how to reset your password.",
"notfound": "<strong>Email address isn't registered!</strong> Please check and try again"
"info": "Enter the email address you used to register or your username.",
"success": "Check your email for details on how to reset your password. For security reasons, this hint is visible even if the account doesn't exist. Recheck your input if you don't receive an email."
},
"external": {
"explanation": "Your account credentials are not stored in Artemis but {{provider}}.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ public void changePasswordInvalidPassword() throws Exception {
}

@Test
public void passwordReset() throws Exception {
public void passwordResetByEmail() throws Exception {
// create user in repo
User user = ModelFactory.generateActivatedUser("authenticateduser");
User createdUser = userCreationService.createUser(new ManagedUserVM(user));
Expand All @@ -395,6 +395,31 @@ public void passwordReset() throws Exception {
request.getMvc().perform(req).andExpect(status().is(HttpStatus.OK.value())).andReturn();
ReflectionTestUtils.invokeMethod(request, "restoreSecurityContext");

verifyPasswordReset(createdUser, resetKeyBefore);
}

@Test
public void passwordResetByUsername() throws Exception {
// create user in repo
User user = ModelFactory.generateActivatedUser("authenticateduser");
User createdUser = userCreationService.createUser(new ManagedUserVM(user));

Optional<User> userBefore = userRepository.findOneByEmailIgnoreCase(createdUser.getEmail());
assertThat(userBefore).isPresent();
String resetKeyBefore = userBefore.get().getResetKey();

// init password reset
// no helper method from request can be used since the String needs to be transferred unaltered; The helpers would add quotes around it
// previous, faulty call:
// request.postWithoutLocation("/api/account/reset-password/init", createdUser.getEmail(), HttpStatus.OK, null);
var req = MockMvcRequestBuilders.post(new URI("/api/account/reset-password/init")).contentType(MediaType.APPLICATION_JSON).content(createdUser.getLogin());
request.getMvc().perform(req).andExpect(status().is(HttpStatus.OK.value())).andReturn();
ReflectionTestUtils.invokeMethod(request, "restoreSecurityContext");

verifyPasswordReset(createdUser, resetKeyBefore);
}

private void verifyPasswordReset(User createdUser, String resetKeyBefore) throws Exception {
// check user data
Optional<User> userPasswordResetInit = userRepository.findOneByEmailIgnoreCase(createdUser.getEmail());
assertThat(userPasswordResetInit).isPresent();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
import { ElementRef } from '@angular/core';
import { ComponentFixture, TestBed, inject } from '@angular/core/testing';
import { FormBuilder } from '@angular/forms';
import { FormBuilder, NgModel } from '@angular/forms';
import { of, throwError } from 'rxjs';
import { ArtemisTestModule } from '../../test.module';
import { PasswordResetInitComponent } from 'app/account/password-reset/init/password-reset-init.component';
import { PasswordResetInitService } from 'app/account/password-reset/init/password-reset-init.service';
import { ProfileService } from 'app/shared/layouts/profiles/profile.service';
import { MockProfileService } from '../../helpers/mocks/service/mock-profile.service';
import { AlertService } from 'app/core/util/alert.service';
import { MockComponent, MockProvider } from 'ng-mocks';
import { MockComponent, MockDirective, MockProvider } from 'ng-mocks';
import { MockTranslateService, TranslatePipeMock } from '../../helpers/mocks/service/mock-translate.service';
import { TranslateService } from '@ngx-translate/core';
import { MockNgbModalService } from '../../helpers/mocks/service/mock-ngb-modal.service';
import { NgbModal } from '@ng-bootstrap/ng-bootstrap';
import { AlertComponent } from 'app/shared/alert/alert.component';
import { By } from '@angular/platform-browser';

describe('PasswordResetInitComponent', () => {
let fixture: ComponentFixture<PasswordResetInitComponent>;
Expand All @@ -22,7 +22,7 @@ describe('PasswordResetInitComponent', () => {
beforeEach(() => {
return TestBed.configureTestingModule({
imports: [ArtemisTestModule],
declarations: [PasswordResetInitComponent, TranslatePipeMock, MockComponent(AlertComponent)],
declarations: [PasswordResetInitComponent, TranslatePipeMock, MockComponent(AlertComponent), MockDirective(NgModel)],
providers: [
FormBuilder,
{ provide: ProfileService, useClass: MockProfileService },
Expand All @@ -39,27 +39,20 @@ describe('PasswordResetInitComponent', () => {
});

it('sets focus after the view has been initialized', () => {
const node = {
focus(): void {},
};
comp.email = new ElementRef(node);
jest.spyOn(node, 'focus');
fixture.detectChanges();

comp.ngAfterViewInit();

expect(node.focus).toHaveBeenCalled();
const emailUsernameInput = fixture.debugElement.query(By.css('#emailUsername')).nativeElement;
const focusedElement = fixture.debugElement.query(By.css(':focus')).nativeElement;
expect(focusedElement).toBe(emailUsernameInput);
});

it('notifies of success upon successful requestReset', inject([PasswordResetInitService], (service: PasswordResetInitService) => {
jest.spyOn(service, 'save').mockReturnValue(of({}));
comp.resetRequestForm.patchValue({
email: '[email protected]',
});
comp.emailUsernameValue = '[email protected]';

comp.requestReset();

expect(service.save).toHaveBeenCalledWith('[email protected]');
expect(comp.success).toBe(true);
}));

it('no notification of success upon error response', inject([PasswordResetInitService], (service: PasswordResetInitService) => {
Expand All @@ -69,31 +62,26 @@ describe('PasswordResetInitComponent', () => {
data: 'something else',
})),
);
comp.resetRequestForm.patchValue({
email: '[email protected]',
});
comp.emailUsernameValue = '[email protected]';

comp.requestReset();

expect(service.save).toHaveBeenCalledWith('[email protected]');
expect(comp.success).toBe(false);
expect(comp.externalResetModalRef).toBe(undefined);
}));

it('no notification of success upon external user error response', inject([PasswordResetInitService], (service: PasswordResetInitService) => {
jest.spyOn(service, 'save').mockReturnValue(
throwError({
throwError(() => ({
status: 400,
error: { errorKey: 'externalUser' },
}),
})),
);
comp.useExternal = true;
comp.resetRequestForm.patchValue({
email: '[email protected]',
});
comp.emailUsernameValue = '[email protected]';
comp.requestReset();

expect(service.save).toHaveBeenCalledWith('[email protected]');
expect(comp.success).toBe(false);
expect(comp.externalResetModalRef).not.toBe(undefined); // External reference
}));
});

0 comments on commit 2f52056

Please sign in to comment.