Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Secure api with express-jwt (wip) #82

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions client/app/app.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,22 @@
<a routerLink="/" class="nav-item nav-link" routerLinkActive="active" [routerLinkActiveOptions]="{exact:true}">
<i class="fa fa-home"></i> Home
</a>
<a routerLink="/cats" class="nav-item nav-link" routerLinkActive="active">
<a routerLink="/cats" class="nav-item nav-link" routerLinkActive="active" *ngIf="auth.loggedIn()">
<i class="fa fa-list"></i> Cats
</a>
<a routerLink="/login" class="nav-item nav-link" routerLinkActive="active" *ngIf="!auth.loggedIn">
<a routerLink="/login" class="nav-item nav-link" routerLinkActive="active" *ngIf="!auth.loggedIn()">
<i class="fa fa-sign-in"></i> Login
</a>
<a routerLink="/register" class="nav-item nav-link" routerLinkActive="active" *ngIf="!auth.loggedIn">
<a routerLink="/register" class="nav-item nav-link" routerLinkActive="active" *ngIf="!auth.loggedIn()">
<i class="fa fa-user-plus"></i> Register
</a>
<a routerLink="/account" class="nav-item nav-link" routerLinkActive="active" *ngIf="auth.loggedIn">
<a routerLink="/account" class="nav-item nav-link" routerLinkActive="active" *ngIf="auth.loggedIn()">
<i class="fa fa-user"></i> Account ({{auth.currentUser.username}})
</a>
<a routerLink="/admin" class="nav-item nav-link" routerLinkActive="active" *ngIf="auth.loggedIn && auth.isAdmin">
<a routerLink="/admin" class="nav-item nav-link" routerLinkActive="active" *ngIf="auth.loggedIn() && auth.isAdmin">
<i class="fa fa-lock"></i> Admin
</a>
<a routerLink="/logout" class="nav-item nav-link" routerLinkActive="active" *ngIf="auth.loggedIn">
<a routerLink="/logout" class="nav-item nav-link" routerLinkActive="active" *ngIf="auth.loggedIn()">
<i class="fa fa-sign-out"></i> Logout
</a>
</div>
Expand Down
8 changes: 2 additions & 6 deletions client/app/app.module.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import { NgModule, CUSTOM_ELEMENTS_SCHEMA } from '@angular/core';

import { RoutingModule } from './routing.module';
import { AuthModule } from './auth.module';
import { SharedModule } from './shared/shared.module';
import { CatService } from './services/cat.service';
import { UserService } from './services/user.service';
import { AuthService } from './services/auth.service';
import { AuthGuardLogin } from './services/auth-guard-login.service';
import { AuthGuardAdmin } from './services/auth-guard-admin.service';
import { AppComponent } from './app.component';
import { CatsComponent } from './cats/cats.component';
import { AboutComponent } from './about/about.component';
Expand All @@ -30,13 +28,11 @@ import { NotFoundComponent } from './not-found/not-found.component';
NotFoundComponent
],
imports: [
AuthModule,
RoutingModule,
SharedModule
],
providers: [
AuthService,
AuthGuardLogin,
AuthGuardAdmin,
CatService,
UserService
],
Expand Down
30 changes: 30 additions & 0 deletions client/app/auth.module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { NgModule } from '@angular/core';
import { Http, RequestOptions } from '@angular/http';
import { AuthHttp, AuthConfig } from 'angular2-jwt';

import { AuthService } from './services/auth.service';
import { AuthGuardLogin } from './services/auth-guard-login.service';
import { AuthGuardAdmin } from './services/auth-guard-admin.service';

export function authHttpServiceFactory(http: Http, options: RequestOptions) {
return new AuthHttp(new AuthConfig({
tokenName: 'token',
tokenGetter: (() => localStorage.getItem('token')),
globalHeaders: [{'Content-Type': 'application/json'}],
}), http, options);
}

@NgModule({
providers: [
AuthService,
AuthGuardLogin,
AuthGuardAdmin,
{
provide: AuthHttp,
useFactory: authHttpServiceFactory,
deps: [Http, RequestOptions]
}
]
})

export class AuthModule { }
2 changes: 1 addition & 1 deletion client/app/login/login.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class LoginComponent implements OnInit {
public toast: ToastComponent) { }

