From 9d51421346178c9189ffb47189f51d573ab42822 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Fri, 19 Aug 2016 18:33:46 -0500
Subject: [PATCH] Use separate email-friendly token for incoming email and let
incoming email token be reset
---
app/controllers/profiles_controller.rb | 10 +++++-
app/models/concerns/token_authenticatable.rb | 10 ++++--
app/models/project.rb | 11 +++---
app/models/user.rb | 12 ++++++-
app/views/profiles/accounts/show.html.haml | 35 +++++++++++--------
config/routes/profile.rb | 1 +
...32256_add_incoming_email_token_to_users.rb | 16 +++++++++
db/schema.rb | 4 ++-
.../email/handler/create_issue_handler.rb | 8 ++---
spec/features/projects/issues/issues_spec.rb | 0
...ken.eml => wrong_incoming_email_token.eml} | 0
.../handler/create_issue_handler_spec.rb | 6 ++--
12 files changed, 79 insertions(+), 34 deletions(-)
create mode 100644 db/migrate/20160819232256_add_incoming_email_token_to_users.rb
create mode 100644 spec/features/projects/issues/issues_spec.rb
rename spec/fixtures/emails/{wrong_authentication_token.eml => wrong_incoming_email_token.eml} (100%)
diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb
index f71e0a1302b..e4865642cd3 100644
--- a/app/controllers/profiles_controller.rb
+++ b/app/controllers/profiles_controller.rb
@@ -26,7 +26,15 @@ class ProfilesController < Profiles::ApplicationController
def reset_private_token
if current_user.reset_authentication_token!
- flash[:notice] = "Token was successfully updated"
+ flash[:notice] = "Private token was successfully updated"
+ end
+
+ redirect_to profile_account_path
+ end
+
+ def reset_incoming_email_token
+ if current_user.reset_incoming_email_token!
+ flash[:notice] = "Incoming email token was successfully updated"
end
redirect_to profile_account_path
diff --git a/app/models/concerns/token_authenticatable.rb b/app/models/concerns/token_authenticatable.rb
index 24c7b26d223..04d30f46210 100644
--- a/app/models/concerns/token_authenticatable.rb
+++ b/app/models/concerns/token_authenticatable.rb
@@ -4,17 +4,21 @@ module TokenAuthenticatable
private
def write_new_token(token_field)
- new_token = generate_token(token_field)
+ new_token = generate_available_token(token_field)
write_attribute(token_field, new_token)
end
- def generate_token(token_field)
+ def generate_available_token(token_field)
loop do
- token = Devise.friendly_token
+ token = generate_token(token_field)
break token unless self.class.unscoped.find_by(token_field => token)
end
end
+ def generate_token(token_field)
+ Devise.friendly_token
+ end
+
class_methods do
def authentication_token_fields
@token_fields || []
diff --git a/app/models/project.rb b/app/models/project.rb
index 686d285410b..56b84b0aebb 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -624,13 +624,12 @@ class Project < ActiveRecord::Base
end
def new_issue_address(author)
- # This feature is disabled for the time being.
- return nil
+ return unless Gitlab::IncomingEmail.enabled? && author
- if Gitlab::IncomingEmail.enabled? && author # rubocop:disable Lint/UnreachableCode
- Gitlab::IncomingEmail.reply_address(
- "#{path_with_namespace}+#{author.authentication_token}")
- end
+ author.ensure_incoming_email_token!
+
+ Gitlab::IncomingEmail.reply_address(
+ "#{path_with_namespace}+#{author.incoming_email_token}")
end
def build_commit_note(commit)
diff --git a/app/models/user.rb b/app/models/user.rb
index 65e96ee6b2e..9a3619b0bc3 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -13,6 +13,7 @@ class User < ActiveRecord::Base
DEFAULT_NOTIFICATION_LEVEL = :participating
add_authentication_token_field :authentication_token
+ add_authentication_token_field :incoming_email_token
default_value_for :admin, false
default_value_for(:external) { current_application_settings.user_default_external }
@@ -119,7 +120,7 @@ class User < ActiveRecord::Base
before_validation :set_public_email, if: ->(user) { user.public_email_changed? }
after_update :update_emails_with_primary_email, if: ->(user) { user.email_changed? }
- before_save :ensure_authentication_token
+ before_save :ensure_authentication_token, :ensure_incoming_email_token
before_save :ensure_external_user_rights
after_save :ensure_namespace_correct
after_initialize :set_projects_limit
@@ -946,4 +947,13 @@ class User < ActiveRecord::Base
signup_domain =~ regexp
end
end
+
+ def generate_token(token_field)
+ if token_field == :incoming_email_token
+ # Needs to be all lowercase and alphanumeric because it's gonna be used in an email address.
+ SecureRandom.hex
+ else
+ super
+ end
+ end
end
diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml
index e2e974ba072..2c256b1b233 100644
--- a/app/views/profiles/accounts/show.html.haml
+++ b/app/views/profiles/accounts/show.html.haml
@@ -8,24 +8,29 @@
.row.prepend-top-default
.col-lg-3.profile-settings-sidebar
%h4.prepend-top-0
- Private Token
+ Private Tokens
%p
- Your private token is used to access application resources without authentication.
+ Your private token is used to access the API and Atom feeds without
+ username/password authentication.
+ %p
+ Your incoming email token is used to create new issues by email, and is
+ included in your project-specific email addresses.
.col-lg-9
- = form_for @user, url: reset_private_token_profile_path, method: :put, html: { class: "private-token" } do |f|
- %p.cgray
- - if current_user.private_token
- = label_tag "token", "Private token", class: "label-light"
- = text_field_tag "token", current_user.private_token, class: "form-control"
- - else
- %span You don`t have one yet. Click generate to fix it.
+ %p.cgray
+ - if current_user.private_token
+ = label_tag "token", "Private token", class: "label-light"
+ = text_field_tag "token", current_user.private_token, class: "form-control"
+ - else
+ %span You don`t have one yet. Click generate to fix it.
%p.help-block
- It can be used for atom feeds or the API. Keep it secret!
- .prepend-top-default
- - if current_user.private_token
- = f.submit 'Reset private token', data: { confirm: "Are you sure?" }, class: "btn btn-default"
- - else
- = f.submit 'Generate', class: "btn btn-default"
+ Keep this token secret, anyone with access to it can interact with the GitLab API as if they were you.
+ .prepend-top-default
+ - if current_user.private_token
+ = link_to 'Reset private token', reset_private_token_profile_path, method: :put, data: { confirm: "Are you sure?" }, class: "btn btn-default"
+ - else
+ = f.submit 'Generate', class: "btn btn-default"
+ = link_to 'Reset incoming email token', reset_incoming_email_token_profile_path, method: :put, data: { confirm: "Are you sure?" }, class: "btn btn-default"
+
%hr
.row.prepend-top-default
.col-lg-3.profile-settings-sidebar
diff --git a/config/routes/profile.rb b/config/routes/profile.rb
index 4cb68c9b34a..52b9a565db8 100644
--- a/config/routes/profile.rb
+++ b/config/routes/profile.rb
@@ -4,6 +4,7 @@ resource :profile, only: [:show, :update] do
get :applications, to: 'oauth/applications#index'
put :reset_private_token
+ put :reset_incoming_email_token
put :update_username
end
diff --git a/db/migrate/20160819232256_add_incoming_email_token_to_users.rb b/db/migrate/20160819232256_add_incoming_email_token_to_users.rb
new file mode 100644
index 00000000000..f2cf956adc9
--- /dev/null
+++ b/db/migrate/20160819232256_add_incoming_email_token_to_users.rb
@@ -0,0 +1,16 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddIncomingEmailTokenToUsers < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def change
+ add_column :users, :incoming_email_token, :string
+ add_concurrent_index :users, :incoming_email_token
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 5476b0c93e5..fd82fb326a6 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20161103171205) do
+ActiveRecord::Schema.define(version: 20160819232256) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -1176,6 +1176,7 @@ ActiveRecord::Schema.define(version: 20161103171205) do
t.boolean "ldap_email", default: false, null: false
t.boolean "external", default: false
t.string "organization"
+ t.string "incoming_email_token"
end
add_index "users", ["admin"], name: "index_users_on_admin", using: :btree
@@ -1185,6 +1186,7 @@ ActiveRecord::Schema.define(version: 20161103171205) do
add_index "users", ["current_sign_in_at"], name: "index_users_on_current_sign_in_at", using: :btree
add_index "users", ["email"], name: "index_users_on_email", unique: true, using: :btree
add_index "users", ["email"], name: "index_users_on_email_trigram", using: :gin, opclasses: {"email"=>"gin_trgm_ops"}
+ add_index "users", ["incoming_email_token"], name: "index_users_on_incoming_email_token", using: :btree
add_index "users", ["name"], name: "index_users_on_name", using: :btree
add_index "users", ["name"], name: "index_users_on_name_trigram", using: :gin, opclasses: {"name"=>"gin_trgm_ops"}
add_index "users", ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true, using: :btree
diff --git a/lib/gitlab/email/handler/create_issue_handler.rb b/lib/gitlab/email/handler/create_issue_handler.rb
index 4e6566af8ab..9f90a3ec2b2 100644
--- a/lib/gitlab/email/handler/create_issue_handler.rb
+++ b/lib/gitlab/email/handler/create_issue_handler.rb
@@ -5,16 +5,16 @@ module Gitlab
module Email
module Handler
class CreateIssueHandler < BaseHandler
- attr_reader :project_path, :authentication_token
+ attr_reader :project_path, :incoming_email_token
def initialize(mail, mail_key)
super(mail, mail_key)
- @project_path, @authentication_token =
+ @project_path, @incoming_email_token =
mail_key && mail_key.split('+', 2)
end
def can_handle?
- !authentication_token.nil?
+ !incoming_email_token.nil?
end
def execute
@@ -29,7 +29,7 @@ module Gitlab
end
def author
- @author ||= User.find_by(authentication_token: authentication_token)
+ @author ||= User.find_by(incoming_email_token: incoming_email_token)
end
def project
diff --git a/spec/features/projects/issues/issues_spec.rb b/spec/features/projects/issues/issues_spec.rb
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/spec/fixtures/emails/wrong_authentication_token.eml b/spec/fixtures/emails/wrong_incoming_email_token.eml
similarity index 100%
rename from spec/fixtures/emails/wrong_authentication_token.eml
rename to spec/fixtures/emails/wrong_incoming_email_token.eml
diff --git a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb
index a5cc7b02936..939189a3bc0 100644
--- a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb
+++ b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb
@@ -18,7 +18,7 @@ xdescribe Gitlab::Email::Handler::CreateIssueHandler, lib: true do
create(
:user,
email: 'jake@adventuretime.ooo',
- authentication_token: 'auth_token'
+ incoming_email_token: 'auth_token'
)
end
@@ -60,8 +60,8 @@ xdescribe Gitlab::Email::Handler::CreateIssueHandler, lib: true do
end
end
- context "when we can't find the authentication_token" do
- let(:email_raw) { fixture_file("emails/wrong_authentication_token.eml") }
+ context "when we can't find the incoming_email_token" do
+ let(:email_raw) { fixture_file("emails/wrong_incoming_email_token.eml") }
it "raises an UserNotFoundError" do
expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotFoundError)
--
GitLab