Together
Together copied to clipboard
Update User to have Administrator Field
Related Issues Project: Admin Dashboard Depends on:
Description
- Update
UserSchema
to have aadministrator
field. - field should be typed as a number
- Update
User.create
inserver/config/passport.js
to set value to 0 - Update current users in production to have 0 (@guel-codes may have some good advice.)
Acceptance Criteria
- All new users should have an administrator value of 0 when created
- Ensure user creation works in all aspects, from running Together to testing environment.
We might want to make the field a boolean type. Is there a reason you want it to be typed as a Number? Either way could work, I was just wondering
I have a solution made for this now. I would love to be assigned this. I also have code ready that prevents access to the adminDashboard route unless user has admin property of 1. However, try as I might, I could not configure the tests to pass. If anyone has more experience with cypress, I would love some help with making the tests pass.
Also just brainstorming the data model here a little bit, it might be beneficial to migrate to a "roles" field on the user model that stores an array of roles. That way someone could be an "admin" with all permissions but someone else could be a "moderator" with read-only etc.
And when they login to the Admin dashboard there can be a check of their roles array for what permissions they have
We might want to make the field a boolean type. Is there a reason you want it to be typed as a Number? Either way could work, I was just wondering
I proposed number because we can set certain access roles. For example:
Level 0 = default user / no access Level 1 = Moderator can access and delete Level 2 = Admin can access and approve the delete
The rough idea would allow some future flexibility as well if there's other features like editing current events and such. After reading your additional comment would you prefer an array of roles instead of just a number system?
I have a solution made for this now. I would love to be assigned this. I also have code ready that prevents access to the adminDashboard route unless user has admin property of 1. However, try as I might, I could not configure the tests to pass. If anyone has more experience with cypress, I would love some help with making the tests pass.
Hey there! Welcome in! I can take a look at the test in a few days. I'm currently traveling a bit and it's hard to get time to sit down to review.
@Caleb-Cohen If we're using the admin
field on the User to indicate what level of access a user has, I think it might make sense to reword to security-level
, admin-level
, or even role
. Something to indicate the integer value isn't just treated as a boolean since you might assume that if the field just reads admin
or administrator
.
It might also be worth considering storing these security levels as strings. I don't imagine the app will ever need such fine control of user privileges to that couldn't be handled by a short list ("user", "moderator", "admin", "superadmin"). This way the values are also more expressive and less easily confused if more roles are added later.
@cblanken I agree. It is ideal to have a role
field that holds an array of strings and then you can index on that field when querying for role based access. I think the first use case for the admin app could be to assign roles to others.
@Caleb-Cohen If we're using the
admin
field on the User to indicate what level of access a user has, I think it might make sense to reword tosecurity-level
,admin-level
, or evenrole
. Something to indicate the integer value isn't just treated as a boolean since you might assume that if the field just readsadmin
oradministrator
.It might also be worth considering storing these security levels as strings. I don't imagine the app will ever need such fine control of user privileges to that couldn't be handled by a short list ("user", "moderator", "admin", "superadmin"). This way the values are also more expressive and less easily confused if more roles are added later.
I agree with this. Thanks for the feedback @cblanken and @guel-codes. @vguzman812 can you update PR to reflect changes?
@vguzman812 let me know if you want to pair up on this and we can
@vguzman812 let me know if you want to pair up on this and we can
Already submitted the new PR a few days ago. The testing for it needs to be done if you're interested in that. I don't know enough cypress to add administrative authorization testing.