Feature/6207 Endpoint api/v1/sign_in now outputs randomized tokens in response upon sign-in #6216
Conversation
7riumph
requested review from
compwron,
FireLemons and
elasticspoon
as code owners
7riumph
changed the title
7riumph
changed the title
7riumph
changed the title
7riumph
changed the title
|
looks good to me so far 🔥 I think instead of storing the token and refresh token as plaintext in the db, we can store it as a hash Then the client would be provided the plaintext versions of the tokens and whenever they call the api, the hash versions will be compared |
| @@ -2,6 +2,10 @@ class Api::V1::Users::SessionsController < Api::V1::BaseController | |||
| def create | |||
| load_resource | |||
| if @user | |||
|
|
|||
| @user.regenerate_session_token! | |||
| @user.regenerate_refresh_token! | |||
There was a problem hiding this comment.
It is actually for two separate tokens. The plan is the session_token expires quickly and is repeatedly replaced for as long as the refresh_token is valid to maintain access.
app/models/supervisor.rb
Outdated
| @@ -81,10 +81,13 @@ def recently_unassigned_volunteers | |||
| # receive_email_notifications :boolean default(TRUE) | |||
| # receive_reimbursement_email :boolean default(FALSE) | |||
| # receive_sms_notifications :boolean default(FALSE), not null | |||
| # refresh_token :string | |||
There was a problem hiding this comment.
maybe make a new type of user, ApiUser?
There was a problem hiding this comment.
Looking into this, may complicate authorization. I'll just create a completely new table for api/v1 credentials so the user model and roles inheriting from it aren't overloaded.
Additionally, will attempt to do more compartmentalizing this PR through a concern :)
app/models/user.rb
Outdated
| after_create :skip_email_confirmation_upon_creation | ||
| after_create :create_preference_set | ||
| before_update :record_previous_email | ||
| has_secure_token :token, length: 36 | ||
| has_secure_token :refresh_token, length: 36 |
There was a problem hiding this comment.
36 what? seconds, minutes, days, weeks?
There was a problem hiding this comment.
Vague syntax refers to token length, in this case. The token is 36 characters long.
…or greater separation of concerns (SoC)
…ctions (randomization)
…ement and test BaseController for signed-in user endpoints)
What github issue is this PR for, if any?
Resolves #6207
What changed, and why?
How is this tested? (please write tests!) 💖💪
Note: if you see a flake in your test build in github actions, please post in slack #casa "Flaky test: " :) 💪
Note: We love capybara tests! If you are writing both haml/js and ruby, please try to test your work with tests at every level including system tests like https://github.com/rubyforgood/casa/tree/main/spec/system
Screenshots please :)
Run your local server and take a screenshot of your work! Try to include the URL of the page as well as the contents of the page.
Feelings gif (optional)
What gif best describes your feeling working on this issue? https://giphy.com/
How to embed: