Feature/6222 - Endpoint /api/v1/users/sign_out revokes access token and refresh token on request #6241
Conversation
|
👀 |
| let(:casa_org) { create(:casa_org) } | ||
| let(:volunteer) { create(:volunteer, casa_org: casa_org) } | ||
| let(:api_credential) { create(:api_credential, user: volunteer) } | ||
| let(:refresh_token) { api_credential.return_new_refresh_token![:refresh_token] } |
There was a problem hiding this comment.
Er, is this regenerating a refresh token a 2nd time?
There was a problem hiding this comment.
I think when you create the api credential table for the first time, the token digest fields are nil - at least in dev environment and so that is why I created the seed file to populate them.
I also need the plain text refresh token to be passed in the authorization header for the sign out.
There was a problem hiding this comment.
Looks good so far, as mentioned on the issues acceptance criteria and in our discussions. Still need to...
- Add the
sign_inroute toroutes.rb(session#destroy) - Add a destroy function to the
sessions_controller.rbusing therevocationfunction(s) helper you made - Address comments, test route with whatever you'd like to use though my preference is curl, then should lgtm 😎
xihai01
requested review from
compwron,
FireLemons and
elasticspoon
as code owners
| @@ -37,6 +37,14 @@ def is_refresh_token_expired? | |||
| refresh_token_expires_at < Time.current | |||
| end | |||
|
|
|||
| def revoke_api_token | |||
| users = User.all | ||
|
|
||
| users.each do |user| | ||
| ApiCredential.create!(user: user, api_token_digest: Digest::SHA256.hexdigest(SecureRandom.hex(18)), refresh_token_digest: Digest::SHA256.hexdigest(SecureRandom.hex(18))) |
There was a problem hiding this comment.
Would this make hashed values of refresh_token and refresh_token_digest different? Since your using a completely new SecureRandom.hex(18). If so, this should not be the case.
It should hash the current refresh_token.
| @@ -100,4 +100,22 @@ | |||
| expect(api_credential.refresh_token_digest).to eq(Digest::SHA256.hexdigest(refresh_token)) | |||
| end | |||
| end | |||
|
|
|||
| describe "#revoke_api_token" do | |||
| @@ -8,6 +8,22 @@ def create | |||
| end | |||
| end | |||
|
|
|||
| def destroy | |||
| # fetch refresh token from request header | |||
| refresh_token = request.headers["Authorization"]&.split(" ")&.last | |||
There was a problem hiding this comment.
Consider how we are structuring the response from the iOS app. If we're using the header, it's the access token or "api_token" that would go there, while the refresh token is in the body.
You'll need both in order to check whose tokens to revoke or set to nil on sign_out.
| # fetch refresh token from request header | ||
| refresh_token = request.headers["Authorization"]&.split(" ")&.last | ||
| # find user's api credentials by refresh token | ||
| api_credential = ApiCredential.find_by(refresh_token_digest: Digest::SHA256.hexdigest(refresh_token)) |
There was a problem hiding this comment.
api_credential = ApiCredential searches user based on only one token, instead do.
user_credentials = ApiCredential.find_by(
api_token_digest: Digest::SHA256.hexdigest(access_token),
refresh_token_digest: Digest::SHA256.hexdigest(refresh_token)
)
if user_credentials
user_credentials.revoke_api_token
user_credentials.revoke_refresh_token
render json: {message: "Signed out successfully."}, status: 200
else
...
What github issue is this PR for, if any?
Resolves #6222
What changed, and why?
Added /api/v1/users/sign_out endpoint so both the access and refresh tokens for that user is cleared from the api_credentials table and set to nil.
Why?: for added security - tokens should be removed from api_credentials table because user is no longer signed in.
How is this tested? (please write tests!) 💖💪
Token Destroyer Helper Function tests (2 in total)→ spec/models/api_credential_spec.rbSign Out Request Test for 200 and 401 Response Cases→ spec/requests/api/v1/users/sessions_spec.rbScreenshots please :)
Testing sign out with postman on localhost
Steps:
First we sign in to fetch the refresh token
Lastly we sign out and pass in refresh token in the request authorization header
Feelings gif (optional)