ngOnInit() {
if (this.auth.loggedIn) {
if (this.auth.loggedIn()) {
this.router.navigate(['/']);
}
this.loginForm = this.formBuilder.group({
Expand Down
6 changes: 5 additions & 1 deletion client/app/services/auth-guard-login.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ export class AuthGuardLogin implements CanActivate {
constructor(public auth: AuthService, private router: Router) {}

canActivate() {
return this.auth.loggedIn;
if (this.auth.loggedIn()) {
Copy link

Choose a reason for hiding this comment

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

you can shrink this: return this.auth.loggedIn();

Copy link
Author

Choose a reason for hiding this comment

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

At the time of writing while referring to angular2-jwt documentation I considered that but just left the code ready for a redirect like in the usage example. Thought that I'd let Davide see the pull request if helpful or requires changes.

Copy link
Owner

@DavideViolante DavideViolante Oct 10, 2017

Choose a reason for hiding this comment

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

It's ok, mabye just add a comment stating that a redirection would fit well in those lines.

return true;
} else {
return false;
}
}

}
11 changes: 6 additions & 5 deletions client/app/services/auth.service.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { Injectable } from '@angular/core';
import { Router } from '@angular/router';
import { JwtHelper } from 'angular2-jwt';
import { JwtHelper, tokenNotExpired } from 'angular2-jwt';

import { UserService } from '../services/user.service';

@Injectable()
export class AuthService {
loggedIn = false;
isAdmin = false;

jwtHelper: JwtHelper = new JwtHelper();
Expand All @@ -28,14 +27,13 @@ export class AuthService {
localStorage.setItem('token', res.token);
const decodedUser = this.decodeUserFromToken(res.token);
this.setCurrentUser(decodedUser);
return this.loggedIn;
return this.loggedIn();
}
);
}

logout() {
localStorage.removeItem('token');
this.loggedIn = false;
this.isAdmin = false;
this.currentUser = { _id: '', username: '', role: '' };
this.router.navigate(['/']);
Expand All @@ -46,12 +44,15 @@ export class AuthService {
}

setCurrentUser(decodedUser) {
this.loggedIn = true;
this.currentUser._id = decodedUser._id;
this.currentUser.username = decodedUser.username;
this.currentUser.role = decodedUser.role;
decodedUser.role === 'admin' ? this.isAdmin = true : this.isAdmin = false;
delete decodedUser.role;
}

loggedIn() {
return tokenNotExpired();
}

}
18 changes: 8 additions & 10 deletions client/app/services/cat.service.ts
Original file line number Diff line number Diff line change
@@ -1,39 +1,37 @@
import { Injectable } from '@angular/core';
import { Http, Headers, RequestOptions } from '@angular/http';
import { AuthHttp } from 'angular2-jwt';

import { Observable } from 'rxjs/Observable';
import 'rxjs/add/operator/map';

@Injectable()
export class CatService {

private headers = new Headers({ 'Content-Type': 'application/json', 'charset': 'UTF-8' });
private options = new RequestOptions({ headers: this.headers });

constructor(private http: Http) { }
constructor(public authHttp: AuthHttp) { }

getCats(): Observable<any> {
return this.http.get('/api/cats').map(res => res.json());
return this.authHttp.get('/api/cats').map(res => res.json());
}

countCats(): Observable<any> {
return this.http.get('/api/cats/count').map(res => res.json());
return this.authHttp.get('/api/cats/count').map(res => res.json());
}

addCat(cat): Observable<any> {
return this.http.post('/api/cat', JSON.stringify(cat), this.options);
return this.authHttp.post('/api/cat', JSON.stringify(cat));
}

getCat(cat): Observable<any> {
return this.http.get(`/api/cat/${cat._id}`).map(res => res.json());
return this.authHttp.get(`/api/cat/${cat._id}`).map(res => res.json());
}

editCat(cat): Observable<any> {
return this.http.put(`/api/cat/${cat._id}`, JSON.stringify(cat), this.options);
return this.authHttp.put(`/api/cat/${cat._id}`, JSON.stringify(cat));
}

deleteCat(cat): Observable<any> {
return this.http.delete(`/api/cat/${cat._id}`, this.options);
return this.authHttp.delete(`/api/cat/${cat._id}`);
}

}
16 changes: 9 additions & 7 deletions client/app/services/user.service.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Injectable } from '@angular/core';
import { Http, Headers, RequestOptions } from '@angular/http';
import { AuthHttp } from 'angular2-jwt';

import { Observable } from 'rxjs/Observable';
import 'rxjs/add/operator/map';
Expand All @@ -10,7 +11,8 @@ export class UserService {
private headers = new Headers({ 'Content-Type': 'application/json', 'charset': 'UTF-8' });
private options = new RequestOptions({ headers: this.headers });

constructor(private http: Http) { }
constructor(private http: Http,
public authHttp: AuthHttp) { }

register(user): Observable<any> {
return this.http.post('/api/user', JSON.stringify(user), this.options);
Expand All @@ -21,27 +23,27 @@ export class UserService {
}

getUsers(): Observable<any> {
return this.http.get('/api/users').map(res => res.json());
return this.authHttp.get('/api/users').map(res => res.json());
}

countUsers(): Observable<any> {
return this.http.get('/api/users/count').map(res => res.json());
return this.authHttp.get('/api/users/count').map(res => res.json());
}

addUser(user): Observable<any> {
return this.http.post('/api/user', JSON.stringify(user), this.options);
return this.authHttp.post('/api/user', JSON.stringify(user));
}

getUser(user): Observable<any> {
return this.http.get(`/api/user/${user._id}`).map(res => res.json());
return this.authHttp.get(`/api/user/${user._id}`).map(res => res.json());
}

editUser(user): Observable<any> {
return this.http.put(`/api/user/${user._id}`, JSON.stringify(user), this.options);
return this.authHttp.put(`/api/user/${user._id}`, JSON.stringify(user));
}

deleteUser(user): Observable<any> {
return this.http.delete(`/api/user/${user._id}`, this.options);
return this.authHttp.delete(`/api/user/${user._id}`);
}

}
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"core-js": "^2.4.1",
"dotenv": "^4.0.0",
"express": "^4.15.3",
"express-jwt": "^5.3.0",
"font-awesome": "^4.7.0",
"jquery": "^3.2.1",
"jsonwebtoken": "^7.4.1",
Expand Down
98 changes: 67 additions & 31 deletions server/controllers/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,57 +4,93 @@ abstract class BaseCtrl {

// Get all
getAll = (req, res) => {
this.model.find({}, (err, docs) => {
if (err) { return console.error(err); }
res.json(docs);
});
if (!req.payload.user._id) {
res.status(401).json({
'message' : 'UnauthorizedError: private'
});
Copy link
Owner

Choose a reason for hiding this comment

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

These 4 lines (7-10) are repeated throughout the entire file. There is surely a better way to write this part, in a way to respect the DRY principle.

Choose a reason for hiding this comment

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

i am trying to say that i have already implemented similar kind of workout

} else {
this.model.find({}, (err, docs) => {
if (err) { return console.error(err); }
res.json(docs);
});
}
};

// Count all
count = (req, res) => {
this.model.count((err, count) => {
if (err) { return console.error(err); }
res.json(count);
});
if (!req.payload.user._id) {
res.status(401).json({
'message' : 'UnauthorizedError: private'
});
} else {
this.model.count((err, count) => {
if (err) { return console.error(err); }
res.json(count);
});
}
};

// Insert
insert = (req, res) => {
const obj = new this.model(req.body);
obj.save((err, item) => {
// 11000 is the code for duplicate key error
if (err && err.code === 11000) {
res.sendStatus(400);
}
if (err) {
return console.error(err);
}
res.status(200).json(item);
});
if (!req.payload.user._id) {
res.status(401).json({
'message' : 'UnauthorizedError: private'
});
} else {
const obj = new this.model(req.body);
obj.save((err, item) => {
// 11000 is the code for duplicate key error
if (err && err.code === 11000) {
res.sendStatus(400);
}
if (err) {
return console.error(err);
}
res.status(200).json(item);
});
}
};

// Get by id
get = (req, res) => {
this.model.findOne({ _id: req.params.id }, (err, obj) => {
if (err) { return console.error(err); }
res.json(obj);
});
if (!req.payload.user._id) {
res.status(401).json({
'message' : 'UnauthorizedError: private'
});
} else {
this.model.findOne({ _id: req.params.id }, (err, obj) => {
if (err) { return console.error(err); }
res.json(obj);
});
}
};

// Update by id
update = (req, res) => {
this.model.findOneAndUpdate({ _id: req.params.id }, req.body, (err) => {
if (err) { return console.error(err); }
res.sendStatus(200);
});
if (!req.payload.user._id) {
res.status(401).json({
'message' : 'UnauthorizedError: private'
});
} else {
this.model.findOneAndUpdate({ _id: req.params.id }, req.body, (err) => {
if (err) { return console.error(err); }
res.sendStatus(200);
});
}
};

// Delete by id
delete = (req, res) => {
this.model.findOneAndRemove({ _id: req.params.id }, (err) => {
if (err) { return console.error(err); }
res.sendStatus(200);
});
if (!req.payload.user._id) {
res.status(401).json({
'message' : 'UnauthorizedError: private'
});
} else {
this.model.findOneAndRemove({ _id: req.params.id }, (err) => {
if (err) { return console.error(err); }
res.sendStatus(200);
});
}
};
}

Expand Down
Loading