-
-
Notifications
You must be signed in to change notification settings - Fork 666
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
Changes from 3 commits
c6a3a37
6a08d2c
d454b3f
08f40d3
597f207
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 { } |
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}`); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,58 +4,95 @@ 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' | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, items) => { | ||
if (err) { return console.error(err); } | ||
res.json(items); | ||
}); | ||
} | ||
}; | ||
|
||
// 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); | ||
}); | ||
} | ||
}; | ||
|
||
} | ||
|
||
export default BaseCtrl; |
There was a problem hiding this comment.
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();
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.