-
Notifications
You must be signed in to change notification settings - Fork 45
Alternate reset password flow, only 2 steps, no redirect #146
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module GraphqlDevise | ||
| module Mutations | ||
| class UpdatePasswordWithToken < Base | ||
| argument :password, String, required: true | ||
| argument :password_confirmation, String, required: true | ||
| argument :reset_password_token, String, required: true | ||
|
|
||
| field :credentials, | ||
| GraphqlDevise::Types::CredentialType, | ||
| null: true, | ||
| description: 'Authentication credentials. Resource must be signed_in in order for credentials to be returned.' | ||
|
|
||
| def resolve(reset_password_token:, **attrs) | ||
| raise_user_error(I18n.t('graphql_devise.passwords.password_recovery_disabled')) unless recoverable_enabled? | ||
|
|
||
| resource = resource_class.with_reset_password_token(reset_password_token) | ||
| raise_user_error(I18n.t('graphql_devise.passwords.reset_token_not_found')) if resource.blank? | ||
| raise_user_error(I18n.t('graphql_devise.passwords.reset_token_expired')) unless resource.reset_password_period_valid? | ||
|
|
||
| if resource.update(attrs) | ||
| yield resource if block_given? | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What kind of black can be passed here? Is there any examples of this?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any block you want, all mutations do the same. Check the specs in this PR for an example. |
||
|
|
||
| response_payload = { authenticatable: resource } | ||
| response_payload[:credentials] = set_auth_headers(resource) if controller.signed_in?(resource_name) | ||
|
|
||
| response_payload | ||
| else | ||
| raise_user_error_list( | ||
| I18n.t('graphql_devise.passwords.update_password_error'), | ||
| errors: resource.errors.full_messages | ||
| ) | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Mutations | ||
| class ResetAdminPasswordWithToken < GraphqlDevise::Mutations::UpdatePasswordWithToken | ||
| field :authenticatable, Types::AdminType, null: false | ||
|
|
||
| def resolve(reset_password_token:, **attrs) | ||
| super do |admin| | ||
| controller.sign_in(admin) | ||
| end | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require 'rails_helper' | ||
|
|
||
| RSpec.describe 'Update Password With Token' do | ||
| include_context 'with graphql query request' | ||
|
|
||
| let(:password) { '12345678' } | ||
| let(:password_confirmation) { password } | ||
|
|
||
| context 'when using the user model' do | ||
| let(:user) { create(:user, :confirmed) } | ||
| let(:query) do | ||
| <<-GRAPHQL | ||
| mutation { | ||
| userUpdatePasswordWithToken( | ||
| resetPasswordToken: "#{token}", | ||
| password: "#{password}", | ||
| passwordConfirmation: "#{password_confirmation}" | ||
| ) { | ||
| authenticatable { email } | ||
| credentials { accessToken } | ||
| } | ||
| } | ||
| GRAPHQL | ||
| end | ||
|
|
||
| context 'when reset password token is valid' do | ||
| let(:token) { user.send(:set_reset_password_token) } | ||
|
|
||
| it 'updates the password' do | ||
| expect do | ||
| post_request | ||
| user.reload | ||
| end.to change(user, :encrypted_password) | ||
|
|
||
| expect(json_response[:data][:userUpdatePasswordWithToken][:credentials]).to be_nil | ||
| expect(json_response[:data][:userUpdatePasswordWithToken][:authenticatable]).to include(email: user.email) | ||
| end | ||
|
|
||
| context 'when token has expired' do | ||
| it 'returns an expired token error' do | ||
| travel_to 10.hours.ago do | ||
| token | ||
| end | ||
|
|
||
| post_request | ||
|
|
||
| expect(json_response[:errors]).to contain_exactly( | ||
| hash_including(message: 'Reset password token is no longer valid.', extensions: { code: 'USER_ERROR' }) | ||
| ) | ||
| end | ||
| end | ||
|
|
||
| context 'when password confirmation does not match' do | ||
| let(:password_confirmation) { 'does not match' } | ||
|
|
||
| it 'returns an error' do | ||
| post_request | ||
|
|
||
| expect(json_response[:errors]).to contain_exactly( | ||
| hash_including( | ||
| message: 'Unable to update user password', | ||
| extensions: { code: 'USER_ERROR', detailed_errors: ["Password confirmation doesn't match Password"] } | ||
| ) | ||
| ) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| context 'when reset password token is not found' do | ||
| let(:token) { user.send(:set_reset_password_token) + 'invalid' } | ||
|
|
||
| it 'returns an error' do | ||
| post_request | ||
|
|
||
| expect(json_response[:errors]).to contain_exactly( | ||
| hash_including(message: 'No user found for the specified reset token.', extensions: { code: 'USER_ERROR' }) | ||
| ) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| context 'when using the admin model' do | ||
| let(:admin) { create(:admin, :confirmed) } | ||
| let(:query) do | ||
| <<-GRAPHQL | ||
| mutation { | ||
| adminUpdatePasswordWithToken( | ||
| resetPasswordToken: "#{token}", | ||
| password: "#{password}", | ||
| passwordConfirmation: "#{password_confirmation}" | ||
| ) { | ||
| authenticatable { email } | ||
| credentials { uid } | ||
| } | ||
| } | ||
| GRAPHQL | ||
| end | ||
|
|
||
| context 'when reset password token is valid' do | ||
| let(:token) { admin.send(:set_reset_password_token) } | ||
|
|
||
| it 'updates the password' do | ||
| expect do | ||
| post_request | ||
| admin.reload | ||
| end.to change(admin, :encrypted_password) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WDYT about checking the password is valid instead of |
||
|
|
||
| expect(json_response[:data][:adminUpdatePasswordWithToken]).to include( | ||
| credentials: { uid: admin.email }, | ||
| authenticatable: { email: admin.email } | ||
| ) | ||
| end | ||
| end | ||
| end | ||
| end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about
Authentication credentials. Resource must be signed_in for credentials to be returned.?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, makes more sense