Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Refactor] Authenticate User for Each Action #150

Merged
merged 8 commits into from
Oct 15, 2023

Conversation

juanarias93
Copy link
Collaborator

@juanarias93 juanarias93 commented Oct 14, 2023

Co-authored @andresconti & @nodescanses.

What && Why

Require user to be authenticated on each action, except for signup and signin/login.

How

  • Add callback for checking if user is authenticated in almost all controller actions.
  • Add check to validate that current_user.id is the same as the /user/:id, for all actions on UsersController.
  • Update HomeController removing the json response, since now it will require authentication.
  • Update tests and make sure are all green.

Links

How to Review

  • Review commit by commit recommended.
  • Review all at once recommended.

Screenshots

image
image

@juanarias93 juanarias93 force-pushed the refactor--authenticate_user_for_all_actions branch 2 times, most recently from 00af2b5 to 61fce6c Compare October 14, 2023 20:45
@juanarias93 juanarias93 force-pushed the refactor--authenticate_user_for_all_actions branch from 61fce6c to f785817 Compare October 14, 2023 21:19
@juanarias93
Copy link
Collaborator Author

@andresconti ya dejé el otro refactor pronto en #152. Salí de esta branch así tenía lo de la autenticación ya pronta y nos ahorrábamos conflictos después.
Cuando retomes este y dejes los tests funcionando rebaseo devuelta y ya quedan los dos prontos 💪

@andresconti
Copy link
Collaborator

@andresconti ya dejé el otro refactor pronto en #152. Salí de esta branch así tenía lo de la autenticación ya pronta y nos ahorrábamos conflictos después. Cuando retomes este y dejes los tests funcionando rebaseo devuelta y ya quedan los dos prontos 💪

Ahi fueron los cambios 💪

@andresconti andresconti marked this pull request as ready for review October 15, 2023 16:27
Copy link
Collaborator Author

@juanarias93 juanarias93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tremendo! 👏

Un par de comentarios chotos.. pero ya está para aprobar.
(por las dudas yo no voy a poder aprobarlo porque soy el autor del PR jeje)

api/app/controllers/users_controller.rb Outdated Show resolved Hide resolved
api/spec/requests/groups/requests_controller_spec.rb Outdated Show resolved Hide resolved
@andresconti andresconti merged commit 67abc88 into develop Oct 15, 2023
1 check passed
@andresconti
Copy link
Collaborator

Tremendo! 👏

Un par de comentarios chotos.. pero ya está para aprobar. (por las dudas yo no voy a poder aprobarlo porque soy el autor del PR jeje)

Me dejo aprobarlo ajaj, me tome el atrevimiento de mergearlo despues de agregar los ultimos cambios de los comment

@juanarias93 juanarias93 deleted the refactor--authenticate_user_for_all_actions branch October 15, 2023 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants