Merge branch '333177-persist' into 'master'

Keep credit card non-sensitive info

See merge request gitlab-org/gitlab!71171

app/services/users/upsert_credit_card_validation_service.rb

@@ -7,6 +7,14 @@ module Users
     end
 
     def execute
+      @params = {
+        user_id: params.fetch(:user_id),
+        credit_card_validated_at: params.fetch(:credit_card_validated_at),
+        expiration_date: get_expiration_date(params),
+        last_digits: Integer(params.fetch(:credit_card_mask_number), 10),
+        holder_name: params.fetch(:credit_card_holder_name)
+      }
+
       ::Users::CreditCardValidation.upsert(@params)
 
       ServiceResponse.success(message: 'CreditCardValidation was set')
@@ -16,5 +24,14 @@ module Users
       Gitlab::ErrorTracking.track_exception(e, params: @params, class: self.class.to_s)
       ServiceResponse.error(message: "Could not set CreditCardValidation: #{e.message}")
     end
+
+    private
+
+    def get_expiration_date(params)
+      year = params.fetch(:credit_card_expiration_year)
+      month = params.fetch(:credit_card_expiration_month)
+
+      Date.new(year, month, -1) # last day of the month
+    end
   end
 end

lib/api/users.rb

@@ -1058,6 +1058,10 @@ module API
       params do
         requires :user_id, type: String, desc: 'The ID or username of the user'
         requires :credit_card_validated_at, type: DateTime, desc: 'The time when the user\'s credit card was validated'
+        requires :credit_card_expiration_month, type: Integer, desc: 'The month the credit card expires'
+        requires :credit_card_expiration_year, type: Integer, desc: 'The year the credit card expires'
+        requires :credit_card_holder_name, type: String, desc: 'The credit card holder name'
+        requires :credit_card_mask_number, type: String, desc: 'The last 4 digits of credit card number'
       end
       put ":user_id/credit_card_validation", feature_category: :users do
         authenticated_as_admin!

spec/requests/api/users_spec.rb

@@ -1457,10 +1457,20 @@ RSpec.describe API::Users do
 
   describe "PUT /user/:id/credit_card_validation" do
     let(:credit_card_validated_time) { Time.utc(2020, 1, 1) }
+    let(:expiration_year) { Date.today.year + 10 }
+    let(:params) do
+      {
+        credit_card_validated_at: credit_card_validated_time,
+        credit_card_expiration_year: expiration_year,
+        credit_card_expiration_month: 1,
+        credit_card_holder_name: 'John Smith',
+        credit_card_mask_number: '1111'
+      }
+    end
 
     context 'when unauthenticated' do
       it 'returns authentication error' do
-        put api("/user/#{user.id}/credit_card_validation"), params: { credit_card_validated_at: credit_card_validated_time }
+        put api("/user/#{user.id}/credit_card_validation"), params: {}
 
         expect(response).to have_gitlab_http_status(:unauthorized)
       end
@@ -1468,7 +1478,7 @@ RSpec.describe API::Users do
 
     context 'when authenticated as non-admin' do
       it "does not allow updating user's credit card validation", :aggregate_failures do
-        put api("/user/#{user.id}/credit_card_validation", user), params: { credit_card_validated_at: credit_card_validated_time }
+        put api("/user/#{user.id}/credit_card_validation", user), params: params
 
         expect(response).to have_gitlab_http_status(:forbidden)
       end
@@ -1476,10 +1486,17 @@ RSpec.describe API::Users do
 
     context 'when authenticated as admin' do
       it "updates user's credit card validation", :aggregate_failures do
-        put api("/user/#{user.id}/credit_card_validation", admin), params: { credit_card_validated_at: credit_card_validated_time }
+        put api("/user/#{user.id}/credit_card_validation", admin), params: params
+
+        user.reload
 
         expect(response).to have_gitlab_http_status(:ok)
-        expect(user.reload.credit_card_validated_at).to eq(credit_card_validated_time)
+        expect(user.credit_card_validation).to have_attributes(
+          credit_card_validated_at: credit_card_validated_time,
+          expiration_date: Date.new(expiration_year, 1, 31),
+          last_digits: 1111,
+          holder_name: 'John Smith'
+        )
       end
 
       it "returns 400 error if credit_card_validated_at is missing" do
@@ -1489,7 +1506,7 @@ RSpec.describe API::Users do
       end
 
       it 'returns 404 error if user not found' do
-        put api("/user/#{non_existing_record_id}/credit_card_validation", admin), params: { credit_card_validated_at: credit_card_validated_time }
+        put api("/user/#{non_existing_record_id}/credit_card_validation", admin), params: params
 
         expect(response).to have_gitlab_http_status(:not_found)
         expect(json_response['message']).to eq('404 User Not Found')

spec/services/users/upsert_credit_card_validation_service_spec.rb

@@ -7,7 +7,17 @@ RSpec.describe Users::UpsertCreditCardValidationService do
 
   let(:user_id) { user.id }
   let(:credit_card_validated_time) { Time.utc(2020, 1, 1) }
-  let(:params) { { user_id: user_id, credit_card_validated_at: credit_card_validated_time } }
+  let(:expiration_year) { Date.today.year + 10 }
+  let(:params) do
+    {
+      user_id: user_id,
+      credit_card_validated_at: credit_card_validated_time,
+      credit_card_expiration_year: expiration_year,
+      credit_card_expiration_month: 1,
+      credit_card_holder_name: 'John Smith',
+      credit_card_mask_number: '1111'
+    }
+  end
 
   describe '#execute' do
     subject(:service) { described_class.new(params) }
@@ -52,6 +62,16 @@ RSpec.describe Users::UpsertCreditCardValidationService do
       end
     end
 
+    shared_examples 'returns an error, tracking the exception' do
+      it do
+        expect(Gitlab::ErrorTracking).to receive(:track_exception)
+
+        result = service.execute
+
+        expect(result.status).to eq(:error)
+      end
+    end
+
     context 'when user id does not exist' do
       let(:user_id) { non_existing_record_id }
 
@@ -61,19 +81,27 @@ RSpec.describe Users::UpsertCreditCardValidationService do
     context 'when missing credit_card_validated_at' do
       let(:params) { { user_id: user_id } }
 
-      it_behaves_like 'returns an error without tracking the exception'
+      it_behaves_like 'returns an error, tracking the exception'
     end
 
     context 'when missing user id' do
       let(:params) { { credit_card_validated_at: credit_card_validated_time } }
 
-      it_behaves_like 'returns an error without tracking the exception'
+      it_behaves_like 'returns an error, tracking the exception'
     end
 
     context 'when unexpected exception happen' do
       it 'tracks the exception and returns an error' do
+        logged_params = {
+          credit_card_validated_at: credit_card_validated_time,
+          expiration_date: Date.new(expiration_year, 1, 31),
+          holder_name: "John Smith",
+          last_digits: 1111,
+          user_id: user_id
+        }
+
         expect(::Users::CreditCardValidation).to receive(:upsert).and_raise(e = StandardError.new('My exception!'))
-        expect(Gitlab::ErrorTracking).to receive(:track_exception).with(e, class: described_class.to_s, params: params)
+        expect(Gitlab::ErrorTracking).to receive(:track_exception).with(e, class: described_class.to_s, params: logged_params)
 
         result = service.execute