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

admin can be added by anyone #12

Open
d0peCode opened this issue Jun 27, 2019 · 6 comments · May be fixed by #23
Open

admin can be added by anyone #12

d0peCode opened this issue Jun 27, 2019 · 6 comments · May be fixed by #23
Assignees
Labels
bug Something isn't working

Comments

@d0peCode
Copy link
Contributor

Currently new admin can be added by anyone.

@kasvith
Copy link
Owner

kasvith commented Jun 27, 2019

Hi, @BorysTyminski can you do a PR fixing the issue :)

@kasvith kasvith added the bug Something isn't working label Jun 27, 2019
@d0peCode
Copy link
Contributor Author

d0peCode commented Jun 28, 2019

How we want to solve this? I think admin should be only created by another admin. However at the start we don't have any admins.

Maybe we should have two other collections for user and admin? I also for sure would like to add email confirmation after registration and maybe IP saving on login and registration as it's very common.


I think this is pretty serious vulnerability. If some developer will use this boilerplate and didn't realize admin can be added by adding role to JSON which goes to API then potential cracker can access any admin endpoint.

@kasvith
Copy link
Owner

kasvith commented Jun 28, 2019

I think we need to support a default admin. Then he can only add other admins

This is pretty common in most products

@d0peCode
Copy link
Contributor Author

d0peCode commented Jun 28, 2019

You still want to have admins and users in one collection? I think we should split them to two different mongo collection and then rename auth.controller.js to user.controller.js and also create user.route.js and refactor auth.route.js to contain only this demonstration "/secret" routes.

@d0peCode
Copy link
Contributor Author

I'm about to create new PR with fix but it will contain lots of changes and I actually splited users and admin to two collections.

@kasvith
Copy link
Owner

kasvith commented Jun 30, 2019

You are good to go

@d0peCode d0peCode linked a pull request Jul 1, 2019 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants