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

[blindstuff]

• [x] 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 (https://github.com/angular-fullstack/generator-angular-fullstack/issues/2774#issuecomment-437958115)

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;
    }