Skip to content

Commit

Permalink
Extract credentials from authentication service (close #447)
Browse files Browse the repository at this point in the history
  • Loading branch information
sinedied committed Apr 8, 2019
1 parent 51c3d89 commit 36173b9
Show file tree
Hide file tree
Showing 21 changed files with 242 additions and 143 deletions.
2 changes: 2 additions & 0 deletions generators/app/templates/src/app/core/_core.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { TranslateModule } from '@ngx-translate/core';
import { RouteReusableStrategy } from './route-reusable-strategy';
<% if (props.auth) { -%>
import { AuthenticationService } from './authentication/authentication.service';
import { CredentialsService } from './authentication/credentials.service';
import { AuthenticationGuard } from './authentication/authentication.guard';
<% } -%>
import { I18nService } from './i18n.service';
Expand All @@ -25,6 +26,7 @@ import { CacheInterceptor } from './http/cache.interceptor';
providers: [
<% if (props.auth) { -%>
AuthenticationService,
CredentialsService,
AuthenticationGuard,
<% } -%>
I18nService,
Expand Down
2 changes: 1 addition & 1 deletion generators/app/templates/src/app/core/_index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
export * from './core.module';
<% if (props.auth) { -%>
export * from './authentication/authentication.service';
export * from './authentication/authentication.service.mock';
export * from './authentication/credentials.service';
export * from './authentication/authentication.guard';
<% } -%>
export * from './i18n.service';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { TestBed, inject } from '@angular/core/testing';
import { Router, RouterStateSnapshot } from '@angular/router';

import { AuthenticationService } from './authentication.service';
import { MockAuthenticationService } from './authentication.service.mock';
import { CredentialsService } from './credentials.service';
import { MockCredentialsService } from './credentials.service.mock';
import { AuthenticationGuard } from './authentication.guard';

describe('AuthenticationGuard', () => {
let authenticationGuard: AuthenticationGuard;
let authenticationService: MockAuthenticationService;
let credentialsService: MockCredentialsService;
let mockRouter: any;
let mockSnapshot: RouterStateSnapshot;

Expand All @@ -20,20 +20,20 @@ describe('AuthenticationGuard', () => {
TestBed.configureTestingModule({
providers: [
AuthenticationGuard,
{ provide: AuthenticationService, useClass: MockAuthenticationService },
{ provide: CredentialsService, useClass: MockCredentialsService },
{ provide: Router, useValue: mockRouter },
]
});
});

beforeEach(inject([
AuthenticationGuard,
AuthenticationService
CredentialsService
], (_authenticationGuard: AuthenticationGuard,
_authenticationService: MockAuthenticationService) => {
_credentialsService: MockCredentialsService) => {

authenticationGuard = _authenticationGuard;
authenticationService = _authenticationService;
credentialsService = _credentialsService;
}));

it('should have a canActivate method', () => {
Expand All @@ -46,7 +46,7 @@ describe('AuthenticationGuard', () => {

it('should return false and redirect to login if user is not authenticated', () => {
// Arrange
authenticationService.credentials = null;
credentialsService.credentials = null;

// Act
const result = authenticationGuard.canActivate(null, mockSnapshot);
Expand All @@ -60,7 +60,7 @@ describe('AuthenticationGuard', () => {
});

it('should save url as queryParam if user is not authenticated', () => {
authenticationService.credentials = null;
credentialsService.credentials = null;
mockRouter.url = '/about';
mockSnapshot.url = '/about';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,18 @@ import { Injectable } from '@angular/core';
import { Router, CanActivate, ActivatedRouteSnapshot, RouterStateSnapshot } from '@angular/router';

import { Logger } from '../logger.service';
import { AuthenticationService } from './authentication.service';
import { CredentialsService } from './credentials.service';

const log = new Logger('AuthenticationGuard');

@Injectable()
export class AuthenticationGuard implements CanActivate {

constructor(private router: Router,
private authenticationService: AuthenticationService) { }
private credentialsService: CredentialsService) { }

canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): boolean {
if (this.authenticationService.isAuthenticated()) {
if (this.credentialsService.isAuthenticated()) {
return true;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Observable, of } from 'rxjs';

import { Credentials, LoginContext } from './authentication.service';
import { LoginContext } from './authentication.service';
import { Credentials } from './credentials.service';

export class MockAuthenticationService {

Expand All @@ -21,8 +22,4 @@ export class MockAuthenticationService {
return of(true);
}

isAuthenticated(): boolean {
return !!this.credentials;
}

}
Original file line number Diff line number Diff line change
@@ -1,29 +1,28 @@
import { TestBed, inject, fakeAsync, tick } from '@angular/core/testing';

import { AuthenticationService, Credentials} from './authentication.service';

const credentialsKey = 'credentials';
import { AuthenticationService } from './authentication.service';
import { CredentialsService, Credentials } from './credentials.service';
import { MockCredentialsService } from './credentials.service.mock';

describe('AuthenticationService', () => {
let authenticationService: AuthenticationService;
let credentialsService: MockCredentialsService;

beforeEach(() => {
TestBed.configureTestingModule({
providers: [AuthenticationService]
providers: [{ provide: CredentialsService, useClass: MockCredentialsService }, AuthenticationService]
});
});

beforeEach(inject([
AuthenticationService
], (_authenticationService: AuthenticationService) => {
authenticationService = _authenticationService;
}));

afterEach(() => {
// Cleanup
localStorage.removeItem(credentialsKey);
sessionStorage.removeItem(credentialsKey);
});
beforeEach(inject(
[AuthenticationService, CredentialsService],
(_authenticationService: AuthenticationService, _credentialsService: MockCredentialsService) => {
authenticationService = _authenticationService;
credentialsService = _credentialsService;
credentialsService.credentials = null;
spyOn(credentialsService, 'setCredentials').and.callThrough();
}
));

describe('login', () => {
it('should return credentials', fakeAsync(() => {
Expand All @@ -42,7 +41,7 @@ describe('AuthenticationService', () => {
}));

it('should authenticate user', fakeAsync(() => {
expect(authenticationService.isAuthenticated()).toBe(false);
expect(credentialsService.isAuthenticated()).toBe(false);

// Act
const request = authenticationService.login({
Expand All @@ -53,11 +52,10 @@ describe('AuthenticationService', () => {

// Assert
request.subscribe(() => {
expect(authenticationService.isAuthenticated()).toBe(true);
expect(authenticationService.credentials).toBeDefined();
expect(authenticationService.credentials).not.toBeNull();
expect((<Credentials>authenticationService.credentials).token).toBeDefined();
expect((<Credentials>authenticationService.credentials).token).not.toBeNull();
expect(credentialsService.isAuthenticated()).toBe(true);
expect(credentialsService.credentials).not.toBeNull();
expect(credentialsService.credentials.token).toBeDefined();
expect(credentialsService.credentials.token).not.toBeNull();
});
}));

Expand All @@ -71,7 +69,8 @@ describe('AuthenticationService', () => {

// Assert
request.subscribe(() => {
expect(sessionStorage.getItem(credentialsKey)).not.toBeNull();
expect(credentialsService.setCredentials).toHaveBeenCalled();
expect((<jasmine.Spy>credentialsService.setCredentials).calls.mostRecent().args[1]).toBe(undefined);
});
}));

Expand All @@ -86,7 +85,8 @@ describe('AuthenticationService', () => {

// Assert
request.subscribe(() => {
expect(localStorage.getItem(credentialsKey)).not.toBeNull();
expect(credentialsService.setCredentials).toHaveBeenCalled();
expect((<jasmine.Spy>credentialsService.setCredentials).calls.mostRecent().args[1]).toBe(true);
});
}));
});
Expand All @@ -102,41 +102,14 @@ describe('AuthenticationService', () => {

// Assert
loginRequest.subscribe(() => {
expect(authenticationService.isAuthenticated()).toBe(true);

const request = authenticationService.logout();
tick();

request.subscribe(() => {
expect(authenticationService.isAuthenticated()).toBe(false);
expect(authenticationService.credentials).toBeNull();
expect(sessionStorage.getItem(credentialsKey)).toBeNull();
expect(localStorage.getItem(credentialsKey)).toBeNull();
});
});
}));

it('should clear persisted user authentication', fakeAsync(() => {
// Arrange
const loginRequest = authenticationService.login({
username: 'toto',
password: '123',
remember: true
});
tick();

// Assert
loginRequest.subscribe(() => {
expect(authenticationService.isAuthenticated()).toBe(true);
expect(credentialsService.isAuthenticated()).toBe(true);

const request = authenticationService.logout();
tick();

request.subscribe(() => {
expect(authenticationService.isAuthenticated()).toBe(false);
expect(authenticationService.credentials).toBeNull();
expect(sessionStorage.getItem(credentialsKey)).toBeNull();
expect(localStorage.getItem(credentialsKey)).toBeNull();
expect(credentialsService.isAuthenticated()).toBe(false);
expect(credentialsService.credentials).toBeNull();
});
});
}));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,35 +1,22 @@
import { Injectable } from '@angular/core';
import { Observable, of } from 'rxjs';

export interface Credentials {
// Customize received credentials here
username: string;
token: string;
}
import { Credentials, CredentialsService } from './credentials.service';

export interface LoginContext {
username: string;
password: string;
remember?: boolean;
}

const credentialsKey = 'credentials';

/**
* Provides a base for authentication workflow.
* The Credentials interface as well as login/logout methods should be replaced with proper implementation.
* The login/logout methods should be replaced with proper implementation.
*/
@Injectable()
export class AuthenticationService {

private _credentials: Credentials | null;

constructor() {
const savedCredentials = sessionStorage.getItem(credentialsKey) || localStorage.getItem(credentialsKey);
if (savedCredentials) {
this._credentials = JSON.parse(savedCredentials);
}
}
constructor(private credentialsService: CredentialsService) { }

/**
* Authenticates the user.
Expand All @@ -42,7 +29,7 @@ export class AuthenticationService {
username: context.username,
token: '123456'
};
this.setCredentials(data, context.remember);
this.credentialsService.setCredentials(data, context.remember);
return of(data);
}

Expand All @@ -52,43 +39,8 @@ export class AuthenticationService {
*/
logout(): Observable<boolean> {
// Customize credentials invalidation here
this.setCredentials();
this.credentialsService.setCredentials();
return of(true);
}

/**
* Checks is the user is authenticated.
* @return True if the user is authenticated.
*/
isAuthenticated(): boolean {
return !!this.credentials;
}

/**
* Gets the user credentials.
* @return The user credentials or null if the user is not authenticated.
*/
get credentials(): Credentials | null {
return this._credentials;
}

/**
* Sets the user credentials.
* The credentials may be persisted across sessions by setting the `remember` parameter to true.
* Otherwise, the credentials are only persisted for the current session.
* @param credentials The user credentials.
* @param remember True to remember credentials across sessions.
*/
private setCredentials(credentials?: Credentials, remember?: boolean) {
this._credentials = credentials || null;

if (credentials) {
const storage = remember ? localStorage : sessionStorage;
storage.setItem(credentialsKey, JSON.stringify(credentials));
} else {
sessionStorage.removeItem(credentialsKey);
localStorage.removeItem(credentialsKey);
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { Credentials } from './credentials.service';

export class MockCredentialsService {

credentials: Credentials | null = {
username: 'test',
token: '123'
};

isAuthenticated(): boolean {
return !!this.credentials;
}

setCredentials(credentials?: Credentials, _remember?: boolean) {
this.credentials = credentials || null;
}

}
Loading

0 comments on commit 36173b9

Please sign in to comment.