Closed o-bm closed 7 months ago
Resolved all conflicts with main branch
I think enum would be sufficient for the role. It is up to preference though
yeah.. I could see that its faster and we don't have many roles anyways but for now I think its fine if we keep it the way it is
Updated and reviewed all comments close #43
Responded to all comments.
Note: I had to alter the db db.sequelize.sync({ alter: true }).then(async ()
because it didn't have the role field in the User model.
I also tested it out and it works properly. You can send a request using Postman and specify the headers as
Notice that it created a new user khamisomar
with id 7 we can see that from the JSON response and we see that it searches in the table and then inserts since khamisomar
user was not in the table
The case where user is already in the table works as well and I tested it
Would we have to alter the db as well?
So creating a user is functional now?
yes
Would we have to alter the db as well?
No. once merged the sequelize database is new i.e all the previous stuff in it is deleted. shouldn't affect anything, you can just create your new users/stuff
How user data is being appended to the request?
When a request is received, it checks for a utorid in the request headers for the user. If a user with the given utorid exists in the database, the middleware retrieves this user and attaches it to the req object as req.user. If the user does not exist it creates a new user and then attaches this new user object to the req.
How to access user data from the request?
I think there is a misunderstanding here. Accessing the user data using the request's headers is to send stuff to the client / further middleware after the login. HTTP requests are independent. So if user logs in then decides to post something (which is another request) these actions involve two separate requests.
We will need to look into sessions which "links" the gap between independent requests. I will do this in a separate PR as this PR is getting a bit messy with the check role stuff as well.
Here is the general outline User requests to login > Authentication Middleware > Store the user info for this session > Then we can check the role etc for subsequent request.
Which routes require what role? What does it mean if a route does not check for roles?
I am happy to discuss this in our meeting
Are roles mutually exclusive? Can a user have more than one role?
No. The only two roles (for now) is user and admin. I don't see the need for users to have multiple roles.
I also see that some routes have
[user, admin]
as allowed roles. But doesn't that mean that that route can be accessed by anyone? If so, we might as well not check the roles. Note that shibboleth already protects all routes, so the user is guaranteed to have a valid utorid if they are accessing a route.
You are correct on this. I prefer having the [user, admin]
on even routes that anyone can access so we have the same template across
Note: I will be removing everything related to roles in this PR and have a separate PR for it and session management
Added a role model to the database and integrated with user Added middle ware check for admin or user Change the routes so it checks for the privileges