Skip to content

Hotfix/refactor password reset methods #69

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Hotfix/refactor password reset methods #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
58e7b9f
refactored reset_password to be an instance method
Jul 21, 2016
dc84c17
merged from master, fixed conflicts
Jul 21, 2016
355148c
refactored generate_token to be an instance method
Jul 21, 2016
7e4d2e1
cleanup
Jul 21, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 25 additions & 14 deletions app/models/customer_account.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,24 +64,16 @@ def self.find_by_reference(reference)
nil
end

def self.generate_token(attributes)
def generate_token(attributes)
post_attributes = { reset_link_with_placeholder: attributes[:reset_link_with_placeholder] }
requestor.custom("email:#{URI.encode_www_form_component(attributes[:email])}/resets", { request_method: :post }, { data: { type: :customer_accounts, attributes: post_attributes } }).first
self.last_result_set = self.class.requestor.custom("email:#{URI.encode_www_form_component(email)}/resets", { request_method: :post }, { data: { type: :customer_accounts, attributes: post_attributes } })
process_errors
end

def self.reset_password(attributes)
def reset_password(attributes)
patch_attributes = { password: attributes[:password] }
result = requestor.custom("email:#{URI.encode_www_form_component(attributes[:email])}/resets/token:#{attributes[:reset_password_token]}", { request_method: :patch }, { data: { type: :customer_accounts, attributes: patch_attributes } }).first
# Need to return object with an error in case of 422 status code
# @TODO refactor this method and 'generate_token' method to be an instance methods and update api accordingly
unless result
error_response = connection.last_response
result = find_by_email(attributes[:email])
if result && error_response.status == 422
error_response.body["errors"].each { |e| result.errors.add(:reset_password_token, e["detail"]) }
end
end
result
self.last_result_set = self.class.requestor.custom("email:#{URI.encode_www_form_component(email)}/resets/token:#{attributes[:reset_password_token]}", { request_method: :patch }, { data: { type: :customer_accounts, attributes: patch_attributes } })
process_errors
end

def orders
Expand All @@ -91,5 +83,24 @@ def orders
def create_note(attributes = {})
::FlexCommerce::Note.create(attributes.merge(attached_to_id: self.id, attached_to_type: self.class.name.demodulize))
end

private

# Logic taken from https://github.com/chingor13/json_api_client/blob/9d882bfb893c6deda87061dfbbd67300ee15e391/lib/json_api_client/resource.rb#L381
def process_errors
if last_result_set.has_errors?
last_result_set.errors.each do |error|
if error.source_parameter
errors.add(error.source_parameter, error.title)
else
errors.add(:base, error.title)
end
end
false
else
self.errors.clear if self.errors
true
end
end
end
end
60 changes: 39 additions & 21 deletions spec/integration/customer_account_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,14 @@

end
context "password resetting" do
let(:account) { build(:customer_account) }
let(:email_to_reset) { account.email }
let(:email_to_reset) { resource_identifier.attributes.email }
let(:encoded_email) { URI.encode_www_form_component(email_to_reset) }
context "generating token" do
let(:reset_link_with_placeholder) { "http://dummy.com/reset_password?email={{ email }}&token={{ token }}" }
subject { subject_class.generate_token(email: email_to_reset, reset_link_with_placeholder: reset_link_with_placeholder) }

