-
Notifications
You must be signed in to change notification settings - Fork 0
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
Exercise 4 #11
Exercise 4 #11
Changes from all commits
303dc5a
d510c54
e037939
de45ca3
815f3bf
cd360a2
f41eba4
6f25488
3cb603c
2e42ac9
1f1eb90
cdc9006
039edb4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
const jwt = require('jsonwebtoken'); | ||
|
||
module.exports = ({ req }) => { | ||
let token = req.headers.authorization || ''; | ||
token = token.replace('Bearer ', ''); | ||
try { | ||
const decodedToken = jwt.verify(token, process.env.JWT_SECRET); | ||
return { token: decodedToken } | ||
} | ||
catch (e) { | ||
return {} | ||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
const { RESTDataSource } = require('apollo-datasource-rest'); | ||
const jwt = require('jsonwebtoken'); | ||
|
||
class AuthAPI extends RESTDataSource { | ||
|
||
constructor() { | ||
super(); | ||
} | ||
|
||
async createToken(userId) { | ||
return jwt.sign({ uId: userId }, process.env.JWT_SECRET, { algorithm: 'HS256', expiresIn: '1h' }); | ||
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.
|
||
} | ||
|
||
} | ||
|
||
module.exports = AuthAPI; |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,27 +1,36 @@ | ||||||
const { RESTDataSource } = require('apollo-datasource-rest'); | ||||||
const { v4: uuidv4 } = require('uuid'); | ||||||
const bcrypt = require('bcrypt'); | ||||||
|
||||||
class UsersAPI extends RESTDataSource { | ||||||
constructor() { | ||||||
super(); | ||||||
this.users = [ | ||||||
{ | ||||||
name: 'Peter', | ||||||
posts: [], | ||||||
}, | ||||||
{ | ||||||
name: 'Max', | ||||||
posts: [], | ||||||
}, | ||||||
]; | ||||||
this.users = []; | ||||||
} | ||||||
|
||||||
async getUser(name) { | ||||||
return this.users.find(user => user.name === name); | ||||||
async getUserById(id) { | ||||||
return this.users.find(user => user.id == id); | ||||||
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.
Suggested change
|
||||||
} | ||||||
|
||||||
async getUsers() { | ||||||
return this.users; | ||||||
} | ||||||
|
||||||
async getUserByEmail(email) { | ||||||
return this.users.find(user => user.email === email); | ||||||
} | ||||||
|
||||||
async createUser(name, email, password) { | ||||||
let obj = {id: uuidv4(), name: name, email: email, posts: [], password: await this.hashPassword(password)}; | ||||||
MaykAkifovski marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
this.users.push(obj); | ||||||
return obj; | ||||||
} | ||||||
|
||||||
async hashPassword(password) { | ||||||
const saltRounds = parseInt(process.env.SALT_ROUNDS); | ||||||
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. Make that a constant |
||||||
return bcrypt.hash(password, saltRounds); | ||||||
} | ||||||
|
||||||
} | ||||||
|
||||||
module.exports = UsersAPI; | ||||||
module.exports = UsersAPI; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,26 +1,41 @@ | ||
const { ApolloServer } = require('apollo-server'); | ||
const typeDefs = require('./typeDefs'); | ||
const resolvers = require('./resolvers'); | ||
const permissions = require('./security/permissions'); | ||
const UsersAPI = require('./datasources/userApi'); | ||
const PostsAPI = require('./datasources/postApi'); | ||
const AuthAPI = require('./datasources/authenticationApi'); | ||
require('dotenv').config(); | ||
const { applyMiddleware } = require('graphql-middleware'); | ||
const { makeExecutableSchema } = require('graphql-tools'); | ||
|
||
|
||
// DO NOT initialize the endpoint inside the dataSources function! | ||
// See https://github.com/apollographql/apollo-server/issues/3150 | ||
// Alternatively, set schema.polling.enable to false. (Haven't tested if this works tho) | ||
const usersApi = new UsersAPI(); | ||
const postsApi = new PostsAPI(); | ||
const authApi = new AuthAPI(); | ||
|
||
const context = require('./context'); | ||
|
||
const schema = applyMiddleware( | ||
makeExecutableSchema({ | ||
typeDefs, | ||
resolvers, | ||
}), | ||
permissions, | ||
); | ||
|
||
const server = new ApolloServer({ | ||
typeDefs, | ||
resolvers, | ||
schema, | ||
context: ({req}) => context({req}), | ||
dataSources: () => { | ||
return { | ||
usersApi: usersApi, | ||
postsApi: postsApi | ||
postsApi: postsApi, | ||
authApi: authApi, | ||
} | ||
} | ||
}); | ||
|
||
|
||
server.listen().then(({url}) => { | ||
console.log(`Server ready at ${url}`); | ||
}) | ||
}); |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,5 +1,3 @@ | ||||||||||||||
const { UserInputError } = require('apollo-server'); | ||||||||||||||
|
||||||||||||||
module.exports = { | ||||||||||||||
Query: { | ||||||||||||||
async posts(parent, args, { dataSources }) { | ||||||||||||||
|
@@ -11,57 +9,38 @@ module.exports = { | |||||||||||||
}, | ||||||||||||||
User: { | ||||||||||||||
async posts(parent, args , { dataSources }) { | ||||||||||||||
const posts = await dataSources.postsApi.getPosts(); | ||||||||||||||
return posts.filter(post => post.author === parent.name); | ||||||||||||||
let posts = await dataSources.postsApi.getPosts(); | ||||||||||||||
return posts.filter(post => post.author === parent.id); | ||||||||||||||
} | ||||||||||||||
}, | ||||||||||||||
Post: { | ||||||||||||||
author(parent) { | ||||||||||||||
async author(parent, args , { dataSources }) { | ||||||||||||||
const userById = await dataSources.usersApi.getUserById(parent.author); | ||||||||||||||
return { | ||||||||||||||
name: parent.author | ||||||||||||||
name: userById.name | ||||||||||||||
}; | ||||||||||||||
Comment on lines
+18
to
21
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. If you return a name like this then it's not possible to nest your queries 'indefinitely': {
posts {
title
votes
author {
name
posts {
title
}
}
}
} as result, the author's posts will be an empty list.
Suggested change
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. That's 100% correct! |
||||||||||||||
} | ||||||||||||||
}, | ||||||||||||||
Mutation:{ | ||||||||||||||
async write(parent, args, { dataSources }) { | ||||||||||||||
Mutation: { | ||||||||||||||
async write(parent, args, { token, dataSources }) { | ||||||||||||||
let { | ||||||||||||||
title, | ||||||||||||||
author: {name} | ||||||||||||||
} = args.post; | ||||||||||||||
|
||||||||||||||
// Check if a post with the title already exists. | ||||||||||||||
let persistedPost = await dataSources.postsApi.getPost(title); | ||||||||||||||
if (persistedPost !== undefined) { | ||||||||||||||
throw new UserInputError("Post with this title already exists.", {invalidArgs: [title]}); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// Check if the author exists | ||||||||||||||
let author = await dataSources.usersApi.getUser(name); | ||||||||||||||
if (author === undefined) { | ||||||||||||||
throw new UserInputError("No such author.", {invalidArgs: [author]}); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
return await dataSources.postsApi.createPost({title, author: name}); | ||||||||||||||
} = args.post; | ||||||||||||||
return await dataSources.postsApi.createPost({ title, author: token.uId }); | ||||||||||||||
}, | ||||||||||||||
async upvote(parent, args, { dataSources }) { | ||||||||||||||
// Why must we mock the current user? We have him right here, in the voter field? | ||||||||||||||
|
||||||||||||||
async upvote(parent, args, { token, dataSources }) { | ||||||||||||||
let postToUpvote = await dataSources.postsApi.getPost(args.title); | ||||||||||||||
if (postToUpvote === undefined) { | ||||||||||||||
throw new UserInputError("Post with this title doesn't exist", {invalidArgs: [args.title]}); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
let upvoter = await dataSources.usersApi.getUser(args.voter.name); | ||||||||||||||
if (upvoter === undefined) { | ||||||||||||||
throw new UserInputError("No such voter.", {invalidArgs: [args.voter]}); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
let alreadyVoted = postToUpvote.upvoters.includes(args.voter.name); | ||||||||||||||
if (alreadyVoted) { | ||||||||||||||
throw new UserInputError("This voter has already upvoted this article", {invalidArgs: [args.title, args.voter]}); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
return await dataSources.postsApi.upvotePost(postToUpvote, args.voter.name); | ||||||||||||||
return await dataSources.postsApi.upvotePost(postToUpvote, token.uId); | ||||||||||||||
}, | ||||||||||||||
async signup(parent, args, { dataSources }) { | ||||||||||||||
const createdUser = await dataSources.usersApi.createUser(args.name, args.email, args.password); | ||||||||||||||
const token = await dataSources.authApi.createToken(createdUser.id); | ||||||||||||||
return token; | ||||||||||||||
}, | ||||||||||||||
Comment on lines
+35
to
+39
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. ⭐ For an implemented signup feature. |
||||||||||||||
async login(parent, args, { dataSources }) { | ||||||||||||||
const user = await dataSources.usersApi.getUserByEmail(args.email); | ||||||||||||||
const token = await dataSources.authApi.createToken(user.id); | ||||||||||||||
return token; | ||||||||||||||
} | ||||||||||||||
Comment on lines
+40
to
44
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. ⭐ For an implemented login feature using JWT. |
||||||||||||||
} | ||||||||||||||
}; |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,43 @@ | ||||
const { isAuthenticated, isPostUpvoted, isPostWithTitlePresent, passwordIsValid, passwordIsTooShort, emailIsTaken, canSeeEmail} = require("./rules"); | ||||
const { shield, and, not, deny, allow, chain} = require('graphql-shield'); | ||||
const { UserInputError } = require('apollo-server'); | ||||
|
||||
const permissions = shield({ | ||||
Query: { | ||||
"*": allow, | ||||
}, | ||||
User: { | ||||
"*": deny, | ||||
name: allow, | ||||
posts: allow, | ||||
email: and(isAuthenticated, canSeeEmail), | ||||
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. You got a 🚀
|
||||
}, | ||||
Post: { | ||||
"*": allow, | ||||
}, | ||||
Mutation: { | ||||
"*": deny, | ||||
signup: and( | ||||
not(emailIsTaken, new UserInputError("A user with this email already exists.")), | ||||
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. ⭐ For unique email address validation. 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. Interesting, didn't know that |
||||
not(passwordIsTooShort, new UserInputError("The password must be at least 8 characters long.")) | ||||
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. ⭐ For password validation. |
||||
), | ||||
login: chain( | ||||
// This may look strange, but if an already authenticated user authenticates (logs in) a second time, she'll have two tokens! This is not good practice. | ||||
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. Why is this a problem? 🤔 |
||||
// There are two options: either implement a cache layer for the tokens, or just disallow authenticated users to login a second time. | ||||
not(isAuthenticated, new Error("Already logged in. Redirect to home page.")), | ||||
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. You would implement a redirect in the frontend, not the backend. |
||||
not(passwordIsTooShort, new UserInputError("The password must be at least 8 characters long.")), | ||||
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.
Suggested change
That shouldn't be necessary on |
||||
passwordIsValid, | ||||
), | ||||
write: and( | ||||
isAuthenticated, | ||||
not(isPostWithTitlePresent, new UserInputError("Post with this title already exists.")) | ||||
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. From my point of view, data validations like this shouldn't be part of your permission matrix. |
||||
), | ||||
upvote: and( | ||||
isAuthenticated, | ||||
isPostWithTitlePresent, | ||||
not(isPostUpvoted, new UserInputError("You've already upvoted this post")) | ||||
) | ||||
} | ||||
}); | ||||
|
||||
module.exports = permissions; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
const { rule } = require('graphql-shield'); | ||
const bcrypt = require('bcrypt'); | ||
|
||
|
||
const isAuthenticated = rule({ cache: 'contextual' })( | ||
async (parent, args, { token, dataSources }) => { | ||
const user = await dataSources.usersApi.getUserById(token.uId); | ||
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. 👍
|
||
return user !== undefined; | ||
} | ||
); | ||
|
||
const canSeeEmail = rule({ cache: 'strict' })( | ||
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. You could call this |
||
async (parent, args, { token }) => { | ||
return token.uId === parent.id; | ||
} | ||
); | ||
|
||
const emailIsTaken = rule({ cache: 'strict' })( | ||
async(parent, args, { token, dataSources }) => { | ||
const user = await dataSources.usersApi.getUserByEmail(args.email); | ||
return user !== undefined; | ||
} | ||
); | ||
|
||
const passwordIsTooShort = rule({ cache: 'strict' })( | ||
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 consider this as data-validation, so it shouldn't be part of |
||
async(parent, args, { token, dataSources }) => { | ||
return args.password.length < 8; | ||
} | ||
); | ||
|
||
const passwordIsValid = rule({ cache: 'strict' })( | ||
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. Also this logic should go into the |
||
async (parent, args, { dataSources }) => { | ||
const user = await dataSources.usersApi.getUserByEmail(args.email); | ||
return user !== undefined && bcrypt.compareSync(args.password, user.password); | ||
} | ||
); | ||
|
||
const isPostWithTitlePresent = rule({ cache: 'strict'})( | ||
async (parent, args, { dataSources }) => { | ||
let title; | ||
if (args.post === undefined) { | ||
title = args.title; | ||
} | ||
else { | ||
title = args.post.title | ||
} | ||
Comment on lines
+41
to
+46
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. You have this if-clause because you re-use this rule for different mutations with different |
||
|
||
const post = await dataSources.postsApi.getPost(title); | ||
return post !== undefined; | ||
} | ||
); | ||
|
||
const isPostUpvoted = rule({ cache: 'strict'})( | ||
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. This code should be as close to the |
||
async (parent, args, { dataSources, token }) => { | ||
const title = args.title; | ||
const post = await dataSources.postsApi.getPost(title); | ||
return post.upvoters.includes(token.uId); | ||
} | ||
); | ||
|
||
exports.isAuthenticated = isAuthenticated; | ||
exports.isPostUpvoted = isPostUpvoted; | ||
exports.isPostWithTitlePresent = isPostWithTitlePresent; | ||
exports.passwordIsValid = passwordIsValid; | ||
exports.passwordIsTooShort = passwordIsTooShort; | ||
exports.emailIsTaken = emailIsTaken; | ||
exports.canSeeEmail = canSeeEmail; | ||
Comment on lines
+61
to
+67
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. Use ES6 imports or |
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.
I would suggest to import a file here wich
require('dotenv')
and exports a constantJWT_SECRET
. You would avoid aprocess.env.JWT_SECRET
being undefined here, regardless of the order how files are loaded.