From a67a74204cef4fff72fa3a050219e808276cf549 Mon Sep 17 00:00:00 2001 From: Caio Almeida <117518+caiosba@users.noreply.github.com> Date: Fri, 10 May 2024 16:57:02 -0300 Subject: [PATCH 01/22] Inverting the logic of the feed workspace filter for clusters (#1887) It should exclude unselected workspaces, instead of returning clusters with all other selected workspaces. Fixes CV2-4601. --- app/models/feed.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/feed.rb b/app/models/feed.rb index be7e2c1be0..3fd3b5b618 100755 --- a/app/models/feed.rb +++ b/app/models/feed.rb @@ -143,7 +143,7 @@ def filtered_clusters(args = {}) query = self.clusters # Filter by workspace - query = query.where("ARRAY[?] && team_ids", team_ids.to_a.map(&:to_i)) unless team_ids.blank? + query = query.where.not("ARRAY[?] && team_ids", self.team_ids - team_ids.to_a.map(&:to_i)) if !team_ids.blank? && team_ids != self.team_ids query = query.where(team_ids: []) if team_ids&.empty? # Invalidate the query # Filter by channel From a96b27a9d965b9fa929411550fcaaf4f612a4f45 Mon Sep 17 00:00:00 2001 From: Mohamed El-Sawy Date: Sun, 12 May 2024 01:42:09 +0300 Subject: [PATCH 02/22] CV2-4595 Validate password strength in check (#1888) * CV2-4595: validate password strength * CV2-4595: fix tests --- app/models/concerns/user_invitation.rb | 7 ++- app/models/concerns/user_private.rb | 5 ++ app/models/user.rb | 3 +- config/locales/en.yml | 1 + db/seeds.rb | 2 +- lib/sample_data.rb | 14 ++++- test/controllers/graphql_controller_9_test.rb | 9 +-- test/controllers/graphql_controller_test.rb | 12 ++-- .../omniauth_callbacks_controller_test.rb | 3 +- .../registrations_controller_test.rb | 43 ++++++++----- test/controllers/sessions_controller_test.rb | 40 ++++++++----- test/lib/check_rack_attack_test.rb | 2 +- test/mailers/registration_mailer_test.rb | 4 +- test/models/bot_user_test.rb | 2 +- test/models/user_test.rb | 60 +++++++++++++------ 15 files changed, 139 insertions(+), 68 deletions(-) diff --git a/app/models/concerns/user_invitation.rb b/app/models/concerns/user_invitation.rb index 22931b000d..55c7b152e8 100644 --- a/app/models/concerns/user_invitation.rb +++ b/app/models/concerns/user_invitation.rb @@ -132,7 +132,12 @@ def self.accept_team_user_invitation(tu, token, options) tu.save! # options should have password & username keys user = User.find_by_invitation_token(token, true) - password = options[:password] || Devise.friendly_token.first(8) + password = options[:password] + if password.blank? + # Generate random passsword that match password_complexity validation + samples = [('a'..'z'), ('A'..'Z'), (0..9), ['@', '#', '$', '%', '&']].map(&:to_a) + password = Devise.friendly_token.first(8).concat(samples.map(&:sample).shuffle.join) + end unless user.nil? invitable = User.accept_invitation!(:invitation_token => token, :password => password) user.update_columns(raw_invitation_token: nil, completed_signup: false) diff --git a/app/models/concerns/user_private.rb b/app/models/concerns/user_private.rb index f3dfa58738..8c0d6a0639 100644 --- a/app/models/concerns/user_private.rb +++ b/app/models/concerns/user_private.rb @@ -58,6 +58,11 @@ def validate_duplicate_email end end + def password_complexity + return if password.blank? || password =~ /^(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9])(?=.*?[#?!@$%^&*-]).{8,70}$/ + errors.add :password, I18n.t(:error_password_not_strong) + end + def handle_duplicate_email(u) if u.is_active? provider = u.get_user_provider(self.email) diff --git a/app/models/user.rb b/app/models/user.rb index 22cd943c4f..89e4714d2b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -43,8 +43,7 @@ class ToSOrPrivacyPolicyReadError < StandardError; end mount_uploader :image, ImageUploader validates :image, size: true - validate :user_is_member_in_current_team - validate :validate_duplicate_email + validate :user_is_member_in_current_team, :validate_duplicate_email, :password_complexity validate :languages_format, unless: proc { |u| u.settings.nil? } validates :api_key_id, absence: true, if: proc { |u| u.type.nil? } validates_presence_of :name diff --git a/config/locales/en.yml b/config/locales/en.yml index f4cb941008..a267fd5fb8 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -308,6 +308,7 @@ en: media_exists: This item already exists source_exists: This source already exists email_exists: has already been taken + error_password_not_strong: "Complexity requirement not met. Length should be 8-70 characters and include at least: 1 uppercase, 1 lowercase, 1 digit and 1 special character" banned_user: Sorry, your account has been banned from %{app_name}. Please contact the support team if you think this is an error. devise: diff --git a/db/seeds.rb b/db/seeds.rb index 7d7271810d..a374bbbd3d 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -56,7 +56,7 @@ class Setup def initialize(existing_user_email) @existing_user_email = existing_user_email @user_names = Array.new(3) { Faker::Name.first_name.downcase } - @user_passwords = Array.new(3) { Faker::Internet.password(min_length: 8) } + @user_passwords = Array.new(3) { random_complex_password } @user_emails = @user_names.map { |name| Faker::Internet.safe_email(name: name) } @team_names = Array.new(4) { Faker::Company.name } diff --git a/lib/sample_data.rb b/lib/sample_data.rb index b1dc6d3b37..b1708e3e8b 100644 --- a/lib/sample_data.rb +++ b/lib/sample_data.rb @@ -27,6 +27,16 @@ def random_machine_name name.downcase end + def random_complex_password(length = 10) + length -= 4 + low = random_string(1).downcase + up = random_string(1).upcase + num = ('0'..'9').to_a + u = ['@', '#', '$', '%', '&'].to_a + complex = (num.sample(1) + u.sample(1)).join + random_string(length).concat(low, up, complex) + end + def random_ip "%d.%d.%d.%d" % [rand(256), rand(256), rand(256), rand(256)] end @@ -70,7 +80,7 @@ def create_bot_user(options = {}) u.name = options[:name] || random_string u.login = options.has_key?(:login) ? options[:login] : random_string u.email = options[:email] || "#{random_string}@#{random_string}.com" - u.password = options[:password] || random_string + u.password = options[:password] || random_complex_password u.password_confirmation = options[:password_confirmation] || u.password u.is_admin = options[:is_admin] if options.has_key?(:is_admin) u.api_key_id = options.has_key?(:api_key_id) ? options[:api_key_id] : create_api_key.id @@ -104,7 +114,7 @@ def create_user(options = {}) u.login = options.has_key?(:login) ? options[:login] : random_string u.token = options.has_key?(:token) ? options[:token] : random_string(50) u.email = options[:email] || "#{random_string}@#{random_string}.com" - u.password = options.has_key?(:password) ? options[:password] : random_string + u.password = options.has_key?(:password) ? options[:password] : random_complex_password u.password_confirmation = options[:password_confirmation] || u.password u.current_team_id = options[:current_team_id] if options.has_key?(:current_team_id) u.is_admin = options[:is_admin] if options.has_key?(:is_admin) diff --git a/test/controllers/graphql_controller_9_test.rb b/test/controllers/graphql_controller_9_test.rb index 69d3e27ce9..648d3e8031 100644 --- a/test/controllers/graphql_controller_9_test.rb +++ b/test/controllers/graphql_controller_9_test.rb @@ -181,7 +181,8 @@ def setup end test "should handle user 2FA" do - u = create_user password: 'test1234' + p1 = random_complex_password + u = create_user password: p1 t = create_team create_team_user team: t, user: u authenticate_with_user(u) @@ -197,16 +198,16 @@ def setup post :create, params: { query: query, team: t.slug } assert_response :success # Enable/Disable 2FA - query = "mutation userTwoFactorAuthentication {userTwoFactorAuthentication(input: { clientMutationId: \"1\", id: #{u.id}, otp_required: #{true}, password: \"test1234\", qrcode: \"#{u.current_otp}\" }) { success }}" + query = "mutation userTwoFactorAuthentication {userTwoFactorAuthentication(input: { clientMutationId: \"1\", id: #{u.id}, otp_required: #{true}, password: \"#{p1}\", qrcode: \"#{u.current_otp}\" }) { success }}" post :create, params: { query: query, team: t.slug } assert_response :success assert u.reload.otp_required_for_login? - query = "mutation userTwoFactorAuthentication {userTwoFactorAuthentication(input: { clientMutationId: \"1\", id: #{u.id}, otp_required: #{false}, password: \"test1234\" }) { success }}" + query = "mutation userTwoFactorAuthentication {userTwoFactorAuthentication(input: { clientMutationId: \"1\", id: #{u.id}, otp_required: #{false}, password: \"#{p1}\" }) { success }}" post :create, params: { query: query, team: t.slug } assert_response :success assert_not u.reload.otp_required_for_login? # Disable with invalid uid - query = "mutation userTwoFactorAuthentication {userTwoFactorAuthentication(input: { clientMutationId: \"1\", id: #{invalid_uid}, otp_required: #{false}, password: \"test1234\" }) { success }}" + query = "mutation userTwoFactorAuthentication {userTwoFactorAuthentication(input: { clientMutationId: \"1\", id: #{invalid_uid}, otp_required: #{false}, password: \"#{p1}\" }) { success }}" post :create, params: { query: query, team: t.slug } assert_response :success end diff --git a/test/controllers/graphql_controller_test.rb b/test/controllers/graphql_controller_test.rb index e9639cb564..839ddaf9bd 100644 --- a/test/controllers/graphql_controller_test.rb +++ b/test/controllers/graphql_controller_test.rb @@ -647,9 +647,10 @@ def setup end test "should change password if token is found and passwords are present and match" do + p1 = random_complex_password u = create_user t = u.send_reset_password_instructions - query = "mutation changePassword { changePassword(input: { clientMutationId: \"1\", reset_password_token: \"#{t}\", password: \"123456789\", password_confirmation: \"123456789\" }) { success } }" + query = "mutation changePassword { changePassword(input: { clientMutationId: \"1\", reset_password_token: \"#{t}\", password: \"#{p1}\", password_confirmation: \"#{p1}\" }) { success } }" post :create, params: { query: query } sleep 1 assert_response :success @@ -657,18 +658,20 @@ def setup end test "should not change password if token is not found and passwords are present and match" do + p1 = random_complex_password u = create_user t = u.send_reset_password_instructions - query = "mutation changePassword { changePassword(input: { clientMutationId: \"1\", reset_password_token: \"#{t}x\", password: \"123456789\", password_confirmation: \"123456789\" }) { success } }" + query = "mutation changePassword { changePassword(input: { clientMutationId: \"1\", reset_password_token: \"#{t}x\", password: \"#{p1}\", password_confirmation: \"#{p1}\" }) { success } }" post :create, params: { query: query } sleep 1 assert_response 400 end test "should not change password if token is found but passwords are not present" do + p1 = random_complex_password u = create_user t = u.send_reset_password_instructions - query = "mutation changePassword { changePassword(input: { clientMutationId: \"1\", reset_password_token: \"#{t}\", password: \"123456789\" }) { success } }" + query = "mutation changePassword { changePassword(input: { clientMutationId: \"1\", reset_password_token: \"#{t}\", password: \"#{p1}\" }) { success } }" post :create, params: { query: query } sleep 1 assert_response :success @@ -676,9 +679,10 @@ def setup end test "should not change password if token is found but passwords do not match" do + p1 = random_complex_password u = create_user t = u.send_reset_password_instructions - query = "mutation changePassword { changePassword(input: { clientMutationId: \"1\", reset_password_token: \"#{t}\", password: \"123456789\", password_confirmation: \"12345678\" }) { success } }" + query = "mutation changePassword { changePassword(input: { clientMutationId: \"1\", reset_password_token: \"#{t}\", password: \"#{p1}\", password_confirmation: \"12345678\" }) { success } }" post :create, params: { query: query } sleep 1 assert_response 400 diff --git a/test/controllers/omniauth_callbacks_controller_test.rb b/test/controllers/omniauth_callbacks_controller_test.rb index 79c909c119..56165f4b89 100644 --- a/test/controllers/omniauth_callbacks_controller_test.rb +++ b/test/controllers/omniauth_callbacks_controller_test.rb @@ -190,7 +190,8 @@ def teardown end test "should connect when current user set" do - u = create_user login: 'test', password: '12345678', password_confirmation: '12345678', email: 'test@test.com' + p1 = random_complex_password + u = create_user login: 'test', password: p1, password_confirmation: p1, email: 'test@test.com' u.confirm authenticate_with_user(u) request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:twitter] diff --git a/test/controllers/registrations_controller_test.rb b/test/controllers/registrations_controller_test.rb index 14f99f4298..df4bd0500e 100644 --- a/test/controllers/registrations_controller_test.rb +++ b/test/controllers/registrations_controller_test.rb @@ -16,8 +16,9 @@ def teardown end test "should create user" do + p1 = random_complex_password assert_difference 'User.count' do - post :create, params: { api_user: { password: '12345678', password_confirmation: '12345678', email: 't@test.com', login: 'test', name: 'Test' } } + post :create, params: { api_user: { password: p1, password_confirmation: p1, email: 't@test.com', login: 'test', name: 'Test' } } assert_response 401 # needs to confirm before login end end @@ -32,67 +33,76 @@ def teardown User.send_user_invitation(members) end User.current = Team.current = nil + p1 = random_complex_password assert_no_difference 'User.count' do - post :create, params: { api_user: { password: '12345678', password_confirmation: '12345678', email: email, login: 'test', name: 'Test' } } + post :create, params: { api_user: { password: p1, password_confirmation: p1, email: email, login: 'test', name: 'Test' } } assert_response :success end end test "should create user if confirmed" do + p1 = random_complex_password User.any_instance.stubs(:confirmation_required?).returns(false) assert_difference 'User.count' do - post :create, params: { api_user: { password: '12345678', password_confirmation: '12345678', email: 't@test.com', login: 'test', name: 'Test' } } + post :create, params: { api_user: { password: p1, password_confirmation: p1, email: 't@test.com', login: 'test', name: 'Test' } } assert_response :success end User.any_instance.unstub(:confirmation_required?) end test "should not create user if password is missing" do + p1 = random_complex_password assert_no_difference 'User.count' do - post :create, params: { api_user: { password_confirmation: '12345678', email: 't@test.com', login: 'test', name: 'Test' } } + post :create, params: { api_user: { password_confirmation: p1, email: 't@test.com', login: 'test', name: 'Test' } } assert_response 400 end end test "should not create user if password is too short" do + p1 = '1234' assert_no_difference 'User.count' do - post :create, params: { api_user: { password: '123456', password_confirmation: '123456', email: 't@test.com', login: 'test', name: 'Test' } } + post :create, params: { api_user: { password: p1, password_confirmation: p1, email: 't@test.com', login: 'test', name: 'Test' } } assert_response 400 end end test "should not create user if password don't match" do + p1 = random_complex_password assert_no_difference 'User.count' do - post :create, params: { api_user: { password: '12345678', password_confirmation: '12345677', email: 't@test.com', login: 'test', name: 'Test' } } + post :create, params: { api_user: { password: random_complex_password, password_confirmation: random_complex_password, email: 't@test.com', login: 'test', name: 'Test' } } assert_response 400 end end test "should not create user if email is not present" do + p1 = random_complex_password assert_no_difference 'User.count' do - post :create, params: { api_user: { password: '12345678', password_confirmation: '12345678', email: '', login: 'test', name: 'Test' } } + post :create, params: { api_user: { password: p1, password_confirmation: p1, email: '', login: 'test', name: 'Test' } } assert_response 400 end end test "should create user if login is not present" do + p1 = random_complex_password assert_difference 'User.count' do - post :create, params: { api_user: { password: '12345678', password_confirmation: '12345678', email: 't@test.com', login: '', name: 'Test' } } + post :create, params: { api_user: { password: p1, password_confirmation: p1, email: 't@test.com', login: '', name: 'Test' } } assert_response 401 # needs to confirm before login end end test "should not create user if name is not present" do + p1 = random_complex_password assert_no_difference 'User.count' do - post :create, params: { api_user: { password: '12345678', password_confirmation: '12345678', email: 't@test.com', login: 'test', name: '' } } + post :create, params: { api_user: { password: p1, password_confirmation: p1, email: 't@test.com', login: 'test', name: '' } } assert_response 400 end end test "should update only a few attributes" do - u = create_user name: 'Foo', login: 'test', token: 'test', email: 'foo@test.com', password: '12345678' + p1 = random_complex_password + u = create_user name: 'Foo', login: 'test', token: 'test', email: 'foo@test.com', password: p1 authenticate_with_user(u) - post :update, params: { api_user: { name: 'Bar', login: 'bar', token: 'bar', email: 'bar@test.com', current_password: '12345678' } } + post :update, params: { api_user: { name: 'Bar', login: 'bar', token: 'bar', email: 'bar@test.com', current_password: p1 } } assert_response :success u = u.reload assert_equal 'Bar', u.name @@ -103,20 +113,23 @@ def teardown end test "should not update account if not logged in" do - post :update, params: { api_user: { name: 'Bar', login: 'bar', token: 'bar', email: 'bar@test.com', current_password: '12345678' } } + p1 = random_complex_password + post :update, params: { api_user: { name: 'Bar', login: 'bar', token: 'bar', email: 'bar@test.com', current_password: p1 } } assert_response 401 end test "should not update account" do - u = create_user name: 'Foo', login: 'test', token: 'test', email: 'foo@test.com', password: '12345678' + p1 = random_complex_password + u = create_user name: 'Foo', login: 'test', token: 'test', email: 'foo@test.com', password: p1 authenticate_with_user(u) - post :update, params: { api_user: { name: 'Bar', login: 'bar', token: 'bar', email: 'bar@test.com', current_password: '12345678', password: '123', password_confirmation: '123' } } + post :update, params: { api_user: { name: 'Bar', login: 'bar', token: 'bar', email: 'bar@test.com', current_password: p1, password: '123', password_confirmation: '123' } } assert_response 400 u = u.reload end test "should destroy account" do - u = create_user name: 'Foo', login: 'test', token: 'test', email: 'foo@test.com', password: '12345678' + p1 = random_complex_password + u = create_user name: 'Foo', login: 'test', token: 'test', email: 'foo@test.com', password: p1 authenticate_with_user(u) assert_difference 'User.count', -1 do delete :destroy, params: {} diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 9e58de0782..c6f878d4a5 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -10,9 +10,10 @@ def setup end test "should login using email" do - u = create_user login: 'test', password: '12345678', password_confirmation: '12345678', email: 'test@test.com' + p1 = random_complex_password + u = create_user login: 'test', password: p1, password_confirmation: p1, email: 'test@test.com' u.confirm - post :create, params: { api_user: { email: 'test@test.com', password: '12345678' } } + post :create, params: { api_user: { email: 'test@test.com', password: p1 } } assert_response :success assert_not_nil @controller.current_api_user # test login activities creation @@ -21,25 +22,28 @@ def setup end test "should require otp_attempt for login using email and 2FA" do - u = create_user login: 'test', password: '12345678', password_confirmation: '12345678', email: 'test@test.com' + p1 = random_complex_password + u = create_user login: 'test', password: p1, password_confirmation: p1, email: 'test@test.com' u.confirm u.two_factor - options = { otp_required: true, password: '12345678', qrcode: u.reload.current_otp } + options = { otp_required: true, password: p1, qrcode: u.reload.current_otp } u.two_factor=(options) - post :create, params: { api_user: { email: 'test@test.com', password: '12345678' } } + post :create, params: { api_user: { email: 'test@test.com', password: p1 } } assert_response 400 assert_nil @controller.current_api_user end test "should not login if password is wrong" do - u = create_user login: 'test', password: '12345678', password_confirmation: '12345678', email: 'test@test.com' - post :create, params: { api_user: { email: 'test@test.com', password: '12345679' } } + p1 = random_complex_password + u = create_user login: 'test', password: p1, password_confirmation: p1, email: 'test@test.com' + post :create, params: { api_user: { email: 'test@test.com', password: random_complex_password } } assert_response 401 assert_nil @controller.current_api_user end test "should logout" do - u = create_user login: 'test', password: '12345678', password_confirmation: '12345678', email: 'test@test.com' + p1 = random_complex_password + u = create_user login: 'test', password: p1, password_confirmation: p1, email: 'test@test.com' authenticate_with_user(u) delete :destroy, params: {} assert_response :success @@ -47,7 +51,8 @@ def setup end test "should logout and redirect to destination" do - u = create_user login: 'test', password: '12345678', password_confirmation: '12345678', email: 'test@test.com' + p1 = random_complex_password + u = create_user login: 'test', password: p1, password_confirmation: p1, email: 'test@test.com' authenticate_with_user(u) delete :destroy, params: { destination: '/' } assert_redirected_to '/' @@ -55,15 +60,17 @@ def setup end test "should login and redirect to destination" do - u = create_user login: 'test', password: '12345678', password_confirmation: '12345678', email: 'test@test.com' + p1 = random_complex_password + u = create_user login: 'test', password: p1, password_confirmation: p1, email: 'test@test.com' u.confirm - post :create, params: { api_user: { email: 'test@test.com', password: '12345678' }, destination: '/admin' } + post :create, params: { api_user: { email: 'test@test.com', password: p1 }, destination: '/admin' } assert_redirected_to '/admin' assert_not_nil @controller.current_api_user end test "should display error if cannot sign out" do - u = create_user login: 'test', password: '12345678', password_confirmation: '12345678', email: 'test@test.com' + p1 = random_complex_password + u = create_user login: 'test', password: p1, password_confirmation: p1, email: 'test@test.com' authenticate_with_user(u) @controller.expects(:sign_out).returns(false) delete :destroy, params: {} @@ -72,11 +79,12 @@ def setup end # test "should lock user after excessive login requests" do - # u = create_user login: 'test', password: '12345678', password_confirmation: '12345678', email: 'test@test.com' + # p1 = random_complex_password + # u = create_user login: 'test', password: p1, password_confirmation: p1, email: 'test@test.com' # Devise.maximum_attempts = 2 # 2.times do - # post :create, params: { api_user: { email: 'test@test.com', password: '12345679' } } + # post :create, params: { api_user: { email: 'test@test.com', password: random_complex_password } } # end # u.reload @@ -86,14 +94,14 @@ def setup # test "should unlock locked user accounts after specified time" do # travel_to Time.zone.local(2023, 12, 12, 01, 04, 44) - # u = create_user login: 'test', password: '12345678', password_confirmation: '12345678', email: 'test@test.com' + # u = create_user login: 'test', password: p1, password_confirmation: p1, email: 'test@test.com' # Devise.unlock_in = 10.minutes # u.lock_access! # travel 30.minutes - # post :create, params: { api_user: { email: 'test@test.com', password: '12345678' } } + # post :create, params: { api_user: { email: 'test@test.com', password: p1 } } # u.reload # assert !u.access_locked? diff --git a/test/lib/check_rack_attack_test.rb b/test/lib/check_rack_attack_test.rb index ce32666574..e643e7c3a9 100644 --- a/test/lib/check_rack_attack_test.rb +++ b/test/lib/check_rack_attack_test.rb @@ -19,7 +19,7 @@ class ThrottlingTest < ActionDispatch::IntegrationTest test "should throttle excessive requests to /api/users/sign_in" do stub_configs({ 'login_rate_limit' => 2 }) do - user_params = { api_user: { email: 'user@example.com', password: 'password' } } + user_params = { api_user: { email: 'user@example.com', password: random_complex_password } } 2.times do post api_user_session_path, params: user_params, as: :json diff --git a/test/mailers/registration_mailer_test.rb b/test/mailers/registration_mailer_test.rb index 13d03d4169..0e7a2bb7df 100644 --- a/test/mailers/registration_mailer_test.rb +++ b/test/mailers/registration_mailer_test.rb @@ -3,7 +3,7 @@ class RegistrationMailerTest < ActionMailer::TestCase test "should send welcome email" do - u = create_user email: 'test@localhost', password: '12345678' + u = create_user email: 'test@localhost', password: 'testA@12' email = RegistrationMailer.welcome_email(u) assert_emails 1 do @@ -12,7 +12,7 @@ class RegistrationMailerTest < ActionMailer::TestCase assert_match email.from.first, CheckConfig.get('default_mail') assert_equal ['test@localhost'], email.to - assert_match /12345678/, email.body.parts.first.to_s + assert_match /testA@12/, email.body.parts.first.to_s end test "should send email for mail duplicate" do diff --git a/test/models/bot_user_test.rb b/test/models/bot_user_test.rb index 15c1c4baf5..3802c3d1ce 100644 --- a/test/models/bot_user_test.rb +++ b/test/models/bot_user_test.rb @@ -16,7 +16,7 @@ def setup end test "should not have password null" do - b = create_bot_user password: '12345678' + b = create_bot_user password: random_complex_password assert_nil b.password end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index a7498e2245..ef31bcf3b7 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -209,6 +209,23 @@ def setup end end + test "should validate password complexity" do + assert_difference 'User.count' do + create_user password: random_complex_password + end + assert_no_difference 'User.count' do + assert_raises ActiveRecord::RecordInvalid do + create_user password: random_string + end + assert_raises ActiveRecord::RecordInvalid do + create_user password: random_complex_password(7) + end + assert_raises ActiveRecord::RecordInvalid do + create_user password: random_complex_password(80) + end + end + end + test "should not register with banned email" do u = create_user is_active: false assert_no_difference 'User.count' do @@ -498,19 +515,22 @@ def setup end test "should set user password" do - u = create_user password: '12345678', password_confirmation: '12345678' - u.set_password = '87654321' + p1 = random_complex_password + u = create_user password: p1, password_confirmation: p1 + p2 = random_complex_password + u.set_password = p2 u.save! - assert_equal '87654321', u.password - assert_equal '87654321', u.password_confirmation + assert_equal p2, u.password + assert_equal p2, u.password_confirmation end test "should not change user password if value is blank" do - u = create_user password: '12345678', password_confirmation: '12345678' + p1 = random_complex_password + u = create_user password: p1, password_confirmation: p1 u.set_password = '' u.save! - assert_equal '12345678', u.password - assert_equal '12345678', u.password_confirmation + assert_equal p1, u.password + assert_equal p1, u.password_confirmation end test "should protect attributes from mass assignment" do @@ -529,8 +549,9 @@ def setup assert_equal 0, u.send(:pending_notifications).size Sidekiq::Extensions::DelayedMailer.jobs.clear assert_equal 0, Sidekiq::Extensions::DelayedMailer.jobs.size - u.password = '12345678' - u.password_confirmation = '12345678' + p1 = random_complex_password + u.password = p1 + u.password_confirmation = p1 u.send(:send_devise_notification, 'confirmation_instructions', 'token', {}) u.save! assert_equal 1, Sidekiq::Extensions::DelayedMailer.jobs.size @@ -1310,7 +1331,8 @@ def setup end test "should have 2FA for email based user" do - u = create_user password: 'test1234' + p1 = random_complex_password + u = create_user password: p1 assert_nil u.otp_secret data = u.two_factor assert_not_nil u.otp_secret @@ -1321,8 +1343,8 @@ def setup assert_raise RuntimeError do u.two_factor=(options) end - options[:password] = 'test1234' - options[:qrcode] = 'test1234' + options[:password] = p1 + options[:qrcode] = p1 assert_raise RuntimeError do u.two_factor=(options) end @@ -1352,23 +1374,25 @@ def setup end test "should reset or change user password" do - u = create_user password: 'test1234' + p1 = random_complex_password + p2 = random_complex_password + u = create_user password: p1 rand_id = u.id + rand(100) - options = { id: rand_id, current_password: 'invalidpassword', password: 'test5678', password_confirmation: 'test5678' } + options = { id: rand_id, current_password: 'invalidpassword', password: p2, password_confirmation: p2 } User.stubs(:current).returns(u) # test change password assert_raises ActiveRecord::RecordNotFound do User.reset_change_password(options) end options[:id] = u.id - assert u.reload.valid_password?('test1234') + assert u.reload.valid_password?(p1) assert_raises RuntimeError do User.reset_change_password(options) end - assert u.reload.valid_password?('test1234') - options[:current_password] = 'test1234' + assert u.reload.valid_password?(p1) + options[:current_password] = p1 User.reset_change_password(options) - assert u.reload.valid_password?('test5678') + assert u.reload.valid_password?(p2) User.unstub(:current) end From 4c892eefd50f15e37cfb2c752d1ccebff8f7df3e Mon Sep 17 00:00:00 2001 From: Mohamed El-Sawy Date: Mon, 13 May 2024 15:08:26 +0300 Subject: [PATCH 03/22] CV2-4604: fix sentry issue (#1889) --- app/models/assignment.rb | 10 ++++++---- lib/tasks/data/project_media.rake | 2 ++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/models/assignment.rb b/app/models/assignment.rb index d0b30fed64..94b4b3b30c 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -133,10 +133,12 @@ def apply_rules_and_actions def update_elasticsearch_assignment if ['Annotation', 'Dynamic'].include?(self.assigned_type) && self.assigned.annotation_type == 'verification_status' pm = self.assigned.annotated - # Update ES - uids = Assignment.where(assigned_type: self.assigned_type, assigned_id: self.assigned_id).map(&:user_id) - data = { 'assigned_user_ids' => uids } - pm.update_elasticsearch_doc(data.keys, data, pm.id) + unless pm.nil? + # Update ES + uids = Assignment.where(assigned_type: self.assigned_type, assigned_id: self.assigned_id).map(&:user_id) + data = { 'assigned_user_ids' => uids } + pm.update_elasticsearch_doc(data.keys, data, pm.id) + end end end end diff --git a/lib/tasks/data/project_media.rake b/lib/tasks/data/project_media.rake index f143af915c..19cdb7ef54 100644 --- a/lib/tasks/data/project_media.rake +++ b/lib/tasks/data/project_media.rake @@ -148,8 +148,10 @@ namespace :check do print '.' value = pm.send(field_name, true) end + sleep 2 end Rails.cache.write('check:project_media:recalculate_cached_field:team_id', team.id) if data_args['slug'].blank? + sleep 5 end minutes = ((Time.now.to_i - started) / 60).to_i puts "[#{Time.now}] Done in #{minutes} minutes." From dbf977539fbb1ad5ce6a0ad35bb70b1cc4d3a48f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 13 May 2024 22:24:59 -0300 Subject: [PATCH 04/22] Bump nokogiri from 1.16.2 to 1.16.5 (#1891) Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.16.2 to 1.16.5. - [Release notes](https://github.com/sparklemotion/nokogiri/releases) - [Changelog](https://github.com/sparklemotion/nokogiri/blob/main/CHANGELOG.md) - [Commits](https://github.com/sparklemotion/nokogiri/compare/v1.16.2...v1.16.5) --- updated-dependencies: - dependency-name: nokogiri dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Gemfile | 2 +- Gemfile.lock | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index 7a3f9faf0e..42fdb78704 100644 --- a/Gemfile +++ b/Gemfile @@ -60,7 +60,7 @@ gem 'elasticsearch-persistence', '7.1.1' gem 'paper_trail', '13.0.0' gem 'graphiql-rails', git: 'https://github.com/meedan/graphiql-rails.git', ref: '8db0eac' gem 'graphql-formatter' -gem 'nokogiri', '1.16.2' +gem 'nokogiri', '1.16.5' gem 'puma' gem 'rack-attack' gem 'rack-cors', '1.0.6', require: 'rack/cors' diff --git a/Gemfile.lock b/Gemfile.lock index 7128897536..02b368e757 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -420,7 +420,7 @@ GEM mime-types-data (3.2022.0105) mini_magick (4.10.1) mini_mime (1.1.5) - mini_portile2 (2.8.5) + mini_portile2 (2.8.6) minitest (5.19.0) minitest-hooks (1.5.0) minitest (> 5.3) @@ -447,7 +447,7 @@ GEM net-protocol netrc (0.11.0) nio4r (2.7.0) - nokogiri (1.16.2) + nokogiri (1.16.5) mini_portile2 (~> 2.8.2) racc (~> 1.4) numerizer (0.1.1) @@ -941,7 +941,7 @@ DEPENDENCIES minitest-retry mocha (~> 2.0) multi_json (= 1.15.0) - nokogiri (= 1.16.2) + nokogiri (= 1.16.5) omniauth-facebook omniauth-google-oauth2! omniauth-slack From 20c4d14888002087b0074d3339c5280a91f5f512 Mon Sep 17 00:00:00 2001 From: Devin Gaffney Date: Thu, 16 May 2024 11:42:33 -0700 Subject: [PATCH 05/22] CV2-4597 rework how cached responses are returned to re-differentiate between empty responses and non-responses (#1883) * CV2-4126 rework how cached responses are returned to re-differentiate between empty responses and non-responses * fix typo * fix fixture * fix broken test with proper query events * fix test fixtures * resolve uncovered lines --- app/models/bot/alegre.rb | 1 + app/models/concerns/alegre_v2.rb | 11 +- test/controllers/reports_controller_test.rb | 52 +++++++- test/models/bot/alegre_v2_test.rb | 132 +++++++++++++++++++- 4 files changed, 188 insertions(+), 8 deletions(-) diff --git a/app/models/bot/alegre.rb b/app/models/bot/alegre.rb index 7a22aa9b8d..2276b900fe 100644 --- a/app/models/bot/alegre.rb +++ b/app/models/bot/alegre.rb @@ -401,6 +401,7 @@ def self.transcribe_audio(pm) def self.media_file_url(pm) # FIXME Ugly hack to get a usable URL in docker-compose development environment. if pm.is_a?(TemporaryProjectMedia) + print("HELLO") url = pm.url else url = (ENV['RAILS_ENV'] != 'development' ? pm.media.file.file.public_url : "#{CheckConfig.get('storage_endpoint')}/#{CheckConfig.get('storage_bucket')}/#{pm.media.file.file.path}") diff --git a/app/models/concerns/alegre_v2.rb b/app/models/concerns/alegre_v2.rb index 69514e1028..aeb6904607 100644 --- a/app/models/concerns/alegre_v2.rb +++ b/app/models/concerns/alegre_v2.rb @@ -383,14 +383,18 @@ def get_required_keys(project_media, field) } end + def get_parsed_cached_data_for_key(key) + value = Redis.new(REDIS_CONFIG).get(key) + Hash[YAML.load(value).collect{|kk,vv| [kk.to_i, vv]}] if value + end + def get_cached_data(required_keys) - redis = Redis.new(REDIS_CONFIG) # For a given project media, we expect a set of keys to be set by the webhook callbacks sent from alegre back to check-api. # For each callback response (which is set via #process_alegre_callback), we store the value as serialized YAML to persist # the data such that symbolized keys return as symbols (as opposed to JSON, which loses the distinction). Here, in effect, # we check to see if all the responses we expect from Alegre have been sent - downstream of this, we check to see if all # responses are non-empty before proceeding to creating relationships. - Hash[required_keys.collect{|k,v| [k, (Hash[YAML.load(redis.get(v)).collect{|kk,vv| [kk.to_i, vv]}] rescue [])]}] + Hash[required_keys.collect{|k,v| [k, get_parsed_cached_data_for_key(v)]}] end def get_similar_items_v2_callback(project_media, field) @@ -420,7 +424,7 @@ def relate_project_media_callback(project_media, field=nil) end def is_cached_data_not_good(cached_data) - cached_data.values.collect{|x| x.to_a.empty?}.include?(true) + cached_data.values.collect{|x| x.nil?}.include?(true) end def get_items_with_similar_media_v2(args={}) @@ -431,6 +435,7 @@ def get_items_with_similar_media_v2(args={}) type = args[:type] if ['audio', 'image', 'video'].include?(type) if project_media.nil? + print("hello") project_media = TemporaryProjectMedia.new project_media.url = media_url project_media.id = Digest::MD5.hexdigest(project_media.url).to_i(16) diff --git a/test/controllers/reports_controller_test.rb b/test/controllers/reports_controller_test.rb index 32e197a704..0af43ffdd5 100644 --- a/test/controllers/reports_controller_test.rb +++ b/test/controllers/reports_controller_test.rb @@ -40,8 +40,56 @@ def from_alegre(pm) pm5 = create_project_media team: @t, media: create_uploaded_video create_project_media team: @t - Bot::Alegre.stubs(:request).returns({ 'result' => [from_alegre(@pm), from_alegre(pm), from_alegre(pm2), from_alegre(pm3), from_alegre(pm4), from_alegre(pm5)] }) - + Bot::Alegre.stubs(:get_items_with_similar_media_v2).returns({ + @pm.id => { + score: 1.0, + context: {"team_id" => @pm.team_id, "project_media_id" => @pm.id, "temporary_media" => false}, + model: Bot::Alegre.get_type(@pm), + source_field: Bot::Alegre.get_type(@pm), + target_field: Bot::Alegre.get_type(@pm), + relationship_type: {:source=>"confirmed_sibling", :target=>"confirmed_sibling"} + }, + pm.id => { + score: 1.0, + context: {"team_id" => pm.team_id, "project_media_id" => pm.id, "temporary_media" => false}, + model: Bot::Alegre.get_type(pm), + source_field: Bot::Alegre.get_type(pm), + target_field: Bot::Alegre.get_type(pm), + relationship_type: {:source=>"confirmed_sibling", :target=>"confirmed_sibling"} + }, + pm2.id => { + score: 1.0, + context: {"team_id" => pm2.team_id, "project_media_id" => pm2.id, "temporary_media" => false}, + model: Bot::Alegre.get_type(pm2), + source_field: Bot::Alegre.get_type(pm2), + target_field: Bot::Alegre.get_type(pm2), + relationship_type: {:source=>"confirmed_sibling", :target=>"confirmed_sibling"} + }, + pm3.id => { + score: 1.0, + context: {"team_id" => pm3.team_id, "project_media_id" => pm3.id, "temporary_media" => false}, + model: Bot::Alegre.get_type(pm3), + source_field: Bot::Alegre.get_type(pm3), + target_field: Bot::Alegre.get_type(pm3), + relationship_type: {:source=>"confirmed_sibling", :target=>"confirmed_sibling"} + }, + pm4.id => { + score: 1.0, + context: {"team_id" => pm4.team_id, "project_media_id" => pm4.id, "temporary_media" => false}, + model: Bot::Alegre.get_type(pm4), + source_field: Bot::Alegre.get_type(pm4), + target_field: Bot::Alegre.get_type(pm4), + relationship_type: {:source=>"confirmed_sibling", :target=>"confirmed_sibling"} + }, + pm5.id => { + score: 1.0, + context: {"team_id" => pm5.team_id, "project_media_id" => pm5.id, "temporary_media" => false}, + model: Bot::Alegre.get_type(pm5), + source_field: Bot::Alegre.get_type(pm5), + target_field: Bot::Alegre.get_type(pm5), + relationship_type: {:source=>"confirmed_sibling", :target=>"confirmed_sibling"} + }, + }) post :index, params: {} assert_response :success assert_equal 7, json_response['data'].size diff --git a/test/models/bot/alegre_v2_test.rb b/test/models/bot/alegre_v2_test.rb index 07c119917f..174bf7aac2 100644 --- a/test/models/bot/alegre_v2_test.rb +++ b/test/models/bot/alegre_v2_test.rb @@ -48,6 +48,15 @@ def teardown assert_equal Bot::Alegre.media_file_url(pm1).class, String end + test "should generate media file url for temporary object" do + project_media = TemporaryProjectMedia.new + project_media.url = "http://example.com" + project_media.id = Digest::MD5.hexdigest(project_media.url).to_i(16) + project_media.team_id = [1,2,3] + project_media.type = "audio" + assert_equal Bot::Alegre.media_file_url(project_media).class, String + end + test "should generate item_doc_id" do pm1 = create_project_media team: @team, media: create_uploaded_audio assert_equal Bot::Alegre.item_doc_id(pm1).class, String @@ -816,8 +825,57 @@ def teardown } } } + unconfirmed_params = { + "model_type": "image", + "data": { + "item": { + "id": "Y2hlY2stcHJvamVjdF9tZWRpYS0yMTQt", + "callback_url": "http://alegre:3100/presto/receive/add_item/image", + "url": "http://minio:9000/check-api-dev/uploads/uploaded_image/55/09572dedf610aad68090214303c14829.png", + "text": nil, + "raw": { + "doc_id": "Y2hlY2stcHJvamVjdF9tZWRpYS0yMTQt", + "context": { + "team_id": pm1.team_id, + "project_media_id": pm1.id, + "has_custom_id": true, + "temporary_media": false, + }, + "url": "http://minio:9000/check-api-dev/uploads/uploaded_image/55/09572dedf610aad68090214303c14829.png", + "threshold": 0.63, + "confirmed": false, + "created_at": "2024-03-14T22:05:47.588975", + "limit": 200, + "requires_callback": true, + "final_task": "search" + }, + "hash_value": "1110101010001011110100000011110010101000000010110101101010100101101111110101101001011010100001011111110101011010010000101010010110101101010110100000001010100101101010111110101000010101011100001110101010101111100001010101001011101010101011010001010101010010" + }, + "results": { + "result": [ + { + "id": "Y2hlY2stcHJvamVjdF9tZWRpYS0yMTQt", + "doc_id": "Y2hlY2stcHJvamVjdF9tZWRpYS0yMTQt", + "pdq": "1110101010001011110100000011110010101000000010110101101010100101101111110101101001011010100001011111110101011010010000101010010110101101010110100000001010100101101010111110101000010101011100001110101010101111100001010101001011101010101011010001010101010010", + "url": "http://minio:9000/check-api-dev/uploads/uploaded_image/55/09572dedf610aad68090214303c14829.png", + "context": [ + { + "team_id": pm2.team_id, + "has_custom_id": true, + "project_media_id": pm2.id, + "temporary_media": false, + } + ], + "score": 1.0, + "model": "image/pdq" + } + ] + } + } + } assert_difference 'Relationship.count' do # Simulate the webhook hitting the server and being executed.... + Bot::Alegre.process_alegre_callback(JSON.parse(unconfirmed_params.to_json)) relationship = Bot::Alegre.process_alegre_callback(JSON.parse(params.to_json)) #hack to force into stringed keys end assert_equal relationship.source, pm2 @@ -833,7 +891,7 @@ def teardown params = { "model_type": "image", "data": { - "is_shortcircuited_callback": true, + "is_shortcircuited_search_result_callback": true, "item": { "callback_url": "http://alegre:3100/presto/receive/add_item/image", "url": "http://minio:9000/check-api-dev/uploads/uploaded_image/55/09572dedf610aad68090214303c14829.png", @@ -847,7 +905,7 @@ def teardown "temporary_media": false, }, "url": "http://minio:9000/check-api-dev/uploads/uploaded_image/55/09572dedf610aad68090214303c14829.png", - "threshold": 0.73, + "threshold": 0.85, "confirmed": true, "created_at": "2024-03-14T22:05:47.588975", "limit": 200, @@ -878,8 +936,57 @@ def teardown } } } + unconfirmed_params = { + "model_type": "image", + "data": { + "is_shortcircuited_search_result_callback": true, + "item": { + "callback_url": "http://alegre:3100/presto/receive/add_item/image", + "url": "http://minio:9000/check-api-dev/uploads/uploaded_image/55/09572dedf610aad68090214303c14829.png", + "text": nil, + "raw": { + "doc_id": "Y2hlY2stcHJvamVjdF9tZWRpYS0yMTQt", + "context": { + "team_id": pm1.team_id, + "project_media_id": pm1.id, + "has_custom_id": true, + "temporary_media": false, + }, + "url": "http://minio:9000/check-api-dev/uploads/uploaded_image/55/09572dedf610aad68090214303c14829.png", + "threshold": 0.73, + "confirmed": false, + "created_at": "2024-03-14T22:05:47.588975", + "limit": 200, + "requires_callback": true, + "final_task": "search" + }, + "hash_value": "1110101010001011110100000011110010101000000010110101101010100101101111110101101001011010100001011111110101011010010000101010010110101101010110100000001010100101101010111110101000010101011100001110101010101111100001010101001011101010101011010001010101010010" + }, + "results": { + "result": [ + { + "id": "Y2hlY2stcHJvamVjdF9tZWRpYS0yMTQt", + "doc_id": "Y2hlY2stcHJvamVjdF9tZWRpYS0yMTQt", + "pdq": "1110101010001011110100000011110010101000000010110101101010100101101111110101101001011010100001011111110101011010010000101010010110101101010110100000001010100101101010111110101000010101011100001110101010101111100001010101001011101010101011010001010101010010", + "url": "http://minio:9000/check-api-dev/uploads/uploaded_image/55/09572dedf610aad68090214303c14829.png", + "context": [ + { + "team_id": pm2.team_id, + "has_custom_id": true, + "project_media_id": pm2.id, + "temporary_media": false, + } + ], + "score": 1.0, + "model": "image/pdq" + } + ] + } + } + } assert_difference 'Relationship.count' do # Simulate the webhook hitting the server and being executed.... + Bot::Alegre.process_alegre_callback(JSON.parse(unconfirmed_params.to_json)) #hack to force into stringed keys relationship = Bot::Alegre.process_alegre_callback(JSON.parse(params.to_json)) #hack to force into stringed keys end assert_equal relationship.source, pm2 @@ -946,9 +1053,28 @@ def teardown end end + test "should handle a call to get_items_with_similar_media_v2 with a temporary request" do + keys = { + confirmed_results: "alegre:async_results:blah_nil_true", + suggested_or_confirmed_results: "alegre:async_results:blah_nil_false" + } + sequence = sequence('get_cached_data_sequence') + Bot::Alegre.stubs(:get_similar_items_v2_async).returns(true) + Bot::Alegre.stubs(:get_cached_data).in_sequence(sequence).returns({blah: nil}).then.returns({}) + Bot::Alegre.stubs(:get_required_keys).returns(keys) + Bot::Alegre.stubs(:get_similar_items_v2_callback).returns({}) + Bot::Alegre.stubs(:delete).returns(true) + assert_equal Bot::Alegre.get_items_with_similar_media_v2(type: "audio", media_url: "http://example.com", timeout: 1), {} + Bot::Alegre.unstub(:get_similar_items_v2_async) + Bot::Alegre.unstub(:get_cached_data) + Bot::Alegre.unstub(:get_required_keys) + Bot::Alegre.unstub(:get_similar_items_v2_callback) + Bot::Alegre.unstub(:delete) + end + test "should get_cached_data with right fallbacks" do pm1 = create_project_media team: @team, media: create_uploaded_audio - assert_equal Bot::Alegre.get_cached_data(Bot::Alegre.get_required_keys(pm1, nil)), {confirmed_results: [], suggested_or_confirmed_results: []} + assert_equal Bot::Alegre.get_cached_data(Bot::Alegre.get_required_keys(pm1, nil)), {confirmed_results: nil, suggested_or_confirmed_results: nil} end test "should relate project media for audio" do From fde805fa2b4eda8efda13e406add5d0d481c054b Mon Sep 17 00:00:00 2001 From: Caio <117518+caiosba@users.noreply.github.com> Date: Thu, 16 May 2024 16:54:02 -0300 Subject: [PATCH 06/22] Adding Jay to CODEOWNERS [skip ci] --- CODEOWNERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CODEOWNERS b/CODEOWNERS index b256017404..31ad382dcc 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1 +1 @@ -* @caiosba @melsawy @DGaffney +* @caiosba @melsawy @DGaffney @jayjay-w From 1a17c24fa7f412ae5d1db2ce89a8e3201932fe3a Mon Sep 17 00:00:00 2001 From: Caio Almeida <117518+caiosba@users.noreply.github.com> Date: Fri, 17 May 2024 08:19:27 -0300 Subject: [PATCH 07/22] Send report for confirmed suggestions. (#1893) Make sure that reports are sent for confirmed suggestions. Fixes CV2-4564. --- app/models/bot/smooch.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/bot/smooch.rb b/app/models/bot/smooch.rb index 65a62e1998..59416cd061 100644 --- a/app/models/bot/smooch.rb +++ b/app/models/bot/smooch.rb @@ -62,8 +62,8 @@ def suggestion_accepted? def self.inherit_status_and_send_report(rid) relationship = Relationship.find_by_id(rid) unless relationship.nil? - # A relationship created by the Smooch Bot or Alegre Bot is related to search results, so the user has already received the report as a search result - return if [BotUser.smooch_user&.id, BotUser.alegre_user&.id].include?(relationship.user_id) + # A relationship created by the Smooch Bot or Alegre Bot is related to search results, so the user has already received the report as a search result - unless it's a suggestion + return if [BotUser.smooch_user&.id, BotUser.alegre_user&.id].include?(relationship.user_id) && !relationship.confirmed_by target = relationship.target parent = relationship.source if ::Bot::Smooch.team_has_smooch_bot_installed(target) && relationship.is_confirmed? From 7c6fad3fcb5b7d5b62c7ad1dd82811a264c4b811 Mon Sep 17 00:00:00 2001 From: Caio Almeida <117518+caiosba@users.noreply.github.com> Date: Sun, 19 May 2024 22:06:29 -0300 Subject: [PATCH 08/22] Don't pause reports automatically when a fact-check is updated. (#1894) Fixes CV2-4620. --- app/models/fact_check.rb | 2 +- test/models/fact_check_test.rb | 35 ++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/app/models/fact_check.rb b/app/models/fact_check.rb index fccd7d90f6..116999d6a6 100644 --- a/app/models/fact_check.rb +++ b/app/models/fact_check.rb @@ -70,7 +70,7 @@ def update_report }) report.merge!({ use_introduction: default_use_introduction, introduction: default_introduction }) if language != report_language data[:options] = report - data[:state] = (self.publish_report ? 'published' : 'paused') + data[:state] = (self.publish_report ? 'published' : 'paused') if data[:state].blank? || !self.publish_report.nil? reports.annotator = self.user || User.current reports.set_fields = data.to_json reports.skip_check_ability = true diff --git a/test/models/fact_check_test.rb b/test/models/fact_check_test.rb index 4102c5d69e..899394d8cc 100644 --- a/test/models/fact_check_test.rb +++ b/test/models/fact_check_test.rb @@ -307,4 +307,39 @@ def setup create_fact_check signature: 'test' end end + + test "should set report status correctly when fact-check is updated" do + RequestStore.store[:skip_cached_field_update] = false + create_report_design_annotation_type + Sidekiq::Testing.inline! do + pm = create_project_media + cd = create_claim_description project_media: pm + assert_equal 'unpublished', pm.reload.report_status + + fc = create_fact_check claim_description: cd, publish_report: true + assert_equal 'published', pm.reload.report_status + + fc = FactCheck.find(fc.id) + fc.title = random_string + fc.save! + assert_equal 'published', pm.reload.report_status + + fc = FactCheck.find(fc.id) + fc.title = random_string + fc.publish_report = false + fc.save! + assert_equal 'paused', pm.reload.report_status + + fc = FactCheck.find(fc.id) + fc.title = random_string + fc.save! + assert_equal 'paused', pm.reload.report_status + + fc = FactCheck.find(fc.id) + fc.title = random_string + fc.publish_report = true + fc.save! + assert_equal 'published', pm.reload.report_status + end + end end From 060f47b258e8b738cf2e0c11eec89dd02c7847a2 Mon Sep 17 00:00:00 2001 From: Jay Joshua <7008757+jayjay-w@users.noreply.github.com> Date: Tue, 21 May 2024 14:07:38 +0200 Subject: [PATCH 09/22] Excessive login attempts: Block IP not user (#1890) * Disable devise user locking Currently, we block users with repeated incorrect sign-in attempts. However, this introduces a scenario where an attacker might target a specific user and lead to their account being locked. To prevent this, we are disabling account lock-out in devise. * Change rack-attack configuration to block offending login requests instead of throttling them * Fix rack-attack tests * Fix failing specs * Update blocklist comment to be more clear Update code comment to make it clear we are adding IP to blocklist and not actually blocking it. --- app/models/user.rb | 2 +- config/initializers/devise.rb | 7 ---- config/initializers/rack_attack.rb | 35 +++++++++++++------- test/controllers/sessions_controller_test.rb | 30 ----------------- test/lib/check_rack_attack_test.rb | 12 +++---- 5 files changed, 30 insertions(+), 56 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 89e4714d2b..d0dcfde3f0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -32,7 +32,7 @@ class ToSOrPrivacyPolicyReadError < StandardError; end devise :registerable, :recoverable, :rememberable, :trackable, :validatable, :confirmable, - :omniauthable, :lockable, omniauth_providers: [:twitter, :facebook, :slack, :google_oauth2] + :omniauthable, omniauth_providers: [:twitter, :facebook, :slack, :google_oauth2] before_create :skip_confirmation_for_non_email_provider, :set_last_received_terms_email_at after_create :create_source_and_account, :set_source_image, :send_welcome_email diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 2268815f45..0c43abae93 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -49,13 +49,6 @@ def http_auth_body end config.mailer = 'DeviseMailer' config.invite_for = 1.month - - # Account lockout - config.lock_strategy = :failed_attempts - config.unlock_strategy = :time - config.unlock_keys = [ :time ] - config.maximum_attempts = CheckConfig.get('devise_maximum_attempts', 5) * 2 - config.unlock_in = CheckConfig.get('devise_unlock_accounts_after', 1).hour end AuthTrail.geocode = false diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index f1971262b4..ac92f49180 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -1,21 +1,32 @@ class Rack::Attack - # Throttle login attempts by IP address - throttle('logins/ip', limit: proc { CheckConfig.get('login_rate_limit', 10, :integer) }, period: 60.seconds) do |req| - if req.path == '/api/users/sign_in' && req.post? - req.ip - end + # Throttle all graphql requests by IP address + throttle('api/graphql', limit: proc { CheckConfig.get('api_rate_limit', 100, :integer) }, period: 60.seconds) do |req| + req.ip if req.path == '/api/graphql' + end + + # Blocklist IP addresses that are permanently blocked + blocklist('block aggressive IPs') do |req| + Rails.cache.fetch("block:#{req.ip}") { false } end - # Throttle login attempts by email address - throttle('logins/email', limit: proc { CheckConfig.get('login_rate_limit', 10, :integer) }, period: 60.seconds) do |req| + # Track excessive login attempts for permanent blocking + track('track excessive logins/ip') do |req| if req.path == '/api/users/sign_in' && req.post? - # Return the email if present, nil otherwise - req.params['user']['email'].presence if req.params['user'] + # Increment the counter for the IP and check if it should be blocked + count = Rails.cache.increment("track:#{req.ip}") || 0 + Rails.cache.write("track:#{req.ip}", count, expires_in: 1.hour) + + # Add IP to blocklist if count exceeds the threshold + if count >= CheckConfig.get('login_block_limit', 100, :integer) + Rails.cache.write("block:#{req.ip}", true) # No expiration + end + + req.ip end end - # Throttle all graphql requests by IP address - throttle('api/graphql', limit: proc { CheckConfig.get('api_rate_limit', 100, :integer) }, period: 60.seconds) do |req| - req.ip if req.path == '/api/graphql' + # Response to blocked requests + self.blocklisted_response = lambda do |env| + [ 403, {}, ['Blocked - Your IP has been permanently blocked due to suspicious activity.']] end end diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index c6f878d4a5..1afbac8faf 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -77,34 +77,4 @@ def setup assert_response 400 assert_not_nil @controller.current_api_user end - - # test "should lock user after excessive login requests" do - # p1 = random_complex_password - # u = create_user login: 'test', password: p1, password_confirmation: p1, email: 'test@test.com' - # Devise.maximum_attempts = 2 - - # 2.times do - # post :create, params: { api_user: { email: 'test@test.com', password: random_complex_password } } - # end - - # u.reload - # assert u.access_locked? - # assert_not_nil u.locked_at - # end - - # test "should unlock locked user accounts after specified time" do - # travel_to Time.zone.local(2023, 12, 12, 01, 04, 44) - # u = create_user login: 'test', password: p1, password_confirmation: p1, email: 'test@test.com' - # Devise.unlock_in = 10.minutes - - - # u.lock_access! - - # travel 30.minutes - # post :create, params: { api_user: { email: 'test@test.com', password: p1 } } - - # u.reload - # assert !u.access_locked? - # assert_nil u.locked_at - # end end diff --git a/test/lib/check_rack_attack_test.rb b/test/lib/check_rack_attack_test.rb index e643e7c3a9..93ad61be07 100644 --- a/test/lib/check_rack_attack_test.rb +++ b/test/lib/check_rack_attack_test.rb @@ -6,8 +6,8 @@ class ThrottlingTest < ActionDispatch::IntegrationTest end test "should throttle excessive requests to /api/graphql" do - stub_configs({ 'api_rate_limit' => 2 }) do - 2.times do + stub_configs({ 'api_rate_limit' => 3 }) do + 3.times do post api_graphql_path assert_response :unauthorized end @@ -17,16 +17,16 @@ class ThrottlingTest < ActionDispatch::IntegrationTest end end - test "should throttle excessive requests to /api/users/sign_in" do - stub_configs({ 'login_rate_limit' => 2 }) do + test "should block IPs with excessive repeated requests to /api/users/sign_in" do + stub_configs({ 'login_block_limit' => 2 }) do user_params = { api_user: { email: 'user@example.com', password: random_complex_password } } - 2.times do + 3.times do post api_user_session_path, params: user_params, as: :json end post api_user_session_path, params: user_params, as: :json - assert_response :too_many_requests + assert_response :forbidden end end end From cf490f2741c1ab373aec83b80b6ce4072cc9b190 Mon Sep 17 00:00:00 2001 From: Mohamed El-Sawy Date: Wed, 22 May 2024 12:16:48 +0300 Subject: [PATCH 10/22] CV2-4638: fix sentry issue and send ID to SecurityMailer instead of the object (#1896) --- app/mailers/security_mailer.rb | 12 +++++++----- app/models/login_activity.rb | 6 +++--- lib/tasks/check_new_signup_notification.rake | 2 +- test/mailers/security_mailer_test.rb | 14 +++++++------- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/app/mailers/security_mailer.rb b/app/mailers/security_mailer.rb index 292d26e022..d0b14efb32 100644 --- a/app/mailers/security_mailer.rb +++ b/app/mailers/security_mailer.rb @@ -1,7 +1,9 @@ class SecurityMailer < ApplicationMailer layout nil - def notify(user, type, activity) + def notify(user_id, type, activity_id) + user = User.find_by_id(user_id) + activity = LoginActivity.find_by_id(activity_id) address = [] Geocoder.configure(language: I18n.locale) ip_result = Geocoder.search(activity.ip).first @@ -16,16 +18,16 @@ def notify(user, type, activity) @location = address.compact.join(', ') @timestamp = activity.created_at @ip = activity.ip - @platform = @user_agent.os.split.first + @platform = begin @user_agent.os.split.first rescue 'Unknown' end subject = I18n.t("mail_security.#{type}_subject", app_name: CheckConfig.get('app_name'), browser: @user_agent.browser, platform: @platform) mail(to: email, subject: subject) end - def custom_notification(user, subject) - @user = user + def custom_notification(user_id, subject) + @user = User.find_by_id(user_id) attachments.inline['signup.png'] = File.read('public/images/signup.png') - mail(to: user.email, subject: subject) + mail(to: @user.email, subject: subject) end end diff --git a/app/models/login_activity.rb b/app/models/login_activity.rb index d84f4ea8cc..48d8ab35ca 100644 --- a/app/models/login_activity.rb +++ b/app/models/login_activity.rb @@ -40,10 +40,10 @@ def send_successfull_login_notification(user) activities = user.login_activities.where(success: true).last(2) ip = activities.map(&:ip).uniq if ip.count > 1 - SecurityMailer.delay.notify(user, 'ip', self) + SecurityMailer.delay.notify(user.id, 'ip', self.id) else user_agent = activities.map(&:user_agent).uniq - SecurityMailer.delay.notify(user, 'device', self) if user_agent.count > 1 + SecurityMailer.delay.notify(user.id, 'device', self.id) if user_agent.count > 1 end end @@ -55,7 +55,7 @@ def send_failed_login_notification(user) failed_attempts = LoginActivity.where('identity = ? AND success = ? AND created_at > ?', self.identity, false, last_notification).count end if failed_attempts >= CheckConfig.get('failed_attempts', 4).to_i - SecurityMailer.delay.notify(user, 'failed', self) + SecurityMailer.delay.notify(user.id, 'failed', self.id) user.set_failed_notifications_time = self.created_at user.skip_check_ability = true user.save! diff --git a/lib/tasks/check_new_signup_notification.rake b/lib/tasks/check_new_signup_notification.rake index 36278c01ff..06894cd920 100644 --- a/lib/tasks/check_new_signup_notification.rake +++ b/lib/tasks/check_new_signup_notification.rake @@ -5,7 +5,7 @@ namespace :check do User.where.not(email: '').find_each do |u| unless u.encrypted_password? print '.' - SecurityMailer.delay.custom_notification(u, subject) + SecurityMailer.delay.custom_notification(u.id, subject) end end end diff --git a/test/mailers/security_mailer_test.rb b/test/mailers/security_mailer_test.rb index 6804e1861c..9d8c5d29e2 100644 --- a/test/mailers/security_mailer_test.rb +++ b/test/mailers/security_mailer_test.rb @@ -11,33 +11,33 @@ def teardown test "should send security notification" do user = create_user la = create_login_activity user: user, success: true - email = SecurityMailer.notify(user, 'ip', la) + email = SecurityMailer.notify(user.id, 'ip', la.id) assert_emails 1 do email.deliver_now end - email = SecurityMailer.notify(user, 'device', la) + email = SecurityMailer.notify(user.id, 'device', la.id) assert_emails 1 do email.deliver_now end - email = SecurityMailer.notify(user, 'failed', la) + email = SecurityMailer.notify(user.id, 'failed', la.id) assert_emails 1 do email.deliver_now end Geocoder.stubs(:search).returns([OpenStruct.new(data: { 'loc' => OpenStruct.new({ city: 'Cairo', country: 'Egypt' }) })]) la = create_login_activity user: user, success: true - email = SecurityMailer.notify(user, 'ip', la) + email = SecurityMailer.notify(user.id, 'ip', la.id) assert_emails 1 do email.deliver_now end Geocoder.unstub(:search) # should not notify if email empty user.update_columns(email: '') - email = SecurityMailer.notify(user, 'ip', la) + email = SecurityMailer.notify(user.id, 'ip', la.id) assert_emails 0 do email.deliver_now end user.update_columns(email: nil) - email = SecurityMailer.notify(user, 'ip', la) + email = SecurityMailer.notify(user.id, 'ip', la.id) assert_emails 0 do email.deliver_now end @@ -46,7 +46,7 @@ def teardown test "should send custom notification" do user = create_user subject = 'email subject' - email = SecurityMailer.custom_notification(user, subject) + email = SecurityMailer.custom_notification(user.id, subject) assert_emails 1 do email.deliver_now end From 98bdac2bcb324efb4f024ca28d44ac4fd14a9653 Mon Sep 17 00:00:00 2001 From: Caio Almeida <117518+caiosba@users.noreply.github.com> Date: Wed, 22 May 2024 10:08:47 -0300 Subject: [PATCH 11/22] Show list of existing media URLs in validation message when trying to import media from feed that already exist in the workspace. (#1895) Fixes: CV2-4631. --- app/models/cluster.rb | 10 ++++++++-- config/locales/en.yml | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/app/models/cluster.rb b/app/models/cluster.rb index eb1e2f7f21..5c10fcdb7f 100644 --- a/app/models/cluster.rb +++ b/app/models/cluster.rb @@ -31,8 +31,14 @@ def import_media_to_team(team, from_project_media, claim_title = nil, claim_cont def import_medias_to_team(team, claim_title, claim_context, parent_id = nil) # Find the first item in this cluster for which the media_id doesn't exist in the target team yet - from_project_media = self.project_medias.where.not(team_id: team.id).select(:id, :media_id).find { |item| !ProjectMedia.where(team_id: team.id, media_id: item.media_id).exists? } - raise 'No media to import. All media items from this item already exist in your workspace.' if from_project_media.nil? + # Should be fine to load these in memory since clusters don't contain thousands of media + existing_items = self.project_medias.where(team_id: team.id).to_a + from_project_media = self.project_medias.where.not(team_id: team.id).select(:id, :media_id).find do |item| + existing_item = ProjectMedia.where(team_id: team.id, media_id: item.media_id).first + existing_items << existing_item + existing_item.nil? + end + raise ActiveRecord::RecordNotUnique.new(I18n.t(:shared_feed_imported_media_already_exist, urls: existing_items.map(&:full_url).uniq.compact_blank.join(', '))) if from_project_media.nil? parent = nil if parent_id.nil? parent = self.import_media_to_team(team, from_project_media, claim_title, claim_context) diff --git a/config/locales/en.yml b/config/locales/en.yml index a267fd5fb8..79bf95e292 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -822,6 +822,7 @@ en: send_every_must_be_a_list_of_days_of_the_week: must be a list of days of the week. send_on_must_be_in_the_future: can't be in the past. cant_delete_default_folder: The default folder can't be deleted + shared_feed_imported_media_already_exist: "No media to import. All media items from this item already exist in your workspace: %{urls}" info: messages: sent_to_trash_by_rule: '"%{item_title}" has been sent to trash by an automation From dce85fac412f5af15e9bef100463f4e833f0f227 Mon Sep 17 00:00:00 2001 From: Caio Almeida <117518+caiosba@users.noreply.github.com> Date: Wed, 22 May 2024 19:58:10 -0300 Subject: [PATCH 12/22] Shared feed import validation message: Format URLs using Markdown (#1897) When trying to import media from a shared feed that already exist in the target workspace, return the existing item URLs in the error message as Markdown, so they can be better displayed on the frontend. Reference: CV2-4631. --- app/models/cluster.rb | 2 +- config/locales/en.yml | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/models/cluster.rb b/app/models/cluster.rb index 5c10fcdb7f..e5ff6cd605 100644 --- a/app/models/cluster.rb +++ b/app/models/cluster.rb @@ -38,7 +38,7 @@ def import_medias_to_team(team, claim_title, claim_context, parent_id = nil) existing_items << existing_item existing_item.nil? end - raise ActiveRecord::RecordNotUnique.new(I18n.t(:shared_feed_imported_media_already_exist, urls: existing_items.map(&:full_url).uniq.compact_blank.join(', '))) if from_project_media.nil? + raise ActiveRecord::RecordNotUnique.new(I18n.t(:shared_feed_imported_media_already_exist, urls: existing_items.uniq.compact_blank.collect{ |pm| "•︎ [#{pm.title}](#{pm.full_url})" }.join("\n"))) if from_project_media.nil? parent = nil if parent_id.nil? parent = self.import_media_to_team(team, from_project_media, claim_title, claim_context) diff --git a/config/locales/en.yml b/config/locales/en.yml index 79bf95e292..364f6241f5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -822,7 +822,10 @@ en: send_every_must_be_a_list_of_days_of_the_week: must be a list of days of the week. send_on_must_be_in_the_future: can't be in the past. cant_delete_default_folder: The default folder can't be deleted - shared_feed_imported_media_already_exist: "No media to import. All media items from this item already exist in your workspace: %{urls}" + shared_feed_imported_media_already_exist: |- + No media eligible to be imported into your workspace. + The media selected to import already exist in your workspace in the following items: + %{urls} info: messages: sent_to_trash_by_rule: '"%{item_title}" has been sent to trash by an automation From a8271cf57d3b7985038ba8446669ef8b355814c2 Mon Sep 17 00:00:00 2001 From: Jay Joshua <7008757+jayjay-w@users.noreply.github.com> Date: Sat, 25 May 2024 12:35:32 +0200 Subject: [PATCH 13/22] Change the way RackAttack blocks excessive login requests (#1899) Here we are changing the way RackAttach blocks excessive login requests - Use redis directly instead of Rails.Cache - Change deprecated responder method call --- config/initializers/rack_attack.rb | 21 ++++++++++++--------- test/lib/check_rack_attack_test.rb | 5 +++-- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index ac92f49180..dd5d86639f 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -1,32 +1,35 @@ class Rack::Attack + redis = Redis.new(REDIS_CONFIG) # Throttle all graphql requests by IP address - throttle('api/graphql', limit: proc { CheckConfig.get('api_rate_limit', 100, :integer) }, period: 60.seconds) do |req| + + throttle('api/graphql', limit: proc { CheckConfig.get('api_rate_limit', 100, :integer) }, period: 60.seconds) do |req| req.ip if req.path == '/api/graphql' end # Blocklist IP addresses that are permanently blocked blocklist('block aggressive IPs') do |req| - Rails.cache.fetch("block:#{req.ip}") { false } + redis.get("block:#{req.ip}") == "true" end # Track excessive login attempts for permanent blocking track('track excessive logins/ip') do |req| if req.path == '/api/users/sign_in' && req.post? + ip = req.ip # Increment the counter for the IP and check if it should be blocked - count = Rails.cache.increment("track:#{req.ip}") || 0 - Rails.cache.write("track:#{req.ip}", count, expires_in: 1.hour) + count = redis.incr("track:#{ip}") + redis.expire("track:#{ip}", 3600) # Set the expiration time to 1 hour # Add IP to blocklist if count exceeds the threshold - if count >= CheckConfig.get('login_block_limit', 100, :integer) - Rails.cache.write("block:#{req.ip}", true) # No expiration + if count.to_i >= CheckConfig.get('login_block_limit', 100, :integer) + redis.set("block:#{ip}", true) # No expiration end - req.ip + ip end end # Response to blocked requests - self.blocklisted_response = lambda do |env| - [ 403, {}, ['Blocked - Your IP has been permanently blocked due to suspicious activity.']] + self.blocklisted_responder = lambda do |req| + [403, {}, ['Blocked - Your IP has been permanently blocked due to suspicious activity.']] end end diff --git a/test/lib/check_rack_attack_test.rb b/test/lib/check_rack_attack_test.rb index 93ad61be07..eb294d866a 100644 --- a/test/lib/check_rack_attack_test.rb +++ b/test/lib/check_rack_attack_test.rb @@ -2,7 +2,8 @@ class ThrottlingTest < ActionDispatch::IntegrationTest setup do - Rails.cache.clear + redis = Redis.new(REDIS_CONFIG) + redis.flushdb end test "should throttle excessive requests to /api/graphql" do @@ -21,7 +22,7 @@ class ThrottlingTest < ActionDispatch::IntegrationTest stub_configs({ 'login_block_limit' => 2 }) do user_params = { api_user: { email: 'user@example.com', password: random_complex_password } } - 3.times do + 2.times do post api_user_session_path, params: user_params, as: :json end From 73a1c18f97381717e5984d00466fd9c4c5bd0ee7 Mon Sep 17 00:00:00 2001 From: Mohamed El-Sawy Date: Mon, 27 May 2024 06:39:49 +0300 Subject: [PATCH 14/22] CV2-4593: use ApplicationRecord.transaction to save message (#1898) * CV2-4593: try to reproduce the issue using unit test * CV2-4593: fix tests --- app/models/concerns/smooch_messages.rb | 44 +++++++++++++------------- test/models/bot/smooch_4_test.rb | 4 ++- test/models/bot/smooch_6_test.rb | 36 +++++++++++++++++++++ test/models/bot/smooch_test.rb | 3 +- 4 files changed, 63 insertions(+), 24 deletions(-) diff --git a/app/models/concerns/smooch_messages.rb b/app/models/concerns/smooch_messages.rb index 5170a06a98..3322ce3a50 100644 --- a/app/models/concerns/smooch_messages.rb +++ b/app/models/concerns/smooch_messages.rb @@ -333,29 +333,29 @@ def default_archived_flag def save_message(message_json, app_id, author = nil, request_type = 'default_requests', associated_obj = nil) message = JSON.parse(message_json) self.get_installation(self.installation_setting_id_keys, app_id) - Team.current = Team.where(id: self.config['team_id']).last - associated = nil - if ['default_requests', 'timeout_requests', 'irrelevant_search_result_requests'].include?(request_type) - message['archived'] = ['default_requests', 'irrelevant_search_result_requests'].include?(request_type) ? self.default_archived_flag : CheckArchivedFlags::FlagCodes::UNCONFIRMED - associated = self.create_project_media_from_message(message) - elsif ['menu_options_requests', 'resource_requests'].include?(request_type) - associated = associated_obj - elsif ['relevant_search_result_requests', 'timeout_search_requests'].include?(request_type) - message['archived'] = (request_type == 'relevant_search_result_requests' ? self.default_archived_flag : CheckArchivedFlags::FlagCodes::UNCONFIRMED) - associated = self.create_project_media_from_message(message) - end - - return if associated.nil? - - # Remember that we received this message. - hash = self.message_hash(message) - Rails.cache.write("smooch:message:#{hash}", associated.id) - - self.smooch_save_tipline_request(message, associated, app_id, author, request_type, associated_obj) + Team.current = Team.find self.config['team_id'].to_i + ApplicationRecord.transaction do + associated = nil + if ['default_requests', 'timeout_requests', 'irrelevant_search_result_requests'].include?(request_type) + message['archived'] = ['default_requests', 'irrelevant_search_result_requests'].include?(request_type) ? self.default_archived_flag : CheckArchivedFlags::FlagCodes::UNCONFIRMED + associated = self.create_project_media_from_message(message) + elsif ['menu_options_requests', 'resource_requests'].include?(request_type) + associated = associated_obj + elsif ['relevant_search_result_requests', 'timeout_search_requests'].include?(request_type) + message['archived'] = (request_type == 'relevant_search_result_requests' ? self.default_archived_flag : CheckArchivedFlags::FlagCodes::UNCONFIRMED) + associated = self.create_project_media_from_message(message) + end - # If item is published (or parent item), send a report right away - self.get_platform_from_message(message) - self.send_report_to_user(message['authorId'], message, associated, message['language'], 'fact_check_report') if self.should_try_to_send_report?(request_type, associated) + unless associated.nil? + # Remember that we received this message. + hash = self.message_hash(message) + Rails.cache.write("smooch:message:#{hash}", associated.id) + self.smooch_save_tipline_request(message, associated, app_id, author, request_type, associated_obj) + # If item is published (or parent item), send a report right away + self.get_platform_from_message(message) + self.send_report_to_user(message['authorId'], message, associated, message['language'], 'fact_check_report') if self.should_try_to_send_report?(request_type, associated) + end + end end def smooch_save_tipline_request(message, associated, app_id, author, request_type, associated_obj) diff --git a/test/models/bot/smooch_4_test.rb b/test/models/bot/smooch_4_test.rb index 34a4b37ece..0a1201b503 100644 --- a/test/models/bot/smooch_4_test.rb +++ b/test/models/bot/smooch_4_test.rb @@ -596,7 +596,9 @@ def teardown medias_count = Media.count assert_no_difference 'ProjectMedia.count' do - Bot::Smooch.save_message(message.to_json, @app_id) + assert_raises ActiveRecord::StatementInvalid do + Bot::Smooch.save_message(message.to_json, @app_id) + end end assert_equal medias_count, Media.count end diff --git a/test/models/bot/smooch_6_test.rb b/test/models/bot/smooch_6_test.rb index b1f22421c5..bcbd691027 100644 --- a/test/models/bot/smooch_6_test.rb +++ b/test/models/bot/smooch_6_test.rb @@ -440,6 +440,42 @@ def send_message_outside_24_hours_window(template, pm = nil) end end + test "should save only one item and one request for same tipline message" do + text = "This is message is so long that it is considered a media" + Sidekiq::Testing.inline! do + message = { + type: 'text', + text: text, + role: 'appUser', + received: 1573082583.219, + name: random_string, + authorId: random_string, + '_id': random_string, + source: { + originalMessageId: random_string, + originalMessageTimestamp: 1573082582, + type: 'whatsapp', + integrationId: random_string + }, + language: 'en', + } + author = BotUser.smooch_user + threads = [] + assert_difference 'ProjectMedia.count' do + assert_difference "TiplineRequest.count" do + assert_raises ActiveRecord::StatementInvalid do + 3.times do |i| + threads << Thread.new { + Bot::Smooch.save_message(message.to_json, @app_id, author, 'timeout_requests', nil) + } + end + threads.map(&:join) + end + end + end + end + end + test "should show main menu as buttons for non-WhatsApp platforms on tipline bot v2" do send_message_to_smooch_bot('Hello', @uid, { 'source' => { 'type' => 'telegram' } }) send_message_to_smooch_bot('1', @uid, { 'source' => { 'type' => 'telegram' } }) diff --git a/test/models/bot/smooch_test.rb b/test/models/bot/smooch_test.rb index 0bc83003ec..8724731980 100644 --- a/test/models/bot/smooch_test.rb +++ b/test/models/bot/smooch_test.rb @@ -429,7 +429,8 @@ def teardown '_id': random_string, authorId: uid, type: 'text', - text: text + text: text, + source: { type: "whatsapp" }, } ] payload = { From b4b1ee5972e5096231ee5a8205a2bcc0198e8b50 Mon Sep 17 00:00:00 2001 From: Mohamed El-Sawy Date: Mon, 27 May 2024 14:21:48 +0300 Subject: [PATCH 15/22] CV2-4604: fix sentry error (#1900) --- app/mailers/security_mailer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/mailers/security_mailer.rb b/app/mailers/security_mailer.rb index d0b14efb32..64acf4513f 100644 --- a/app/mailers/security_mailer.rb +++ b/app/mailers/security_mailer.rb @@ -6,7 +6,7 @@ def notify(user_id, type, activity_id) activity = LoginActivity.find_by_id(activity_id) address = [] Geocoder.configure(language: I18n.locale) - ip_result = Geocoder.search(activity.ip).first + ip_result = Geocoder.search(activity&.ip)&.first unless ip_result.blank? || ip_result.data["loc"].blank? loc_result = Geocoder.search(ip_result.data["loc"]).first address = [loc_result.city, loc_result.country] unless loc_result.nil? From a699d1f20099176c6264c2ad74d8e88a8375a9e4 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 28 May 2024 22:12:46 -0300 Subject: [PATCH 16/22] Bump rexml from 3.2.5 to 3.2.8 (#1892) Bumps [rexml](https://github.com/ruby/rexml) from 3.2.5 to 3.2.8. - [Release notes](https://github.com/ruby/rexml/releases) - [Changelog](https://github.com/ruby/rexml/blob/master/NEWS.md) - [Commits](https://github.com/ruby/rexml/compare/v3.2.5...v3.2.8) --- updated-dependencies: - dependency-name: rexml dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Gemfile.lock | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 02b368e757..a785b32865 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -726,7 +726,8 @@ GEM mime-types (>= 1.16, < 4.0) netrc (~> 0.8) retriable (3.1.2) - rexml (3.2.5) + rexml (3.2.8) + strscan (>= 3.0.9) rotp (6.3.0) rqrcode (2.1.1) chunky_png (~> 1.0) @@ -816,6 +817,7 @@ GEM ssrf_filter (1.0.7) streamio-ffmpeg (3.0.2) multi_json (~> 1.8) + strscan (3.1.0) swagger-docs (0.2.9) activesupport (>= 3) rails (>= 3) From f5d1ae32e7faf2366cdd40355ca45233a482abef Mon Sep 17 00:00:00 2001 From: Jay Joshua <7008757+jayjay-w@users.noreply.github.com> Date: Wed, 29 May 2024 04:13:29 +0300 Subject: [PATCH 17/22] Fix flaky test in OmniauthIntegrationTest (#1902) Since we added rate limiting for login requests, we need to flush redis before the tests run. --- test/integration/omniauth_integration_test.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/integration/omniauth_integration_test.rb b/test/integration/omniauth_integration_test.rb index 97b05534c2..b01a32e567 100644 --- a/test/integration/omniauth_integration_test.rb +++ b/test/integration/omniauth_integration_test.rb @@ -1,6 +1,11 @@ require_relative '../test_helper' class OmniauthIntegrationTest < ActionDispatch::IntegrationTest + setup do + redis = Redis.new(REDIS_CONFIG) + redis.flushdb + end + test "should close in case of failure" do get '/api/users/auth/slack/callback', params: { error: 'access_denied' } assert_redirected_to '/close.html' From e9fd77b72e87d98b5a1fb0c0fd3642e686c116eb Mon Sep 17 00:00:00 2001 From: Caio Almeida <117518+caiosba@users.noreply.github.com> Date: Thu, 30 May 2024 08:58:24 -0300 Subject: [PATCH 18/22] Do not raise exception if child item can't inherit status from parent item. (#1905) Otherwise, an exception can prevent the report from being sent. Fixes: CV2-4656. --- app/models/bot/smooch.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/bot/smooch.rb b/app/models/bot/smooch.rb index 59416cd061..cd0285efb4 100644 --- a/app/models/bot/smooch.rb +++ b/app/models/bot/smooch.rb @@ -71,7 +71,7 @@ def self.inherit_status_and_send_report(rid) status = parent.last_verification_status if !s.nil? && s.status != status s.status = status - s.save! + s.save end ::Bot::Smooch.send_report_from_parent_to_child(parent.id, target.id) end From 410ef56ce5a7540a5674eaf8cc0289b26de745b6 Mon Sep 17 00:00:00 2001 From: Devin Gaffney Date: Mon, 3 Jun 2024 14:15:56 -0700 Subject: [PATCH 19/22] CV2-4637 comment out functionality that would call soon-to-be deprecated endpoints (#1906) * CV2-4637 comment out functionality that would call soon-to-be deprecated endpoints * revert red herring and comment out just broken modality * update to reenable * update to remove now unecessary test * Update request_test.rb to comment out instead of delete --- app/models/request.rb | 30 +++++++++++++++--------------- test/models/request_test.rb | 30 +++++++++++++++--------------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/app/models/request.rb b/app/models/request.rb index 058d93755c..81c9e58346 100644 --- a/app/models/request.rb +++ b/app/models/request.rb @@ -55,11 +55,11 @@ def attach_to_similar_request!(alegre_limit = 20) params = { text: media.quote, models: models_thresholds.keys, per_model_threshold: models_thresholds.transform_values{ |v| v['threshold'] }, limit: alegre_limit, context: context } similar_request_id = ::Bot::Alegre.request('post', '/text/similarity/search/', params)&.dig('result').to_a.collect{ |result| result&.dig('_source', 'context', 'request_id').to_i }.find{ |id| id != 0 && id < self.id } end - elsif ['UploadedImage', 'UploadedAudio', 'UploadedVideo'].include?(media.type) - threshold = 0.85 #FIXME: Should be feed setting - type = media.type.gsub(/^Uploaded/, '').downcase - params = { url: media.file.file.public_url, threshold: threshold, limit: alegre_limit, context: context } - similar_request_id = ::Bot::Alegre.request('post', "/#{type}/similarity/search/", params)&.dig('result').to_a.collect{ |result| result&.dig('context').to_a.collect{ |c| c['request_id'].to_i } }.flatten.find{ |id| id != 0 && id < self.id } + # elsif ['UploadedImage', 'UploadedAudio', 'UploadedVideo'].include?(media.type) + # threshold = 0.85 #FIXME: Should be feed setting + # type = media.type.gsub(/^Uploaded/, '').downcase + # params = { url: media.file.file.public_url, threshold: threshold, limit: alegre_limit, context: context } + # similar_request_id = ::Bot::Alegre.request('post', "/#{type}/similarity/search/", params)&.dig('result').to_a.collect{ |result| result&.dig('context').to_a.collect{ |c| c['request_id'].to_i } }.flatten.find{ |id| id != 0 && id < self.id } end end unless similar_request_id.blank? @@ -195,16 +195,16 @@ def self.send_to_alegre(id) context: context } ::Bot::Alegre.request('post', '/text/similarity/', params) - elsif ['UploadedImage', 'UploadedAudio', 'UploadedVideo'].include?(media.type) - type = media.type.gsub(/^Uploaded/, '').downcase - url = media.file&.file&.public_url - params = { - doc_id: doc_id, - url: url, - context: context, - match_across_content_types: true, - } - ::Bot::Alegre.request('post', "/#{type}/similarity/", params) + # elsif ['UploadedImage', 'UploadedAudio', 'UploadedVideo'].include?(media.type) + # type = media.type.gsub(/^Uploaded/, '').downcase + # url = media.file&.file&.public_url + # params = { + # doc_id: doc_id, + # url: url, + # context: context, + # match_across_content_types: true, + # } + # ::Bot::Alegre.request('post', "/#{type}/similarity/", params) end end diff --git a/test/models/request_test.rb b/test/models/request_test.rb index 00a62ccaae..354bd184ec 100644 --- a/test/models/request_test.rb +++ b/test/models/request_test.rb @@ -176,21 +176,21 @@ def setup assert_not_equal [r2], r1.reload.similar_requests Bot::Alegre.unstub(:request) end - - test "should attach to similar media" do - Bot::Alegre.stubs(:request).returns(true) - f = create_feed - m1 = create_uploaded_image - r1 = create_request request_type: 'image', media: m1, feed: f - m2 = create_uploaded_image - r2 = create_request request_type: 'image', media: m2, feed: f - response = { 'result' => [{ 'context' => [{ 'request_id' => r1.id }] }] } - Bot::Alegre.stubs(:request).with('post', '/image/similarity/search/', { url: m2.file.file.public_url, threshold: 0.85, limit: 20, context: { feed_id: f.id } }).returns(response) - r2.attach_to_similar_request! - assert_equal r1, r2.reload.similar_to_request - assert_equal [r2], r1.reload.similar_requests - Bot::Alegre.unstub(:request) - end + + # test "should attach to similar media" do + # Bot::Alegre.stubs(:request).returns(true) + # f = create_feed + # m1 = create_uploaded_image + # r1 = create_request request_type: 'image', media: m1, feed: f + # m2 = create_uploaded_image + # r2 = create_request request_type: 'image', media: m2, feed: f + # response = { 'result' => [{ 'context' => [{ 'request_id' => r1.id }] }] } + # Bot::Alegre.stubs(:request).with('post', '/image/similarity/search/', { url: m2.file.file.public_url, threshold: 0.85, limit: 20, context: { feed_id: f.id } }).returns(response) + # r2.attach_to_similar_request! + # assert_equal r1, r2.reload.similar_to_request + # assert_equal [r2], r1.reload.similar_requests + # Bot::Alegre.unstub(:request) + # end test "should attach to similar link" do Bot::Alegre.stubs(:request).returns(true) From fe6a0f903a723596933f7557361a9f74a9e42f77 Mon Sep 17 00:00:00 2001 From: Mohamed El-Sawy Date: Tue, 4 Jun 2024 18:49:55 +0300 Subject: [PATCH 20/22] CV2-4666: Sentry issue (#1907) * CV2-4666: check existing smooch_message_id before creating new one and pass id to SmoochWorker instead of object * CV2-4666: fix tests and apply PR comments --- app/models/concerns/smooch_messages.rb | 9 +++++++-- app/workers/smooch_worker.rb | 11 ++++++++--- test/models/bot/smooch_3_test.rb | 10 +++------- test/models/bot/smooch_4_test.rb | 4 +--- test/models/bot/smooch_6_test.rb | 8 ++++---- test/models/bot/smooch_7_test.rb | 18 +++++++++--------- 6 files changed, 32 insertions(+), 28 deletions(-) diff --git a/app/models/concerns/smooch_messages.rb b/app/models/concerns/smooch_messages.rb index 3322ce3a50..0e8d90b28a 100644 --- a/app/models/concerns/smooch_messages.rb +++ b/app/models/concerns/smooch_messages.rb @@ -322,7 +322,9 @@ def save_message_later(message, app_id, request_type = 'default_requests', annot queue = RequestStore.store[:smooch_bot_queue].to_s queue = queue.blank? ? 'smooch_priority' : (mapping[queue] || 'smooch_priority') type = (message['type'] == 'text' && !message['text'][/https?:\/\/[^\s]+/, 0].blank?) ? 'link' : message['type'] - SmoochWorker.set(queue: queue).perform_in(1.second + interval.seconds, message.to_json, type, app_id, request_type, YAML.dump(annotated)) + associated_id = annotated&.id + associated_class = annotated.class.name + SmoochWorker.set(queue: queue).perform_in(1.second + interval.seconds, message.to_json, type, app_id, request_type, associated_id, associated_class) end def default_archived_flag @@ -330,8 +332,11 @@ def default_archived_flag Bot::Alegre.team_has_alegre_bot_installed?(team_id) ? CheckArchivedFlags::FlagCodes::PENDING_SIMILARITY_ANALYSIS : CheckArchivedFlags::FlagCodes::NONE end - def save_message(message_json, app_id, author = nil, request_type = 'default_requests', associated_obj = nil) + def save_message(message_json, app_id, author = nil, request_type = 'default_requests', associated_id = nil, associated_class = nil) message = JSON.parse(message_json) + return if TiplineRequest.where(smooch_message_id: message['_id']).exists? + associated_obj = nil + associated_obj = associated_class.constantize.where(id: associated_id).last unless associated_id.nil? self.get_installation(self.installation_setting_id_keys, app_id) Team.current = Team.find self.config['team_id'].to_i ApplicationRecord.transaction do diff --git a/app/workers/smooch_worker.rb b/app/workers/smooch_worker.rb index f307669605..89ea760b50 100644 --- a/app/workers/smooch_worker.rb +++ b/app/workers/smooch_worker.rb @@ -6,11 +6,16 @@ class SmoochWorker sidekiq_retry_in { |_count, _exception| 20 } - def perform(json_message, type, app_id, request_type, annotated) - annotated = YAML.load(annotated) + def perform(message_json, type, app_id, request_type, associated_id = nil, associated_class = nil) User.current = BotUser.smooch_user + # This is a temp code for existing jobs and should remove it in next deployment + annotated = begin YAML.load(associated_id) rescue nil end + unless annotated.nil? + associated_id = annotated.id + associated_class = annotated.class.name + end benchmark.send("smooch_save_#{type}_message") do - Bot::Smooch.save_message(json_message, app_id, User.current, request_type, annotated) + Bot::Smooch.save_message(message_json, app_id, User.current, request_type, associated_id, associated_class) end benchmark.finish User.current = nil diff --git a/test/models/bot/smooch_3_test.rb b/test/models/bot/smooch_3_test.rb index 046a4f501a..84a56a7e6d 100644 --- a/test/models/bot/smooch_3_test.rb +++ b/test/models/bot/smooch_3_test.rb @@ -35,7 +35,7 @@ def teardown language: 'en' }.to_json assert_difference 'ProjectMedia.count' do - SmoochWorker.perform_async(json_message, 'image', @app_id, 'default_requests', YAML.dump({})) + SmoochWorker.perform_async(json_message, 'image', @app_id, 'default_requests') end end end @@ -203,9 +203,7 @@ def teardown Bot::Smooch.save_message(message.to_json, @app_id) end message['mediaUrl'] = @video_url_2 - assert_raises 'ActiveRecord::RecordInvalid' do - Bot::Smooch.save_message(message.to_json, @app_id) - end + Bot::Smooch.save_message(message.to_json, @app_id) # audio message = { type: 'file', @@ -224,9 +222,7 @@ def teardown Bot::Smooch.save_message(message.to_json, @app_id) end message['mediaUrl'] = @audio_url_2 - assert_raises 'ActiveRecord::RecordInvalid' do - Bot::Smooch.save_message(message.to_json, @app_id) - end + Bot::Smooch.save_message(message.to_json, @app_id) end end diff --git a/test/models/bot/smooch_4_test.rb b/test/models/bot/smooch_4_test.rb index 0a1201b503..5ed99aa1e7 100644 --- a/test/models/bot/smooch_4_test.rb +++ b/test/models/bot/smooch_4_test.rb @@ -596,9 +596,7 @@ def teardown medias_count = Media.count assert_no_difference 'ProjectMedia.count' do - assert_raises ActiveRecord::StatementInvalid do - Bot::Smooch.save_message(message.to_json, @app_id) - end + Bot::Smooch.save_message(message.to_json, @app_id) end assert_equal medias_count, Media.count end diff --git a/test/models/bot/smooch_6_test.rb b/test/models/bot/smooch_6_test.rb index bcbd691027..baf44e760c 100644 --- a/test/models/bot/smooch_6_test.rb +++ b/test/models/bot/smooch_6_test.rb @@ -396,7 +396,7 @@ def send_message_outside_24_hours_window(template, pm = nil) }, language: 'en', } - Bot::Smooch.save_message(message.to_json, @app_id, nil, 'menu_options_requests', pm) + Bot::Smooch.save_message(message.to_json, @app_id, nil, 'menu_options_requests', pm.id, pm.class.name) message = { type: 'text', text: random_string, @@ -413,7 +413,7 @@ def send_message_outside_24_hours_window(template, pm = nil) }, language: 'en', } - Bot::Smooch.save_message(message.to_json, @app_id, nil, 'menu_options_requests', pm) + Bot::Smooch.save_message(message.to_json, @app_id, nil, 'menu_options_requests', pm.id, pm.class.name) # verifiy new channel value data = {"main" => CheckChannels::ChannelCodes::MANUAL, "others" => [CheckChannels::ChannelCodes::WHATSAPP, CheckChannels::ChannelCodes::MESSENGER]} assert_equal data, pm.reload.channel @@ -433,7 +433,7 @@ def send_message_outside_24_hours_window(template, pm = nil) }, language: 'en', } - Bot::Smooch.save_message(message.to_json, @app_id, nil, 'menu_options_requests', pm2) + Bot::Smooch.save_message(message.to_json, @app_id, nil, 'menu_options_requests', pm2.id, pm2.class.name) # verifiy new channel value data = {"main" => CheckChannels::ChannelCodes::WHATSAPP, "others" => [CheckChannels::ChannelCodes::MESSENGER]} assert_equal data, pm2.reload.channel @@ -466,7 +466,7 @@ def send_message_outside_24_hours_window(template, pm = nil) assert_raises ActiveRecord::StatementInvalid do 3.times do |i| threads << Thread.new { - Bot::Smooch.save_message(message.to_json, @app_id, author, 'timeout_requests', nil) + Bot::Smooch.save_message(message.to_json, @app_id, author, 'timeout_requests', nil, nil) } end threads.map(&:join) diff --git a/test/models/bot/smooch_7_test.rb b/test/models/bot/smooch_7_test.rb index 6e170ecc11..ad852ffb92 100644 --- a/test/models/bot/smooch_7_test.rb +++ b/test/models/bot/smooch_7_test.rb @@ -502,12 +502,12 @@ def teardown language: 'en', } end - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'relevant_search_result_requests', pm) - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'relevant_search_result_requests', pm) - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'timeout_search_requests', pm) - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm) - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm) - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'relevant_search_result_requests', pm.id, pm.class.name) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'relevant_search_result_requests', pm.id, pm.class.name) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'timeout_search_requests', pm.id, pm.class.name) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm.id, pm.class.name) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm.id, pm.class.name) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm.id, pm.class.name) message = lambda do { type: 'text', @@ -526,9 +526,9 @@ def teardown language: 'en', } end - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'relevant_search_result_requests', pm2) - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm2) - Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm2) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'relevant_search_result_requests', pm2.id, pm2.class.name) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm2.id, pm2.class.name) + Bot::Smooch.save_message(message.call.to_json, @app_id, nil, 'irrelevant_search_result_requests', pm2.id, pm2.class.name) # Verify cached field assert_equal 6, pm.tipline_search_results_count assert_equal 2, pm.positive_tipline_search_results_count From 5c643cf1fa5ae06f9c163c11aecc1b4695d31603 Mon Sep 17 00:00:00 2001 From: Caio Almeida <117518+caiosba@users.noreply.github.com> Date: Wed, 5 Jun 2024 12:56:30 -0300 Subject: [PATCH 21/22] Fixing flaky integration tests (#1910) --- test/integration/api_version_integration_test.rb | 5 +++-- test/integration/dynamic_annotation_integration_test.rb | 5 +++++ test/integration/short_url_integration_test.rb | 5 +++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/test/integration/api_version_integration_test.rb b/test/integration/api_version_integration_test.rb index 9f67ffd59e..23936aea49 100644 --- a/test/integration/api_version_integration_test.rb +++ b/test/integration/api_version_integration_test.rb @@ -1,8 +1,9 @@ require_relative '../test_helper' class ApiVersionIntegrationTest < ActionDispatch::IntegrationTest - def setup - super + setup do + redis = Redis.new(REDIS_CONFIG) + redis.flushdb end test "should get default version 1" do diff --git a/test/integration/dynamic_annotation_integration_test.rb b/test/integration/dynamic_annotation_integration_test.rb index 8691d97b69..17b6c96390 100644 --- a/test/integration/dynamic_annotation_integration_test.rb +++ b/test/integration/dynamic_annotation_integration_test.rb @@ -1,6 +1,11 @@ require_relative '../test_helper' class DynamicAnnotationIntegrationTest < ActionDispatch::IntegrationTest + setup do + redis = Redis.new(REDIS_CONFIG) + redis.flushdb + end + test "should create task response free text" do assert_nothing_raised do at = create_annotation_type annotation_type: 'task_response_free_text', label: 'Task Response Free Text', description: 'Free text response that can added to a task' diff --git a/test/integration/short_url_integration_test.rb b/test/integration/short_url_integration_test.rb index f0592e008d..87a5a95416 100644 --- a/test/integration/short_url_integration_test.rb +++ b/test/integration/short_url_integration_test.rb @@ -1,6 +1,11 @@ require_relative '../test_helper' class ShortUrlIntegrationTest < ActionDispatch::IntegrationTest + setup do + redis = Redis.new(REDIS_CONFIG) + redis.flushdb + end + test "should not access by other host other than the short host" do assert_routing "#{CheckConfig.get('short_url_host')}/x1y2z3", { host: 'localhost', controller: 'shortener/shortened_urls', action: 'show', id: 'x1y2z3' } end From 7e784d3f7adc66f319b7267aee675686025cf904 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 5 Jun 2024 16:38:25 -0300 Subject: [PATCH 22/22] Bump actionpack from 6.1.7.7 to 6.1.7.8 (#1908) Bumps [actionpack](https://github.com/rails/rails) from 6.1.7.7 to 6.1.7.8. - [Release notes](https://github.com/rails/rails/releases) - [Changelog](https://github.com/rails/rails/blob/v7.1.3.4/actionpack/CHANGELOG.md) - [Commits](https://github.com/rails/rails/compare/v6.1.7.7...v6.1.7.8) --- updated-dependencies: - dependency-name: actionpack dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Gemfile.lock | 114 +++++++++++++++++++++++++-------------------------- 1 file changed, 57 insertions(+), 57 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index a785b32865..65e7439737 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -74,62 +74,62 @@ GEM specs: aasm (5.2.0) concurrent-ruby (~> 1.0) - actioncable (6.1.7.7) - actionpack (= 6.1.7.7) - activesupport (= 6.1.7.7) + actioncable (6.1.7.8) + actionpack (= 6.1.7.8) + activesupport (= 6.1.7.8) nio4r (~> 2.0) websocket-driver (>= 0.6.1) - actionmailbox (6.1.7.7) - actionpack (= 6.1.7.7) - activejob (= 6.1.7.7) - activerecord (= 6.1.7.7) - activestorage (= 6.1.7.7) - activesupport (= 6.1.7.7) + actionmailbox (6.1.7.8) + actionpack (= 6.1.7.8) + activejob (= 6.1.7.8) + activerecord (= 6.1.7.8) + activestorage (= 6.1.7.8) + activesupport (= 6.1.7.8) mail (>= 2.7.1) - actionmailer (6.1.7.7) - actionpack (= 6.1.7.7) - actionview (= 6.1.7.7) - activejob (= 6.1.7.7) - activesupport (= 6.1.7.7) + actionmailer (6.1.7.8) + actionpack (= 6.1.7.8) + actionview (= 6.1.7.8) + activejob (= 6.1.7.8) + activesupport (= 6.1.7.8) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (6.1.7.7) - actionview (= 6.1.7.7) - activesupport (= 6.1.7.7) + actionpack (6.1.7.8) + actionview (= 6.1.7.8) + activesupport (= 6.1.7.8) rack (~> 2.0, >= 2.0.9) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.2.0) - actiontext (6.1.7.7) - actionpack (= 6.1.7.7) - activerecord (= 6.1.7.7) - activestorage (= 6.1.7.7) - activesupport (= 6.1.7.7) + actiontext (6.1.7.8) + actionpack (= 6.1.7.8) + activerecord (= 6.1.7.8) + activestorage (= 6.1.7.8) + activesupport (= 6.1.7.8) nokogiri (>= 1.8.5) - actionview (6.1.7.7) - activesupport (= 6.1.7.7) + actionview (6.1.7.8) + activesupport (= 6.1.7.8) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.1, >= 1.2.0) - activejob (6.1.7.7) - activesupport (= 6.1.7.7) + activejob (6.1.7.8) + activesupport (= 6.1.7.8) globalid (>= 0.3.6) - activemodel (6.1.7.7) - activesupport (= 6.1.7.7) - activerecord (6.1.7.7) - activemodel (= 6.1.7.7) - activesupport (= 6.1.7.7) + activemodel (6.1.7.8) + activesupport (= 6.1.7.8) + activerecord (6.1.7.8) + activemodel (= 6.1.7.8) + activesupport (= 6.1.7.8) activerecord-import (1.1.0) activerecord (>= 3.2) - activestorage (6.1.7.7) - actionpack (= 6.1.7.7) - activejob (= 6.1.7.7) - activerecord (= 6.1.7.7) - activesupport (= 6.1.7.7) + activestorage (6.1.7.8) + actionpack (= 6.1.7.8) + activejob (= 6.1.7.8) + activerecord (= 6.1.7.8) + activesupport (= 6.1.7.8) marcel (~> 1.0) mini_mime (>= 1.1.0) - activesupport (6.1.7.7) + activesupport (6.1.7.8) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 1.6, < 2) minitest (>= 5.1) @@ -365,7 +365,7 @@ GEM ffi-compiler (>= 1.0, < 2.0) http_parser.rb (0.6.0) httpclient (2.8.3) - i18n (1.14.1) + i18n (1.14.5) concurrent-ruby (~> 1.0) i18n-generators (1.1.3) mechanize @@ -420,7 +420,7 @@ GEM mime-types-data (3.2022.0105) mini_magick (4.10.1) mini_mime (1.1.5) - mini_portile2 (2.8.6) + mini_portile2 (2.8.7) minitest (5.19.0) minitest-hooks (1.5.0) minitest (> 5.3) @@ -635,7 +635,7 @@ GEM pusher-signature (~> 0.1.8) pusher-signature (0.1.8) raabro (1.4.0) - racc (1.7.3) + racc (1.8.0) rack (2.2.8.1) rack-attack (6.7.0) rack (>= 1.0, < 4) @@ -646,20 +646,20 @@ GEM rack-test (1.1.0) rack (>= 1.0, < 3) railroady (1.6.0) - rails (6.1.7.7) - actioncable (= 6.1.7.7) - actionmailbox (= 6.1.7.7) - actionmailer (= 6.1.7.7) - actionpack (= 6.1.7.7) - actiontext (= 6.1.7.7) - actionview (= 6.1.7.7) - activejob (= 6.1.7.7) - activemodel (= 6.1.7.7) - activerecord (= 6.1.7.7) - activestorage (= 6.1.7.7) - activesupport (= 6.1.7.7) + rails (6.1.7.8) + actioncable (= 6.1.7.8) + actionmailbox (= 6.1.7.8) + actionmailer (= 6.1.7.8) + actionpack (= 6.1.7.8) + actiontext (= 6.1.7.8) + actionview (= 6.1.7.8) + activejob (= 6.1.7.8) + activemodel (= 6.1.7.8) + activerecord (= 6.1.7.8) + activestorage (= 6.1.7.8) + activesupport (= 6.1.7.8) bundler (>= 1.15.0) - railties (= 6.1.7.7) + railties (= 6.1.7.8) sprockets-rails (>= 2.0.0) rails-controller-testing (1.0.5) actionpack (>= 5.0.1.rc1) @@ -672,9 +672,9 @@ GEM rails-graphql-generator (0.1.0) rails-html-sanitizer (1.4.4) loofah (~> 2.19, >= 2.19.1) - railties (6.1.7.7) - actionpack (= 6.1.7.7) - activesupport (= 6.1.7.7) + railties (6.1.7.8) + actionpack (= 6.1.7.8) + activesupport (= 6.1.7.8) method_source rake (>= 12.2) thor (~> 1.0) @@ -878,7 +878,7 @@ GEM websocket-driver (0.7.6) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) - zeitwerk (2.6.13) + zeitwerk (2.6.15) PLATFORMS ruby