Commit 84efe260 authored by Ruben Davila's avatar Ruben Davila

Merge branch 'master' into 8-11-stable

parents 5cc14b56 5a4ecb98
......@@ -82,6 +82,7 @@ v 8.11.0 (unreleased)
- Make "New issue" button in Issue page less obtrusive !5457 (winniehell)
- Gitlab::Metrics.current_transaction needs to be public for RailsQueueDuration
- Fix search for notes which belongs to deleted objects
- Allow Akismet to be trained by submitting issues as spam or ham !5538
- Add GitLab Workhorse version to admin dashboard (Katarzyna Kobierska Ula Budziszewska)
- Allow branch names ending with .json for graph and network page !5579 (winniehell)
- Add the `sprockets-es6` gem
......@@ -114,6 +115,7 @@ v 8.11.0 (unreleased)
- Add auto-completition in pipeline (Katarzyna Kobierska Ula Budziszewska)
- Fix a memory leak caused by Banzai::Filter::SanitizationFilter
- Speed up todos queries by limiting the projects set we join with
- Ensure file editing in UI does not overwrite commited changes without warning user
v 8.10.5
- Add a data migration to fix some missing timestamps in the members table. !5670
......
......@@ -338,7 +338,7 @@ GEM
httparty (0.13.7)
json (~> 1.8)
multi_xml (>= 0.5.2)
httpclient (2.7.0.1)
httpclient (2.8.2)
i18n (0.7.0)
ice_nine (0.11.1)
influxdb (0.2.3)
......
......@@ -164,6 +164,10 @@
@include btn-outline($white-light, $orange-normal, $orange-normal, $orange-light, $white-light, $orange-light);
}
&.btn-spam {
@include btn-outline($white-light, $red-normal, $red-normal, $red-light, $white-light, $red-light);
}
&.btn-danger,
&.btn-remove,
&.btn-red {
......
......@@ -14,4 +14,14 @@ class Admin::SpamLogsController < Admin::ApplicationController
head :ok
end
end
def mark_as_ham
spam_log = SpamLog.find(params[:id])
if HamService.new(spam_log).mark_as_ham!
redirect_to admin_spam_logs_path, notice: 'Spam log successfully submitted as ham.'
else
redirect_to admin_spam_logs_path, alert: 'Error with Akismet. Please check the logs for more info.'
end
end
end
module SpammableActions
extend ActiveSupport::Concern
included do
before_action :authorize_submit_spammable!, only: :mark_as_spam
end
def mark_as_spam
if SpamService.new(spammable).mark_as_spam!
redirect_to spammable, notice: "#{spammable.class.to_s} was submitted to Akismet successfully."
else
redirect_to spammable, alert: 'Error with Akismet. Please check the logs for more info.'
end
end
private
def spammable
raise NotImplementedError, "#{self.class} does not implement #{__method__}"
end
def authorize_submit_spammable!
access_denied! unless current_user.admin?
end
end
......@@ -17,6 +17,7 @@ class Projects::BlobController < Projects::ApplicationController
before_action :require_branch_head, only: [:edit, :update]
before_action :editor_variables, except: [:show, :preview, :diff]
before_action :validate_diff_params, only: :diff
before_action :set_last_commit_sha, only: [:edit, :update]
def new
commit unless @repository.empty?
......@@ -33,7 +34,6 @@ class Projects::BlobController < Projects::ApplicationController
end
def edit
@last_commit = Gitlab::Git::Commit.last_for_path(@repository, @ref, @path).sha
blob.load_all_data!(@repository)
end
......@@ -55,6 +55,10 @@ class Projects::BlobController < Projects::ApplicationController
create_commit(Files::UpdateService, success_path: after_edit_path,
failure_view: :edit,
failure_path: namespace_project_blob_path(@project.namespace, @project, @id))
rescue Files::UpdateService::FileChangedError
@conflict = true
render :edit
end
def preview
......@@ -152,7 +156,8 @@ class Projects::BlobController < Projects::ApplicationController
file_path: @file_path,
commit_message: params[:commit_message],
file_content: params[:content],
file_content_encoding: params[:encoding]
file_content_encoding: params[:encoding],
last_commit_sha: params[:last_commit_sha]
}
end
......@@ -161,4 +166,9 @@ class Projects::BlobController < Projects::ApplicationController
render nothing: true
end
end
def set_last_commit_sha
@last_commit_sha = Gitlab::Git::Commit.
last_for_path(@repository, @ref, @path).sha
end
end
......@@ -4,6 +4,7 @@ class Projects::IssuesController < Projects::ApplicationController
include IssuableActions
include ToggleAwardEmoji
include IssuableCollections
include SpammableActions
before_action :redirect_to_external_issue_tracker, only: [:index, :new]
before_action :module_enabled
......@@ -185,6 +186,7 @@ class Projects::IssuesController < Projects::ApplicationController
alias_method :subscribable_resource, :issue
alias_method :issuable, :issue
alias_method :awardable, :issue
alias_method :spammable, :issue
def authorize_read_issue!
return render_404 unless can?(current_user, :read_issue, @issue)
......
......@@ -20,13 +20,19 @@ module SortingHelper
end
def projects_sort_options_hash
{
options = {
sort_value_name => sort_title_name,
sort_value_recently_updated => sort_title_recently_updated,
sort_value_oldest_updated => sort_title_oldest_updated,
sort_value_recently_created => sort_title_recently_created,
sort_value_oldest_created => sort_title_oldest_created,
}
if current_controller?('admin/projects')
options.merge!(sort_value_largest_repo => sort_title_largest_repo)
end
options
end
def sort_title_priority
......
module Spammable
extend ActiveSupport::Concern
module ClassMethods
def attr_spammable(attr, options = {})
spammable_attrs << [attr.to_s, options]
end
end
included do
has_one :user_agent_detail, as: :subject, dependent: :destroy
attr_accessor :spam
after_validation :check_for_spam, on: :create
cattr_accessor :spammable_attrs, instance_accessor: false do
[]
end
delegate :ip_address, :user_agent, to: :user_agent_detail, allow_nil: true
end
def submittable_as_spam?
if user_agent_detail
user_agent_detail.submittable?
else
false
end
end
def spam?
......@@ -13,4 +36,33 @@ module Spammable
def check_for_spam
self.errors.add(:base, "Your #{self.class.name.underscore} has been recognized as spam and has been discarded.") if spam?
end
def spam_title
attr = self.class.spammable_attrs.find do |_, options|
options.fetch(:spam_title, false)
end
public_send(attr.first) if attr && respond_to?(attr.first.to_sym)
end
def spam_description
attr = self.class.spammable_attrs.find do |_, options|
options.fetch(:spam_description, false)
end
public_send(attr.first) if attr && respond_to?(attr.first.to_sym)
end
def spammable_text
result = self.class.spammable_attrs.map do |attr|
public_send(attr.first)
end
result.reject(&:blank?).join("\n")
end
# Override in Spammable if further checks are necessary
def check_for_spam?
true
end
end
......@@ -36,6 +36,9 @@ class Issue < ActiveRecord::Base
scope :order_due_date_asc, -> { reorder('issues.due_date IS NULL, issues.due_date ASC') }
scope :order_due_date_desc, -> { reorder('issues.due_date IS NULL, issues.due_date DESC') }
attr_spammable :title, spam_title: true
attr_spammable :description, spam_description: true
state_machine :state, initial: :opened do
event :close do
transition [:reopened, :opened] => :closed
......@@ -262,4 +265,9 @@ class Issue < ActiveRecord::Base
def overdue?
due_date.try(:past?) || false
end
# Only issues on public projects should be checked for spam
def check_for_spam?
project.public?
end
end
......@@ -7,4 +7,8 @@ class SpamLog < ActiveRecord::Base
user.block
user.destroy
end
def text
[title, description].join("\n")
end
end
class UserAgentDetail < ActiveRecord::Base
belongs_to :subject, polymorphic: true
validates :user_agent, :ip_address, :subject_id, :subject_type, presence: true
def submittable?
!submitted?
end
end
class AkismetService
attr_accessor :owner, :text, :options
def initialize(owner, text, options = {})
@owner = owner
@text = text
@options = options
end
def is_spam?
return false unless akismet_enabled?
params = {
type: 'comment',
text: text,
created_at: DateTime.now,
author: owner.name,
author_email: owner.email,
referrer: options[:referrer],
}
begin
is_spam, is_blatant = akismet_client.check(options[:ip_address], options[:user_agent], params)
is_spam || is_blatant
rescue => e
Rails.logger.error("Unable to connect to Akismet: #{e}, skipping check")
false
end
end
def submit_ham
return false unless akismet_enabled?
params = {
type: 'comment',
text: text,
author: owner.name,
author_email: owner.email
}
begin
akismet_client.submit_ham(options[:ip_address], options[:user_agent], params)
true
rescue => e
Rails.logger.error("Unable to connect to Akismet: #{e}, skipping!")
false
end
end
def submit_spam
return false unless akismet_enabled?
params = {
type: 'comment',
text: text,
author: owner.name,
author_email: owner.email
}
begin
akismet_client.submit_spam(options[:ip_address], options[:user_agent], params)
true
rescue => e
Rails.logger.error("Unable to connect to Akismet: #{e}, skipping!")
false
end
end
private
def akismet_client
@akismet_client ||= ::Akismet::Client.new(current_application_settings.akismet_api_key,
Gitlab.config.gitlab.url)
end
def akismet_enabled?
current_application_settings.akismet_enabled
end
end
class CreateSpamLogService < BaseService
def initialize(project, user, params)
super(project, user, params)
end
def execute
spam_params = params.merge({ user_id: @current_user.id,
project_id: @project.id } )
spam_log = SpamLog.new(spam_params)
spam_log.save
spam_log
end
end
......@@ -15,6 +15,7 @@ module Files
else
params[:file_content]
end
@last_commit_sha = params[:last_commit_sha]
# Validate parameters
validate
......
......@@ -2,11 +2,34 @@ require_relative "base_service"
module Files
class UpdateService < Files::BaseService
class FileChangedError < StandardError; end
def commit
repository.update_file(current_user, @file_path, @file_content,
branch: @target_branch,
previous_path: @previous_path,
message: @commit_message)
end
private
def validate
super
if file_has_changed?
raise FileChangedError.new("You are attempting to update a file that has changed since you started editing it.")
end
end
def file_has_changed?
return false unless @last_commit_sha && last_commit
@last_commit_sha != last_commit.sha
end
def last_commit
@last_commit ||= Gitlab::Git::Commit.
last_for_path(@source_project.repository, @source_branch, @file_path)
end
end
end
class HamService
attr_accessor :spam_log
def initialize(spam_log)
@spam_log = spam_log
end
def mark_as_ham!
if akismet.submit_ham
spam_log.update_attribute(:submitted_as_ham, true)
else
false
end
end
private
def akismet
@akismet ||= AkismetService.new(
spam_log.user,
spam_log.text,
ip_address: spam_log.source_ip,
user_agent: spam_log.user_agent
)
end
end
......@@ -3,29 +3,34 @@ module Issues
def execute
filter_params
label_params = params.delete(:label_ids)
request = params.delete(:request)
api = params.delete(:api)
issue = project.issues.new(params)
issue.author = params[:author] || current_user
@request = params.delete(:request)
@api = params.delete(:api)
@issue = project.issues.new(params)
@issue.author = params[:author] || current_user
issue.spam = spam_check_service.execute(request, api)
@issue.spam = spam_service.check(@api)
if issue.save
issue.update_attributes(label_ids: label_params)
notification_service.new_issue(issue, current_user)
todo_service.new_issue(issue, current_user)
event_service.open_issue(issue, current_user)
issue.create_cross_references!(current_user)
execute_hooks(issue, 'open')
if @issue.save
@issue.update_attributes(label_ids: label_params)
notification_service.new_issue(@issue, current_user)
todo_service.new_issue(@issue, current_user)
event_service.open_issue(@issue, current_user)
user_agent_detail_service.create
@issue.create_cross_references!(current_user)
execute_hooks(@issue, 'open')
end
issue
@issue
end
private
def spam_check_service
SpamCheckService.new(project, current_user, params)
def spam_service
SpamService.new(@issue, @request)
end
def user_agent_detail_service
UserAgentDetailService.new(@issue, @request)
end
end
end
class SpamCheckService < BaseService
include Gitlab::AkismetHelper
attr_accessor :request, :api
def execute(request, api)
@request, @api = request, api
return false unless request || check_for_spam?(project)
return false unless is_spam?(request.env, current_user, text)
create_spam_log
true
end
private
def text
[params[:title], params[:description]].reject(&:blank?).join("\n")
end
def spam_log_attrs
{
user_id: current_user.id,
project_id: project.id,
title: params[:title],
description: params[:description],
source_ip: client_ip(request.env),
user_agent: user_agent(request.env),
noteable_type: 'Issue',
via_api: api
}
end
def create_spam_log
CreateSpamLogService.new(project, current_user, spam_log_attrs).execute
end
end
class SpamService
attr_accessor :spammable, :request, :options
def initialize(spammable, request = nil)
@spammable = spammable
@request = request
@options = {}
if @request
@options[:ip_address] = @request.env['action_dispatch.remote_ip'].to_s
@options[:user_agent] = @request.env['HTTP_USER_AGENT']
@options[:referrer] = @request.env['HTTP_REFERRER']
else
@options[:ip_address] = @spammable.ip_address
@options[:user_agent] = @spammable.user_agent
end
end
def check(api = false)
return false unless request && check_for_spam?
return false unless akismet.is_spam?
create_spam_log(api)
true
end
def mark_as_spam!
return false unless spammable.submittable_as_spam?
if akismet.submit_spam
spammable.user_agent_detail.update_attribute(:submitted, true)
else
false
end
end
private
def akismet
@akismet ||= AkismetService.new(
spammable_owner,
spammable.spammable_text,
options
)
end
def spammable_owner
@user ||= User.find(spammable_owner_id)
end
def spammable_owner_id
@owner_id ||=
if spammable.respond_to?(:author_id)
spammable.author_id
elsif spammable.respond_to?(:creator_id)
spammable.creator_id
end
end
def check_for_spam?
spammable.check_for_spam?
end
def create_spam_log(api)
SpamLog.create(
{
user_id: spammable_owner_id,
title: spammable.spam_title,
description: spammable.spam_description,
source_ip: options[:ip_address],
user_agent: options[:user_agent],
noteable_type: spammable.class.to_s,
via_api: api
}
)
end
end
class UserAgentDetailService
attr_accessor :spammable, :request
def initialize(spammable, request)
@spammable, @request = spammable, request
end
def create
return unless request
spammable.create_user_agent_detail(user_agent: request.env['HTTP_USER_AGENT'], ip_address: request.env['action_dispatch.remote_ip'].to_s)
end
end
......@@ -24,6 +24,11 @@
= link_to 'Remove user', admin_spam_log_path(spam_log, remove_user: true),
data: { confirm: "USER #{user.name} WILL BE REMOVED! Are you sure?" }, method: :delete, class: "btn btn-xs btn-remove"
%td
- if spam_log.submitted_as_ham?
.btn.btn-xs.disabled
Submitted as ham
- else
= link_to 'Submit as ham', mark_as_ham_admin_spam_log_path(spam_log), method: :post, class: 'btn btn-xs btn-warning'
- if user && !user.blocked?
= link_to 'Block user', block_admin_user_path(user), data: {confirm: 'USER WILL BE BLOCKED! Are you sure?'}, method: :put, class: "btn btn-xs"
- else
......
- page_title "Edit", @blob.path, @ref
- if @conflict
.alert.alert-danger
Someone edited the file the same time you did. Please check out
= link_to "the file", namespace_project_blob_path(@project.namespace, @project, tree_join(@target_branch, @file_path)), target: "_blank"
and make sure your changes will not unintentionally remove theirs.
.file-editor
%ul.nav-links.no-bottom.js-edit-mode
%li.active
......@@ -13,8 +19,7 @@
= form_tag(namespace_project_update_blob_path(@project.namespace, @project, @id), method: :put, class: 'form-horizontal js-quick-submit js-requires-input js-edit-blob-form') do
= render 'projects/blob/editor', ref: @ref, path: @path, blob_data: @blob.data
= render 'shared/new_commit_form', placeholder: "Update #{@blob.name}"
= hidden_field_tag 'last_commit', @last_commit
= hidden_field_tag 'last_commit_sha', @last_commit_sha
= hidden_field_tag 'content', '', id: "file-content"
= hidden_field_tag 'from_merge_request_id', params[:from_merge_request_id]
= render 'projects/commit_button', ref: @ref, cancel_path: namespace_project_blob_path(@project.namespace, @project, @id)
......
......@@ -37,14 +37,19 @@
= link_to 'Close issue', issue_path(@issue, issue: { state_event: :close }, status_only: true, format: 'json'), data: {no_turbolink: true}, class: "btn-close #{issue_button_visibility(@issue, true)}", title: 'Close issue'
%li
= link_to 'Edit', edit_namespace_project_issue_path(@project.namespace, @project, @issue)
- if @issue.submittable_as_spam? && current_user.admin?
%li
= link_to 'Submit as spam', mark_as_spam_namespace_project_issue_path(@project.namespace, @project, @issue), method: :post, class: 'btn-spam', title: 'Submit as spam'
- if can?(current_user, :create_issue, @project)
= link_to new_namespace_project_issue_path(@project.namespace, @project), class: 'hidden-xs hidden-sm btn btn-grouped new-issue-link btn-new btn-inverted', title: 'New issue', id: 'new_issue_link' do
New issue
- if can?(current_user, :update_issue, @issue)
= link_to 'Reopen issue', issue_path(@issue, issue: { state_event: :reopen }, status_only: true, format: 'json'), data: {no_turbolink: true}, class: "hidden-xs hidden-sm btn btn-grouped btn-reopen #{issue_button_visibility(@issue, false)}", title: 'Reopen issue'
= link_to 'Close issue', issue_path(@issue, issue: { state_event: :close }, status_only: true, format: 'json'), data: {no_turbolink: true}, class: "hidden-xs hidden-sm btn btn-grouped btn-close #{issue_button_visibility(@issue, true)}", title: 'Close issue'
= link_to edit_namespace_project_issue_path(@project.namespace, @project, @issue), class: 'hidden-xs hidden-sm btn btn-grouped issuable-edit' do
Edit
- if @issue.submittable_as_spam? && current_user.admin?
= link_to 'Submit as spam', mark_as_spam_namespace_project_issue_path(@project.namespace, @project, @issue), method: :post, class: 'hidden-xs hidden-sm btn btn-grouped btn-spam', title: 'Submit as spam'
= link_to 'Edit', edit_namespace_project_issue_path(@project.namespace, @project, @issue), class: 'hidden-xs hidden-sm btn btn-grouped issuable-edit'
.issue-details.issuable-details
......
......@@ -252,7 +252,11 @@ Rails.application.routes.draw do
resource :impersonation, only: :destroy
resources :abuse_reports, only: [:index, :destroy]
resources :spam_logs, only: [:index, :destroy]
resources :spam_logs, only: [:index, :destroy] do
member do
post :mark_as_ham
end
end
resources :applications
......@@ -813,6 +817,7 @@ Rails.application.routes.draw do
member do
post :toggle_subscription
post :toggle_award_emoji
post :mark_as_spam
get :referenced_merge_requests
get :related_branches
get :can_create_branch
......
class CreateUserAgentDetails < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
def change
create_table :user_agent_details do |t|
t.string :user_agent, null: false
t.string :ip_address, null: false
t.integer :subject_id, null: false
t.string :subject_type, null: false
t.boolean :submitted, default: false, null: false
t.timestamps null: false
end
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class RemoveProjectIdFromSpamLogs < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = true
# When a migration requires downtime you **must** uncomment the following
# constant and define a short and easy to understand explanation as to why the
# migration requires downtime.
DOWNTIME_REASON = 'Removing a column that contains data that is not used anywhere.'
# When using the methods "add_concurrent_index" or "add_column_with_default"
# you must disable the use of transactions as these methods can not run in an
# existing transaction. When using "add_concurrent_index" make sure that this
# method is the _only_ method called in the migration, any other changes
# should go in a separate migration. This ensures that upon failure _only_ the
# index creation fails and can be retried or reverted easily.
#
# To disable transactions uncomment the following line and remove these
# comments:
# disable_ddl_transaction!
def change
remove_column :spam_logs, :project_id, :integer
end
end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddSubmittedAsHamToSpamLogs < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
# When a migration requires downtime you **must** uncomment the following
# constant and define a short and easy to understand explanation as to why the
# migration requires downtime.
# DOWNTIME_REASON = ''
disable_ddl_transaction!
def change
add_column_with_default :spam_logs, :submitted_as_ham, :boolean, default: false
end
end
......@@ -926,12 +926,12 @@ ActiveRecord::Schema.define(version: 20160810142633) do
t.string "source_ip"
t.string "user_agent"
t.boolean "via_api"
t.integer "project_id"
t.string "noteable_type"
t.string "title"
t.text "description"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.boolean "submitted_as_ham", default: false, null: false
end
create_table "subscriptions", force: :cascade do |t|
......@@ -999,6 +999,16 @@ ActiveRecord::Schema.define(version: 20160810142633) do
add_index "u2f_registrations", ["key_handle"], name: "index_u2f_registrations_on_key_handle", using: :btree
add_index "u2f_registrations", ["user_id"], name: "index_u2f_registrations_on_user_id", using: :btree
create_table "user_agent_details", force: :cascade do |t|
t.string "user_agent", null: false
t.string "ip_address", null: false
t.integer "subject_id", null: false
t.string "subject_type", null: false
t.boolean "submitted", default: false, null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
end
create_table "users", force: :cascade do |t|
t.string "email", default: "", null: false
t.string "encrypted_password", default: "", null: false
......
......@@ -33,3 +33,26 @@ To use Akismet:
7. Save the configuration.
![Screenshot of Akismet settings](img/akismet_settings.png)
## Training
> *Note:* Training the Akismet filter is only available in 8.11 and above.
As a way to better recognize between spam and ham, you can train the Akismet
filter whenever there is a false positive or false negative.
When an entry is recognized as spam, it is rejected and added to the Spam Logs.
From here you can review if they are really spam. If one of them is not really
spam, you can use the `Submit as ham` button to tell Akismet that it falsely
recognized an entry as spam.
![Screenshot of Spam Logs](img/spam_log.png)
If an entry that is actually spam was not recognized as such, you will be able
to also submit this to Akismet. The `Submit as spam` button will only appear
to admin users.
![Screenshot of Issue](img/submit_issue.png)
Training Akismet will help it to recognize spam more accurately in the future.
......@@ -106,16 +106,6 @@ Feature: Project Merge Requests
And I sort the list by "Least popular"
Then The list should be sorted by "Least popular"
@javascript
Scenario: I comment on a merge request diff
Given project "Shop" have "Bug NS-05" open merge request with diffs inside
And I visit merge request page "Bug NS-05"
And I click on the Changes tab
And I leave a comment like "Line is wrong" on diff
And I switch to the merge request's comments tab
Then I should see a discussion has started on diff
And I should see a badge of "1" next to the discussion link
@javascript
Scenario: I see a new comment on merge request diff from another user in the discussion tab
Given project "Shop" have "Bug NS-05" open merge request with diffs inside
......
......@@ -3,8 +3,6 @@ module API
class Issues < Grape::API
before { authenticate! }
helpers ::Gitlab::AkismetHelper
helpers do
def filter_issues_state(issues, state)
case state
......
module Gitlab
module AkismetHelper
def akismet_enabled?
current_application_settings.akismet_enabled
end
def akismet_client
@akismet_client ||= ::Akismet::Client.new(current_application_settings.akismet_api_key,
Gitlab.config.gitlab.url)
end
def client_ip(env)
env['action_dispatch.remote_ip'].to_s
end
def user_agent(env)
env['HTTP_USER_AGENT']
end
def check_for_spam?(project)
akismet_enabled? && project.public?
end
def is_spam?(environment, user, text)
client = akismet_client
ip_address = client_ip(environment)
user_agent = user_agent(environment)
params = {
type: 'comment',
text: text,
created_at: DateTime.now,
author: user.name,
author_email: user.email,
referrer: environment['HTTP_REFERER'],
}
begin
is_spam, is_blatant = client.check(ip_address, user_agent, params)
is_spam || is_blatant
rescue => e
Rails.logger.error("Unable to connect to Akismet: #{e}, skipping check")
false
end
end
end
end
......@@ -7,13 +7,14 @@ describe Admin::GroupsController do
before do
sign_in(admin)
Sidekiq::Testing.fake!
end
describe 'DELETE #destroy' do
it 'schedules a group destroy' do
Sidekiq::Testing.fake! do
expect { delete :destroy, id: project.group.path }.to change(GroupDestroyWorker.jobs, :size).by(1)
end
end
it 'redirects to the admin group path' do
delete :destroy, id: project.group.path
......
......@@ -34,4 +34,16 @@ describe Admin::SpamLogsController do
expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound)
end
end
describe '#mark_as_ham' do
before do
allow_any_instance_of(AkismetService).to receive(:submit_ham).and_return(true)
end
it 'submits the log as ham' do
post :mark_as_ham, id: first_spam.id
expect(response).to have_http_status(302)
expect(SpamLog.find(first_spam.id).submitted_as_ham).to be_truthy
end
end
end
......@@ -89,13 +89,14 @@ describe GroupsController do
context 'as the group owner' do
before do
Sidekiq::Testing.fake!
sign_in(user)
end
it 'schedules a group destroy' do
Sidekiq::Testing.fake! do
expect { delete :destroy, id: group.path }.to change(GroupDestroyWorker.jobs, :size).by(1)
end
end
it 'redirects to the root path' do
delete :destroy, id: group.path
......
......@@ -274,8 +274,8 @@ describe Projects::IssuesController do
describe 'POST #create' do
context 'Akismet is enabled' do
before do
allow_any_instance_of(Gitlab::AkismetHelper).to receive(:check_for_spam?).and_return(true)
allow_any_instance_of(Gitlab::AkismetHelper).to receive(:is_spam?).and_return(true)
allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true)
allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true)
end
def post_spam_issue
......@@ -300,6 +300,52 @@ describe Projects::IssuesController do
expect(spam_logs[0].title).to eq('Spam Title')
end
end
context 'user agent details are saved' do
before do
request.env['action_dispatch.remote_ip'] = '127.0.0.1'
end
def post_new_issue
sign_in(user)
project = create(:empty_project, :public)
post :create, {
namespace_id: project.namespace.to_param,
project_id: project.to_param,
issue: { title: 'Title', description: 'Description' }
}
end
it 'creates a user agent detail' do
expect{ post_new_issue }.to change(UserAgentDetail, :count).by(1)
end
end
end
describe 'POST #mark_as_spam' do
context 'properly submits to Akismet' do
before do
allow_any_instance_of(AkismetService).to receive_messages(submit_spam: true)
allow_any_instance_of(ApplicationSetting).to receive_messages(akismet_enabled: true)
end
def post_spam
admin = create(:admin)
create(:user_agent_detail, subject: issue)
project.team << [admin, :master]
sign_in(admin)
post :mark_as_spam, {
namespace_id: project.namespace.path,
project_id: project.path,
id: issue.iid
}
end
it 'updates issue' do
post_spam
expect(issue.submittable_as_spam?).to be_falsey
end
end
end
describe "DELETE #destroy" do
......
FactoryGirl.define do
factory :user_agent_detail do
ip_address '127.0.0.1'
user_agent 'AppleWebKit/537.36'
association :subject, factory: :issue
end
end
require 'spec_helper'
feature 'Diff notes', js: true, feature: true do
include WaitForAjax
before do
login_as :admin
@merge_request = create(:merge_request)
@project = @merge_request.source_project
end
context 'merge request diffs' do
let(:comment_button_class) { '.add-diff-note' }
let(:notes_holder_input_class) { 'js-temp-notes-holder' }
let(:notes_holder_input_xpath) { './following-sibling::*[contains(concat(" ", @class, " "), " notes_holder ")]' }
let(:test_note_comment) { 'this is a test note!' }
context 'when hovering over the parallel view diff file' do
before(:each) do
visit diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)
click_link 'Side-by-side'
end
context 'with an old line on the left and no line on the right' do
it 'should allow commenting on the left side' do
should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_23_22"]').find(:xpath, '..'), 'left')
end
it 'should not allow commenting on the right side' do
should_not_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_23_22"]').find(:xpath, '..'), 'right')
end
end
context 'with no line on the left and a new line on the right' do
it 'should not allow commenting on the left side' do
should_not_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15"]').find(:xpath, '..'), 'left')
end
it 'should allow commenting on the right side' do
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15"]').find(:xpath, '..'), 'right')
end
end
context 'with an old line on the left and a new line on the right' do
it 'should allow commenting on the left side' do
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"]').find(:xpath, '..'), 'left')
end
it 'should allow commenting on the right side' do
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"]').find(:xpath, '..'), 'right')
end
end
context 'with an unchanged line on the left and an unchanged line on the right' do
it 'should allow commenting on the left side' do
should_allow_commenting(first('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]').find(:xpath, '..'), 'left')
end
it 'should allow commenting on the right side' do
should_allow_commenting(first('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]').find(:xpath, '..'), 'right')
end
end
context 'with a match line' do
it 'should not allow commenting on the left side' do
should_not_allow_commenting(first('.match').find(:xpath, '..'), 'left')
end
it 'should not allow commenting on the right side' do
should_not_allow_commenting(first('.match').find(:xpath, '..'), 'right')
end
end
end
context 'when hovering over the inline view diff file' do
before do
visit diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)
click_link 'Inline'
end
context 'with a new line' do
it 'should allow commenting' do
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]'))
end
end
context 'with an old line' do
it 'should allow commenting' do
should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'))
end
end
context 'with an unchanged line' do
it 'should allow commenting' do
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]'))
end
end
context 'with a match line' do
it 'should not allow commenting' do
should_not_allow_commenting(first('.match'))
end
end
end
def should_allow_commenting(line_holder, diff_side = nil)
line = get_line_components(line_holder, diff_side)
line[:content].hover
expect(line[:num]).to have_css comment_button_class
comment_on_line(line_holder, line)
wait_for_ajax
assert_comment_persistence(line_holder)
end
def should_not_allow_commenting(line_holder, diff_side = nil)
line = get_line_components(line_holder, diff_side)
line[:content].hover
expect(line[:num]).not_to have_css comment_button_class
end
def get_line_components(line_holder, diff_side = nil)
if diff_side.nil?
get_inline_line_components(line_holder)
else
get_parallel_line_components(line_holder, diff_side)
end
end
def get_inline_line_components(line_holder)
{ content: line_holder.first('.line_content'), num: line_holder.first('.diff-line-num') }
end
def get_parallel_line_components(line_holder, diff_side = nil)
side_index = diff_side == 'left' ? 0 : 1
{ content: line_holder.all('.line_content')[side_index], num: line_holder.all('.diff-line-num')[side_index] }
end
def comment_on_line(line_holder, line)
line[:num].find(comment_button_class).trigger 'click'
expect(line_holder).to have_xpath notes_holder_input_xpath
notes_holder_input = line_holder.find(:xpath, notes_holder_input_xpath)
expect(notes_holder_input[:class]).to include(notes_holder_input_class)
notes_holder_input.fill_in 'note[note]', with: test_note_comment
click_button 'Comment'
end
def assert_comment_persistence(line_holder)
expect(line_holder).to have_xpath notes_holder_input_xpath
notes_holder_saved = line_holder.find(:xpath, notes_holder_input_xpath)
expect(notes_holder_saved[:class]).not_to include(notes_holder_input_class)
expect(notes_holder_saved).to have_content test_note_comment
end
end
end
require 'spec_helper'
feature 'User wants to edit a file', feature: true do
include WaitForAjax
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:commit_params) do
{
source_branch: project.default_branch,
target_branch: project.default_branch,
commit_message: "Committing First Update",
file_path: ".gitignore",
file_content: "First Update",
last_commit_sha: Gitlab::Git::Commit.last_for_path(project.repository, project.default_branch,
".gitignore").sha
}
end
background do
project.team << [user, :master]
login_as user
visit namespace_project_edit_blob_path(project.namespace, project,
File.join(project.default_branch, '.gitignore'))
end
scenario 'file has been updated since the user opened the edit page' do
Files::UpdateService.new(project, user, commit_params).execute
click_button 'Commit Changes'
expect(page).to have_content 'Someone edited the file the same time you did.'
end
end
require 'spec_helper'
describe Gitlab::AkismetHelper, type: :helper do
let(:project) { create(:project, :public) }
let(:user) { create(:user) }
before do
allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url))
allow_any_instance_of(ApplicationSetting).to receive(:akismet_enabled).and_return(true)
allow_any_instance_of(ApplicationSetting).to receive(:akismet_api_key).and_return('12345')
end
describe '#check_for_spam?' do
it 'returns true for public project' do
expect(helper.check_for_spam?(project)).to eq(true)
end
it 'returns false for private project' do
project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PRIVATE)
expect(helper.check_for_spam?(project)).to eq(false)
end
end
describe '#is_spam?' do
it 'returns true for spam' do
environment = {
'action_dispatch.remote_ip' => '127.0.0.1',
'HTTP_USER_AGENT' => 'Test User Agent'
}
allow_any_instance_of(::Akismet::Client).to receive(:check).and_return([true, true])
expect(helper.is_spam?(environment, user, 'Is this spam?')).to eq(true)
end
end
end
require 'spec_helper'
describe Issue, 'Spammable' do
let(:issue) { create(:issue, description: 'Test Desc.') }
describe 'Associations' do
it { is_expected.to have_one(:user_agent_detail).dependent(:destroy) }
end
describe 'ClassMethods' do
it 'should return correct attr_spammable' do
expect(issue.spammable_text).to eq("#{issue.title}\n#{issue.description}")
end
end
describe 'InstanceMethods' do
it 'should be invalid if spam' do
issue = build(:issue, spam: true)
expect(issue.valid?).to be_falsey
end
describe '#check_for_spam?' do
it 'returns true for public project' do
issue.project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC)
expect(issue.check_for_spam?).to eq(true)
end
it 'returns false for other visibility levels' do
expect(issue.check_for_spam?).to eq(false)
end
end
end
end
......@@ -52,19 +52,20 @@ describe IrkerService, models: true do
let(:colorize_messages) { '1' }
before do
@irker_server = TCPServer.new 'localhost', 0
allow(irker).to receive_messages(
active: true,
project: project,
project_id: project.id,
service_hook: true,
server_host: 'localhost',
server_port: 6659,
server_host: @irker_server.addr[2],
server_port: @irker_server.addr[1],
default_irc_uri: 'irc://chat.freenode.net/',
recipients: recipients,
colorize_messages: colorize_messages)
irker.valid?
@irker_server = TCPServer.new 'localhost', 6659
end
after do
......
require 'rails_helper'
describe UserAgentDetail, type: :model do
describe '.submittable?' do
it 'is submittable when not already submitted' do
detail = build(:user_agent_detail)
expect(detail.submittable?).to be_truthy
end
it 'is not submittable when already submitted' do
detail = build(:user_agent_detail, submitted: true)
expect(detail.submittable?).to be_falsey
end
end
describe '.valid?' do
it 'is valid with a subject' do
detail = build(:user_agent_detail)
expect(detail).to be_valid
end
it 'is invalid without a subject' do
detail = build(:user_agent_detail, subject: nil)
expect(detail).not_to be_valid
end
end
end
......@@ -531,8 +531,8 @@ describe API::API, api: true do
describe 'POST /projects/:id/issues with spam filtering' do
before do
allow_any_instance_of(Gitlab::AkismetHelper).to receive(:check_for_spam?).and_return(true)
allow_any_instance_of(Gitlab::AkismetHelper).to receive(:is_spam?).and_return(true)
allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true)
allow_any_instance_of(AkismetService).to receive_messages(is_spam?: true)
end
let(:params) do
......@@ -554,7 +554,6 @@ describe API::API, api: true do
expect(spam_logs[0].description).to eq('content here')
expect(spam_logs[0].user).to eq(user)
expect(spam_logs[0].noteable_type).to eq('Issue')
expect(spam_logs[0].project_id).to eq(project.id)
end
end
......
require "spec_helper"
describe Files::UpdateService do
subject { described_class.new(project, user, commit_params) }
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:file_path) { 'files/ruby/popen.rb' }
let(:new_contents) { "New Content" }
let(:commit_params) do
{
file_path: file_path,
commit_message: "Update File",
file_content: new_contents,
file_content_encoding: "text",
last_commit_sha: last_commit_sha,
source_project: project,
source_branch: project.default_branch,
target_branch: project.default_branch,
}
end
before do
project.team << [user, :master]
end
describe "#execute" do
context "when the file's last commit sha does not match the supplied last_commit_sha" do
let(:last_commit_sha) { "foo" }
it "returns a hash with the correct error message and a :error status " do
expect { subject.execute }.
to raise_error(Files::UpdateService::FileChangedError,
"You are attempting to update a file that has changed since you started editing it.")
end
end
context "when the file's last commit sha does match the supplied last_commit_sha" do
let(:last_commit_sha) { Gitlab::Git::Commit.last_for_path(project.repository, project.default_branch, file_path).sha }
it "returns a hash with the :success status " do
results = subject.execute
expect(results).to match({ status: :success })
end
it "updates the file with the new contents" do
subject.execute
results = project.repository.blob_at_branch(project.default_branch, file_path)
expect(results.data).to eq(new_contents)
end
end
context "when the last_commit_sha is not supplied" do
let(:commit_params) do
{
file_path: file_path,
commit_message: "Update File",
file_content: new_contents,
file_content_encoding: "text",
source_project: project,
source_branch: project.default_branch,
target_branch: project.default_branch,
}
end
it "returns a hash with the :success status " do
results = subject.execute
expect(results).to match({ status: :success })
end
it "updates the file with the new contents" do
subject.execute
results = project.repository.blob_at_branch(project.default_branch, file_path)
expect(results.data).to eq(new_contents)
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment