Logged in user will not be detected if refreshing in AuthGuarded route

[blindstuff]

• I understand that GitHub issues are not for tech support, but for questions specific to this generator, bug reports, and feature requests.

Item	Version
generator-angular-fullstack	5.0.0 rc 4 + fix PRs
Node	8.10.0
npm	5.7.1
Operating System	Windows 10

Item	Answer
Transpiler	TypeScript
Markup	HTML
CSS	SCSS
Router	ngRoute
Client Tests	Mocha
DB	SQL
Auth	Y

A user that has sucesfully logged on will not be detected as logged in if the site is refreshed inside an AuthGuarded route.

Steps to reproduce bug:

1. Use generator
2. Log in as admin user
3. Click on Admin tab on nav bar
4. Use browser refresh

Cause:
Router is evaluating the CanActivate function before the constructor has retrevied the user from the Token

AuthGuard triggered

canActivate() {
	return this.authService.isLoggedIn();
}

isLoggedIn(callback?) {
	let is = !!this.currentUser._id;
	//at this time is continues to be undefined
	safeCb(callback)(is);
	return Promise.resolve(is);
}

AuthGuard Cancels Navigation

This part of the constructor finishes running and get the user too late.

if(localStorage.getItem('id_token')) {
	this.UserService.get().toPromise()
		.then((user: User) => {
			this.currentUser = user;
		})
		.catch(err => {
			console.log(err);

			localStorage.removeItem('id_token');
		});
}

Click on another authguarded route after this will work. Bug only occurs while refreshing.

[blindstuff]

Implemented the following solution, will submit a PR if in agreement with strategy. @Awk34

auth.service.ts:

constructor(private http: HttpClient, private userService: UserService) {
	this.http = http;
	this.UserService = userService;

	//Not calling this as it will get done the first time isLoggedIn() is called
        //TODO: Delete this comment block. Currently commented for clarity.
	/*
	if(localStorage.getItem('id_token')) {
		this.UserService.get().toPromise()
			.then((user: User) => {
				this.currentUser = user;
			})
			.catch(err => {
				console.log(err);

				localStorage.removeItem('id_token');
			});
	}
	*/
}

isLoggedIn(callback?) {
	let is = !!this.currentUser._id;
	if(is || !localStorage.getItem('id_token')){
		//Got the user locally, or there is no id_token stored
		safeCb(callback)(is);
		return Promise.resolve(is);
	}else{
		//User not yet loaded locally, but id_token is present
		return this.UserService.get().toPromise()
			.then((user: User) => {
				this.currentUser = user;
				return !!this.currentUser._id;
			})
			.catch(err => {
				console.log(err);
				localStorage.removeItem('id_token');
				return false;
			});
	}
}

auth-guard.service.ts:

//Change can activate to return a Promise
canActivate(): Promise<boolean> {
	return this.authService.isLoggedIn();
}

[koraysels]

Definitely an improvement, this works pretty good for me. Thanks!

[albert-92]

Cool solution @blindstuff (#2774 (comment))

How about taking it even a little bit further like:

auth.service.ts:

hasRolePromise(role, callback?) {
    let is = !!this.currentUser._id;
    if(is || !localStorage.getItem('id_token')) {
        //Got the user locally, or there is no id_token stored
        safeCb(callback)(is);
        return Promise.resolve(is);
    } else {
        //User not yet loaded locally, but id_token is present
        return this.UserService.get().toPromise()
            .then((user: User) => {
                this.currentUser = user;
                return this.hasRole(user.role, role)
            })
            .catch(err => {
                console.log(err);
                localStorage.removeItem('id_token');
                return false;
            });
    }
}

auth-guard.service.ts:

static parameters = [Router, AuthService];
constructor(
    private router: Router,
    private authService: AuthService
) {
    this.authService = authService;
    this.router = router;
}

canActivate(route: ActivatedRouteSnapshot): Promise<boolean> {
    return this.authService.hasRolePromise(route.data.role).then(is => {
        if (!is) {
            this.router.navigate(['/login']);
        }
        return is;
    });
}

and for example in the admin module
admin.module.ts:

const adminRoutes: Routes = [{
    path: 'admin',
    component: AdminComponent,
    canActivate: [AuthGuard],
    data: {role: 'admin'}
}];

[blindstuff]

Good idea @albert-92

I had solved this in my implementation by editing canActivate. Either route seems to do well, I preferred to keep them separate, but am open to either.

canActivate(route: ActivatedRouteSnapshot): Promise<boolean> {
        // this will be passed from the route config
        // on the data property
        const expectedRole = route.data.expectedRole;

        let promise = new Promise<boolean>((resolve,reject) => {
            this.authService.isLoggedIn().then(is => {
                resolve(this.authService.hasRole(this.authService.currentUser.role,expectedRole));
            });
        });

        return promise;
    }