Feat/token based authentication 2

[muttaqin1]

Add token refresh and logout feature.
resolved: #46 , #71

[bellaabdelouahab]

@muttaqin1 sorry for not paying attention before, but why would you send the refresh token to client ?
isn't suppose to be in server and each time the user tries to login with an out dated token we check if the refresh token still valid, if so then we send a new token

[bellaabdelouahab]

Can you consider those changes,
and take a look at this msg

[muttaqin1]

@muttaqin1 sorry for not paying attention before, but why would you send the refresh token to client ? isn't suppose to be in server and each time the user tries to login with an out dated token we check if the refresh token still valid, if so then we send a new token

By storing the refresh token in server what are you trying to achieve? I haven't seen this kind of implementation yet. So i was not aware about it. I will surely change the feature as you are saying but i want you to have a look at this video once.
@bellaabdelouahab

https://youtu.be/ahDQI7msr0A

[bellaabdelouahab]

@muttaqin1 sorry for not paying attention before, but why would you send the refresh token to client ? isn't suppose to be in server and each time the user tries to login with an out dated token we check if the refresh token still valid, if so then we send a new token

By storing the refresh token in server what are you trying to achieve? I haven't seen this kind of implementation yet. So i was not aware about it. I will surely change the feature as you are saying but i want you to have a look at this video once. @bellaabdelouahab

https://youtu.be/ahDQI7msr0A

well before I get into details just type this in google : where to store the refresh token key
@muttaqin1

[bellaabdelouahab]

by storing the refresh token on server side we achieve a secure way to generate an access token with out give user access to refresh token.
i might have confused you but when i said not to send it to client side i meant to not do it like this

{
    "tokens": {
        "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjY0YzkxYmExMWI1NGVmOWUxZTdlNmE4OCIsImlhdCI6MTY5MDkxMTEzNSwiZXhwIjoxNjkwOTk3NTM1fQ.Wxp2HUVLbCsNgVdzI7IDomjlTHgHufKCkYj0xx97M6A",
            "refreshToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjY0YzkxYmExMWI1NGVmOWUxZTdlNmE4OCIsImlhdCI6MTY5MDkxMTEzNSwiZXhwIjoxNjkxNTE1OTM1fQ.O6x_fbrwSF7JCt9UnJDBnqxlPm4dByT1kRB9pAV2laY"
    },

but instead when i user sign in we store the refresh token on httpOnly cookie
this way the client side wont have access to the refresh token and also since the backend and front end will share the same server we can add same site flag to limit the csrf attacks
@muttaqin1

[muttaqin1]

The frontend app receives the refresh token and the refresh token is stored in a secured cookie.(Rest client-server)
Whenever the client need to send the refresh token it takes the token from the cookie and send it to the server.
But definitely we can directly store the token in the secured cookie.I was confused a bit.
@bellaabdelouahab

[bellaabdelouahab]

The frontend app receives the refresh token and the refresh token is stored in a secured cookie.(Rest client-server) Whenever the client need to send the refresh token it takes the token from the cookie and send it to the server. But definitely we can directly store the token in the secured cookie.I was confused a bit. @bellaabdelouahab

if you store it in the cookie it can be accessed by any user or xss attacks but if we use httponly cookie along with same site cors policy we can securely generate access token without the refresh token being compromised

[muttaqin1]

The frontend app receives the refresh token and the refresh token is stored in a secured cookie.(Rest client-server) Whenever the client need to send the refresh token it takes the token from the cookie and send it to the server. But definitely we can directly store the token in the secured cookie.I was confused a bit. @bellaabdelouahab

if you store it in the cookie it can be accessed by any user or ssx attacks but if we use httponly cookie along with same site cors policy we can securely generate access token without the refresh token being compromised

Yes, you are right.
@bellaabdelouahab

[bellaabdelouahab]

@muttaqin1
and another thing,
i see that in refresh token function you re deleting the hole document ! can you explain why ?
code ref :

Student-Workflow-Organizer/backend-app/utils/authorization/generateTokens.js

Line 14 in 394231f

if (exists) await TokenModel.findByIdAndDelete(exists._id);

[muttaqin1]

@muttaqin1
and another thing,
i see that in refresh token function you re deleting the hole document ! can you explain why ?
code ref :

Student-Workflow-Organizer/backend-app/utils/authorization/generateTokens.js

Line 14 in 394231f

if (exists) await TokenModel.findByIdAndDelete(exists._id);

Every time we generate new token we also create a token record. If we dont delete the existing token record when we create new token. There will be duplicate token records which will be a serious issue.
@bellaabdelouahab

[bellaabdelouahab]

Every time we generate new token we also create a token record. If we dont delete the existing token record when we create new token. There will be duplicate token records which will be a serious issue. @bellaabdelouahab

but why deleting the refresh token tho ?
the refresh token should be deleted only if the user logs out ain't that correct ?
@muttaqin1
and sorry for this hard review 😁😂it's just i have time today to go throw all the code

[bellaabdelouahab]

I've viewed all files i think there is no more comments except the ones I already made

[muttaqin1]

Every time we generate new token we also create a token record. If we dont delete the existing token record when we create new token. There will be duplicate token records which will be a serious issue. @bellaabdelouahab

but why deleting the refresh token tho ?
the refresh token should be deleted only if the user logs out ain't that correct ?
@muttaqin1
and sorry for this hard review 😁😂it's just i have time today to go throw all the code

Okay i will change that. Its good, i want you to do hard reviews😁

[muttaqin1]

Can you test this for me?
@bellaabdelouahab

[bellaabdelouahab]

i will approve this pr since i will write unit tests for it while it's not big, but on your next pr please make sure to add requested changes