Skip to content
This repository has been archived by the owner on Jul 6, 2020. It is now read-only.

Auth guards #282

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Auth guards #282

wants to merge 10 commits into from

Conversation

jayaike
Copy link
Contributor

@jayaike jayaike commented Jan 26, 2020

Fixes # #271

Changes proposed in this pull request:

  • Implementation of Route guard to prevent a user from seeing protected routes at all before the redirect.

Previously:

bandicam-2020-01-15-07-01-01-050_Trim

Currently:

routeguard

@jayaike
Copy link
Contributor Author

jayaike commented Jan 26, 2020

@Sanji515 Please take a look

@jayaike
Copy link
Contributor Author

jayaike commented Jan 26, 2020

@Ram81 @RishabhJain2018 @vkartik97 PTAL

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@a74e917). Click here to learn what that means.
The diff coverage is 31.91%.

@@            Coverage Diff            @@
##             master     #282   +/-   ##
=========================================
  Coverage          ?   50.63%           
=========================================
  Files             ?       66           
  Lines             ?     3768           
  Branches          ?      443           
=========================================
  Hits              ?     1908           
  Misses            ?     1766           
  Partials          ?       94
Impacted Files Coverage Δ
...bliclists/challengelist/challengelist.component.ts 47.57% <ø> (ø)
...rc/app/components/dashboard/dashboard.component.ts 90.9% <ø> (ø)
...bmissions/challengeviewallsubmissions.component.ts 31.73% <100%> (ø)
...rc/app/components/challenge/challenge.component.ts 87.85% <14.28%> (ø)
src/app/app.component.ts 35.21% <25%> (ø)
...e/challengesettings/challengesettings.component.ts 45.16% <33.33%> (ø)
...lengeleaderboard/challengeleaderboard.component.ts 34.07% <33.33%> (ø)
src/app/services/endpoints.service.ts 64.89% <50%> (ø)
Impacted Files Coverage Δ
...bliclists/challengelist/challengelist.component.ts 47.57% <ø> (ø)
...rc/app/components/dashboard/dashboard.component.ts 90.9% <ø> (ø)
...bmissions/challengeviewallsubmissions.component.ts 31.73% <100%> (ø)
...rc/app/components/challenge/challenge.component.ts 87.85% <14.28%> (ø)
src/app/app.component.ts 35.21% <25%> (ø)
...e/challengesettings/challengesettings.component.ts 45.16% <33.33%> (ø)
...lengeleaderboard/challengeleaderboard.component.ts 34.07% <33.33%> (ø)
src/app/services/endpoints.service.ts 64.89% <50%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a74e917...02a743f. Read the comment docs.

@@ -35,6 +35,8 @@ import {HostAnalyticsComponent} from './components/analytics/host-analytics/host
import {ResetPasswordComponent} from './components/auth/reset-password/reset-password.component';
import {ResetPasswordConfirmComponent} from './components/auth/reset-password-confirm/reset-password-confirm.component';

import { AuthGuard } from './guards/auth-guard.service';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a better name: Authentication/authorization?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is normal convention

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return true;
}

const tree: UrlTree = this.router.parseUrl(routePath);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure how this will help. I think UrlTree was introduced as return type from Angular 7 version.

Isn't it we can do this as well simply:

return isAuthenticated

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't because it will behave the same way as above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check out the GIF in the PR description. You will see the difference

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants