Closed entrotech closed 4 years ago
Found a bug - you can create or delete a project with anybody's loginId via postman if you add their loginId in the request body. (this will get closed with the next pull request) -->
Reopening this issue to remove ability for admin to update, delete, and create projects. The logic was incorrect for that part because there is not an isAdmin property in the user object of the request (only in the database table).
Need to ask Rosemary if admins should be allowed to delete projects.
@ramccarr @Dsomers74 Who can Create, Read, Update or Delete (CRUD) , whose projects?
recommendation for LADOT: If LADOT admins cannot delete users projects, then if a user complains that they put a project in and it's now gone, LADOT can confidently say, that no person has access to delete other than the user themselves. recommendation for USER: can delete own projects
recommendation for LADOT: If LADOT admins cannot update users projects, then if a user complains that they put a project in and it's now changed, LADOT can confidently say, that no person has access to update other than the user themselves. recommendation for USER: can update own projects
recommendation for LADOT: LADOT admins can read all projects recommendation for USER: can read their own projects
recommendation for LADOT: LADOT admins can create their own projects recommendation for USER: can create their own project
Do you agree with these recommendations or have questions?
The above recommendations has been approved (admins should not be allowed to delete or update projects)
@nclairesays - can you just verify that this is the current implementation in the back end, and then we can close this issue.
@KPHowley yes i can do that this weekend.
Regarding delete -- I was working on cypress tests, I discovered that the delete function has code/logic missing so the current version of the app on staging does not have that update yet. I should be able to push an update sunday or monday evening.
I'm pretty sure the Read, Create, and Update are correct. But I can also double check again just in case.
I started reviewing the test but the recent removal of the land use page (aka the change) made it necessary to change the corresponding parts of the test.
I also tried checking out a revision before the change for review but the staging database land use rules were changed from input to calculation to support the change. The live database should work but it's not for testing.
There are some bugs introduced by the change which the existing test cases exposed. This review will have to wait until these bugs are fixed.
Here are the updates made to this issue's test and to the 6 original tests. https://github.com/hackforla/tdm-calculator/tree/fang-update-tests
If a user is not an admin, only allow them to see their own projects
[x] Modify all project routes in node to extract the user's loginId from the JWT using existing middleware.
[x] For POST request, no further change required.
[x] For PUT request, Web API should return 403 if loginId does not match loginId of project.
[x] For DELETE request, stored procedure should accept user's loginId as a parameter, and only delete the project if it matches project.loginId
[x] For getAll and getById service methods, modify sproc and service to pass user's loginId to sproc, and modify sproc to only return projects that are authored by the user (unless the user has admin role).
[x] Hide Project Menu Item from NavBar if user is not logged in.