robisim74 / AngularSPAWebAPI

Angular Single Page Application with an ASP.NET Core Web API that uses token authentication
MIT License
231 stars 59 forks source link

Question about the AuthGard #21

Closed TonyWoo closed 6 years ago

TonyWoo commented 7 years ago

Hi, Thanks for providing so good example.

I just got a question, when I try the authGard and refresh the page, and if the networking is slow, then the AuthGard will think current user don't have any roles. It will redirect me to the login page.

I think it's because below getUserInfo is async and it didn't return the result before we call the AuthGard?

@Injectable() export class AuthenticationService { // On bootstrap or refresh, tries to get user's data. if (this.tokenNotExpired()) { this.getUserInfo().subscribe( (userInfo: any) => { this.changeUser(userInfo); }); }

I'm not sure how to fix this, can you please help take a look?

Thanks.

robisim74 commented 7 years ago

Thanks for reporting. Got it. Recently I changed the AuthGuard to protect the pages also by the role. Later I'll give it a look (yes, the problem is that we have two async requests, one for the authentication, and one for the user's info).

aalmajanob commented 7 years ago

Hi TonyWoo, Roberto,

I have been playing with the Chrome Network options which simulates a slow conenction (dev. tools / Network / Throttling):

  constructor(
        protected http: Http,
        protected authHttp: AuthHttp,
        protected router: Router,
        private permissionsService: PermissionsService) {

        // On bootstrap or refresh, tries to get user´s data (including related info i.e. Employee Permissions etc...)
        this.getUserInfoOnLoad();
        this.permissionsService.getEmployeePermissionsInfoOnLoad();
    }
    /**
     * Retrieves user data from storage during on load context
     */
    private getUserInfoOnLoad() {
        // 1. Check for Token expiration
        if (this.tokenNotExpired()) {

            // 2. Check for Token is active (with Identity Introspection)
            this.getTokenInstrospection()
                .subscribe(
                (res: any) => {
                    if (res.active != null && res.active) {
                        // Retrieves data from storage
                        const signedInUserModel: ISignedinUserModel = this.getUserFromStorage();
                        // Tells all the subscribers about the new data & roles.
                        this.signedInUserSubject.next(signedInUserModel);
                        this.signedInRolesSubject.next(signedInUserModel.roles);
                    } else {
                        this.signout(); // signout if Token is not active
                        this.router.navigate(['/home']);
                    }
                },
                (error: any) => {
                    this.signout(); // signout on error (introspection value could not be resolved)
                    this.router.navigate(['/home']);

                    const errMsg = (error.message) ? error.message :
                        error.status ? `${error.status} - ${error.statusText}` : 'Server error';
                    console.log(errMsg);
                });
        } else {
            this.signout(); // signout if Token is expired
        }

    }

Regards, Andrés.

robisim74 commented 7 years ago

Hi Andrés, thanks a lot for the suggestion! I was in doubt if to store user info for security reasons, but yes, many auth libraries and samples do it, because the APIs are also protected server side. I think I'll implement it by default. However check for the role in AuthGuard is only an option.

aalmajanob commented 7 years ago

Hi Roberto,

I find a good approach to store just the minimun required (non sensitive) user information at the local or session storage (for shared applications) since the backend will prevent any kind of front-end "manipulation".

Regards,

robisim74 commented 7 years ago

@TonyWoo I pushed the changes: I created a new BowserStorage class, and also the user's info are stored, so the service tries to retrieve them without calling the UserInfo endpoint. Roles in AuthGuard should be fully supported now. This is the commit: https://github.com/robisim74/AngularSPAWebAPI/commit/8fa384c293e0c36a1a9fb3e4847ae8eceb52dc66. Clean the browser cache before trying the sample app.

@aalmajanob Thank you again.

TonyWoo commented 7 years ago

Hi @robisim74 , Thanks for you help. I just changed the code to save the user info in sessionStorage before also :)

Just has question, why we need to check the hasStorage and what's the Storage? Thank you.

BowserStorage this.hasStorage = typeof Storage !== 'undefined';

robisim74 commented 7 years ago

Storage is the interface of session/localStorage: https://developer.mozilla.org/en-US/docs/Web/API/Storage I check if it exists because it is a window object: so there aren't problems for those who use server side rendering.

TonyWoo commented 7 years ago

@robisim74 Thank you:)