Skip to content
This repository was archived by the owner on Nov 25, 2020. It is now read-only.

Commit

Permalink
feat(authenticateStep): use authService.authenticate
Browse files Browse the repository at this point in the history
  • Loading branch information
doktordirk committed Jun 2, 2016
1 parent 7e64bbf commit 5b9306f
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 20 deletions.
19 changes: 10 additions & 9 deletions src/authenticateStep.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,24 @@
import {inject} from 'aurelia-dependency-injection';
import {Authentication} from './authentication';
import {Redirect} from 'aurelia-router';

@inject(Authentication)
import {AuthService} from './authService';

@inject(AuthService)
export class AuthenticateStep {
constructor(authentication) {
this.authentication = authentication;
constructor(authService) {
this.authService = authService;
}

run(routingContext, next) {
const isLoggedIn = this.authentication.isAuthenticated();
const loginRoute = this.authentication.config.loginRoute;
const isLoggedIn = this.authService.authenticated;

This comment has been minimized.

Copy link
@siimv

siimv Dec 29, 2016

I wonder why this change? I believe it causes a bug when using sessionStorage.

When I am using sessionStorage (and auth token is in the store) and I refresh the page, then this.authService.authenticated is false but this.authService.isAuthenticated() is true.

This comment has been minimized.

Copy link
@RWOverdijk

RWOverdijk Jan 3, 2017

Member

I think this is an oversight. I'm sure there's a reason for it, so we'll have to check.

const loginRoute = this.authService.config.loginRoute;

if (routingContext.getAllInstructions().some(i => i.config.settings.authenticate === true)) {
if (routingContext.getAllInstructions().some(route => route.config.settings.authenticate === true)) {
if (!isLoggedIn) {
return next.cancel(new Redirect(loginRoute));
}
} else if (isLoggedIn && routingContext.getAllInstructions().some(i => i.fragment === loginRoute)) {
return next.cancel(new Redirect( this.authentication.config.loginRedirect ));
} else if (isLoggedIn && routingContext.getAllInstructions().some(route => route.fragment === loginRoute)) {
return next.cancel(new Redirect( this.authService.config.loginRedirect ));
}

return next();
Expand Down
24 changes: 13 additions & 11 deletions test/authenticateStep.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,15 @@ const routes = {
describe('AuthenticateStep', () => {
describe('.run()', () => {
const authenticateStep = new Container().get(AuthenticateStep);
function next() {return;}
let loginRoute = authenticateStep.authentication.config.loginRoute;
let loginRoute = authenticateStep.authService.config.loginRoute;

it('should not redirect when not authenticated and no route requires it', () => {
let routingContext = {
getAllInstructions: () => routes.authenticateNone
};

function next() {return;}
next.cancel = redirect => {throw new Error();};

spyOn(next, 'cancel');

authenticateStep.run(routingContext, next);
Expand All @@ -46,6 +45,7 @@ describe('AuthenticateStep', () => {
getAllInstructions: () => routes.authenticateChild
};

function next() {return;}
next.cancel = redirect => {
expect(redirect.url).toBe(loginRoute);
done();
Expand All @@ -59,6 +59,7 @@ describe('AuthenticateStep', () => {
getAllInstructions: () => routes.authenticateParent
};

function next() {return;}
next.cancel = redirect => {
expect(redirect.url).toBe(loginRoute);
done();
Expand All @@ -72,11 +73,11 @@ describe('AuthenticateStep', () => {
getAllInstructions: () => routes.authenticateNone
};

function next() {return;}
next.cancel = redirect => {throw new Error();};

spyOn(next, 'cancel');

authenticateStep.authentication.isAuthenticated = () => true;
authenticateStep.authService.authenticated = true;

authenticateStep.run(routingContext, next);

Expand All @@ -88,11 +89,11 @@ describe('AuthenticateStep', () => {
getAllInstructions: () => routes.authenticateChild
};

function next() {return;}
next.cancel = redirect => {throw new Error();};

spyOn(next, 'cancel');

authenticateStep.authentication.isAuthenticated = () => true;
authenticateStep.authService.authenticated = true;

authenticateStep.run(routingContext, next);

Expand All @@ -104,11 +105,11 @@ describe('AuthenticateStep', () => {
getAllInstructions: () => routes.authenticateParent
};

function next() {return;}
next.cancel = redirect => {throw new Error();};

spyOn(next, 'cancel');

authenticateStep.authentication.isAuthenticated = () => true;
authenticateStep.authService.authenticated = true;

authenticateStep.run(routingContext, next);

Expand All @@ -120,12 +121,13 @@ describe('AuthenticateStep', () => {
getAllInstructions: () => routes.onLoginRoute
};

function next() {return;}
next.cancel = redirect => {
expect(redirect.url).toBe(authenticateStep.authentication.config.loginRedirect);
expect(redirect.url).toBe(authenticateStep.authService.config.loginRedirect);
done();
};

authenticateStep.authentication.isAuthenticated = () => true;
authenticateStep.authService.authenticated = true;

authenticateStep.run(routingContext, next);
});
Expand Down

0 comments on commit 5b9306f

Please sign in to comment.