let!(:find_by_email_stub) { stub_request(:get, "#{api_root}/customer_accounts/email:#{resource_identifier.attributes.email}.json_api").with(headers: { "Accept" => "application/vnd.api+json" }).to_return body: singular_resource.to_h.to_json, status: response_status, headers: default_headers }
let(:resource) { subject_class.find_by_email(resource_identifier.attributes.email) }
subject { resource.generate_token(reset_link_with_placeholder: reset_link_with_placeholder) }
let(:generate_token_body) do
{
data: {
Expand All @@ -145,21 +147,36 @@
}
end
let!(:generate_token_stub) { stub_request(:post, "#{api_root}/customer_accounts/email:#{encoded_email}/resets.json_api").with(headers: write_headers, body: generate_token_body).to_return body: singular_resource.to_h.to_json, status: response_status, headers: default_headers }
it "should return an instance which can then be reset using the token" do
expect(subject).to be_a(subject_class)
it "should return true if successfull" do
expect(subject).to be_truthy
end
context "with a more complex email" do
let(:account) { build(:customer_account, email: "[email protected]") }
it "should return an instance which can then be reset using the token" do
expect(subject).to be_a(subject_class)
let(:email_to_reset) { "[email protected]" }
it "should return true if successfull" do
resource.email = email_to_reset
expect(subject).to be_truthy
end
end
context "when unsuccessfull" do
let(:response_status) { 422 }
let(:error_message) { "Reset password token is invalid" }
let(:error_422) { build(:json_api_document, errors: [build(:json_api_error, status: "422", detail: error_message, title: error_message)]) }
let!(:generate_token_stub) { stub_request(:post, "#{api_root}/customer_accounts/email:#{encoded_email}/resets.json_api").with(headers: write_headers, body: generate_token_body).to_return body: error_422.to_h.to_json, status: response_status, headers: default_headers }

it "should return false and set resource errors" do
expect(subject).to be_falsey
expect(resource.errors).to be_present
expect(resource.errors.first).to include(error_message)
end
end
end

context "performing the reset" do
let(:reset_password_token) { "reset_password_token" }
let(:new_password) { "new_password" }
subject { subject_class.reset_password(email: email_to_reset, reset_password_token: reset_password_token, password: new_password) }
let!(:find_by_email_stub) { stub_request(:get, "#{api_root}/customer_accounts/email:#{resource_identifier.attributes.email}.json_api").with(headers: { "Accept" => "application/vnd.api+json" }).to_return body: singular_resource.to_h.to_json, status: response_status, headers: default_headers }
let(:resource) { subject_class.find_by_email(resource_identifier.attributes.email) }
subject { resource.reset_password(reset_password_token: reset_password_token, password: new_password) }
let(:reset_password_body) do
{
data: {
Expand All @@ -170,26 +187,27 @@
}
}
end
let!(:reset_password_stub) { stub_request(:patch, "#{api_root}/customer_accounts/email:#{encoded_email}/resets/token:#{reset_password_token}.json_api").with(headers: write_headers, body: reset_password_body).to_return body: singular_resource.to_h.to_json, status: response_status, headers: default_headers }
it "should return an instance" do
expect(subject).to be_a(subject_class)

context "successfull reset_password" do
let!(:reset_password_stub) { stub_request(:patch, "#{api_root}/customer_accounts/email:#{resource_identifier.attributes.email}/resets/token:#{reset_password_token}.json_api").with(headers: write_headers, body: reset_password_body).to_return body: singular_resource.to_h.to_json, status: response_status, headers: default_headers }
it "should return true" do
expect(subject).to be_truthy
expect(resource.errors).to be_empty
end
end

context "when invalid/expired token" do
context "failed reset_password due to invalid token" do
let(:response_status) { 422 }
let(:error_message) { "Reset password token is invalid" }
let(:error_422) { build(:json_api_document, errors: [build(:json_api_error, status: "422", detail: error_message, title: error_message)]) }
let(:email_to_reset) { resource_identifier.attributes.email }
let!(:reset_password_stub) { stub_request(:patch, "#{api_root}/customer_accounts/email:#{encoded_email}/resets/token:#{reset_password_token}.json_api").with(headers: write_headers, body: reset_password_body).to_return body: error_422.to_h.to_json, status: response_status, headers: default_headers }
let!(:find_by_email_stub) { stub_request(:get, "#{api_root}/customer_accounts/email:#{resource_identifier.attributes.email}.json_api").with(headers: { 'Accept'=>'application/vnd.api+json' }).to_return body: singular_resource.to_h.to_json, status: response_status, headers: default_headers }
it "should return an instance with error" do
expect(subject).to be_a(subject_class)
expect(subject.errors).to be_present
expect(subject.errors.first).to include(error_message)
let!(:reset_password_stub) { stub_request(:patch, "#{api_root}/customer_accounts/email:#{resource_identifier.attributes.email}/resets/token:#{reset_password_token}.json_api").with(headers: write_headers, body: reset_password_body).to_return body: error_422.to_h.to_json, status: response_status, headers: default_headers }
it "should return false and set resource errors" do
expect(subject).to be_falsey
expect(resource.errors).to be_present
expect(resource.errors.first).to include(error_message)
end
end
end

end
context "authenticating" do
let(:expected_body) {
Expand Down