Distinguish user and admin #23
Conversation
|
Hi @d0peCode can you resolve the merge conflict? |
|
@kasvith I can tonight. At the moment I need to do some tasks in private repo |
|
Sure no problem :) |
|
@kasvith this branch has no conflicts and jarvis passed but additional code review is highly recommended :) |
|
Also I moved functions from models to |
|
Hi thanks @d0peCode for this awesome work. I will do a code review and merge soon. |
|
|
||
| userSchema.post('save', async function saved (doc, next) { | ||
| try { | ||
| const mailOptions = { | ||
| from: 'noreply', | ||
| to: this.email, | ||
| subject: 'Confirm creating account', | ||
| html: `<div><h1>Hello new user!</h1><p>Click <a href="${config.hostname}/api/auth/confirm?key=${this.activationKey}">link</a> to activate your new account.</p></div><div><h1>Hello developer!</h1><p>Feel free to change this template ;).</p></div>` | ||
| html: `<div><h1>Hello new user!</h1><p>Click <a href="${config.hostname}/api/user/confirm?key=${this.activationKey}">link</a> to activate your new account.</p></div><div><h1>Hello developer!</h1><p>Feel free to change this template ;).</p></div>` |
There was a problem hiding this comment.
config.hostname is not available, this will not work
There was a problem hiding this comment.
I added a config param. Check in config file
There was a problem hiding this comment.
But it seems I've forgotten to add it to .env.example somehow
There was a problem hiding this comment.
Oh I didn't know but wanted to keep this change so I renamed it to BASE_URI in next PR. Sorry for confusion.
There was a problem hiding this comment.
I added hostname because imagine we are running in something like kubernets. In that scenario when sending emails we should use the web address not the internal IP.
But I see the name is more confusing, we should rename it to something like siteUrl which makes more sense
There was a problem hiding this comment.
I agree, we can rename it and when this PR will be merged I'll resolve conflicts in second one.
| DEFAULT_ADMIN_NAME=admin | ||
| [email protected] | ||
| DEFAULT_ADMIN_PASSWORD=password |
There was a problem hiding this comment.
Can you add a new line at the end of file
| @@ -24,7 +22,7 @@ const handleJWT = (req, res, next, roles) => async (err, user, info) => { | |||
| } | |||
|
|
|||
| // see if user is authorized to do the action | |||
| if (!roles.includes(user.role)) { | |||
| if (role && role.includes('admin') && !user.admin) { | |||
There was a problem hiding this comment.
This method was used to add new roles to user model. Not just admin. There maybe an editor, moderator etc as many as possible.
In route definition we can add which roles can see the resouece. Like say you want a protected route only for admins and editors.
we can simply put
route.post('/page-edit', authorize(['admin', 'editor']))
This PR fix #12 bug. I distinguish user and admin. Changes:
authroute contain only demonstration/secretroutes, however there is no longer option to make only user access route (which was useless in my opinion anyway).utils/seed.jswhich create default admin with credentials fromconfig/index.js->.env.models/utilsdirectory to keep DRY principle as functions are used in two models now (admin, user) and I didn't want to copy them. How ever I didn't manage to moveSchema.method({ transform () { ...to external file, more about it in Move Schema.method transform to external file #22.Changed from original projecttoChangelog, deleted one point fromTodo.Please do code review.