From ed1d4fa477789659f9343593bf06d50e70750561 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Fri, 7 Aug 2015 00:06:20 -0700 Subject: [PATCH] Remove user OAuth tokens stored in database for Bitbucket, GitHub, and GitLab and request them each session. Pass these tokens to the project import data. This prevents the need to encrypt these tokens and clear them in case they expire or get revoked. For example, if you deleted and re-created OAuth2 keys for Bitbucket, you would get an Error 500 with no way to recover: ``` Started GET "/import/bitbucket/status" for x.x.x.x at 2015-08-07 05:24:10 +0000 Processing by Import::BitbucketController#status as HTML Completed 500 Internal Server Error in 607ms (ActiveRecord: 2.3ms) NameError (uninitialized constant Import::BitbucketController::Unauthorized): app/controllers/import/bitbucket_controller.rb:77:in `rescue in go_to_bitbucket_for_permissions' app/controllers/import/bitbucket_controller.rb:74:in `go_to_bitbucket_for_permissions' app/controllers/import/bitbucket_controller.rb:86:in `bitbucket_unauthorized' ``` Closes #1871 --- CHANGELOG | 1 + .../import/bitbucket_controller.rb | 23 ++++++++++----- app/controllers/import/github_controller.rb | 16 +++++++---- app/controllers/import/gitlab_controller.rb | 16 +++++++---- app/workers/repository_import_worker.rb | 3 +- ...14065925_remove_oauth_tokens_from_users.rb | 8 ++++++ db/schema.rb | 4 --- lib/gitlab/bitbucket_import/importer.rb | 15 ++++++---- lib/gitlab/bitbucket_import/key_adder.rb | 7 +++-- lib/gitlab/bitbucket_import/key_deleter.rb | 7 +++-- .../bitbucket_import/project_creator.rb | 12 +++++--- lib/gitlab/github_import/importer.rb | 4 ++- lib/gitlab/github_import/project_creator.rb | 13 ++++++--- lib/gitlab/gitlab_import/importer.rb | 14 ++++++---- lib/gitlab/gitlab_import/project_creator.rb | 12 +++++--- .../import/bitbucket_controller_spec.rb | 28 ++++++++++++------- .../import/github_controller_spec.rb | 21 ++++++++++---- .../import/gitlab_controller_spec.rb | 22 ++++++++++----- .../bitbucket_import/project_creator_spec.rb | 7 +++-- .../github_import/project_creator_spec.rb | 6 ++-- .../gitlab_import/project_creator_spec.rb | 6 ++-- 21 files changed, 162 insertions(+), 83 deletions(-) create mode 100644 db/migrate/20150814065925_remove_oauth_tokens_from_users.rb diff --git a/CHANGELOG b/CHANGELOG index 44ffb56d5fd..7eb6ab81dd9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.0.0 (unreleased) + - Remove user OAuth tokens from the database and request new tokens each session (Stan Hu) - Only show recent push event if the branch still exists or a recent merge request has not been created (Stan Hu) - Remove satellites - Better performance for web editor (switched from satellites to rugged) diff --git a/app/controllers/import/bitbucket_controller.rb b/app/controllers/import/bitbucket_controller.rb index 4e6c0b66634..f84f85a7df8 100644 --- a/app/controllers/import/bitbucket_controller.rb +++ b/app/controllers/import/bitbucket_controller.rb @@ -13,10 +13,9 @@ class Import::BitbucketController < Import::BaseController access_token = client.get_token(request_token, params[:oauth_verifier], callback_import_bitbucket_url) - current_user.bitbucket_access_token = access_token.token - current_user.bitbucket_access_token_secret = access_token.secret + session[:bitbucket_access_token] = access_token.token + session[:bitbucket_access_token_secret] = access_token.secret - current_user.save redirect_to status_import_bitbucket_url end @@ -46,19 +45,20 @@ class Import::BitbucketController < Import::BaseController namespace = get_or_create_namespace || (render and return) - unless Gitlab::BitbucketImport::KeyAdder.new(repo, current_user).execute + unless Gitlab::BitbucketImport::KeyAdder.new(repo, current_user, access_params).execute @access_denied = true render return end - @project = Gitlab::BitbucketImport::ProjectCreator.new(repo, namespace, current_user).execute + @project = Gitlab::BitbucketImport::ProjectCreator.new(repo, namespace, current_user, access_params).execute end private def client - @client ||= Gitlab::BitbucketImport::Client.new(current_user.bitbucket_access_token, current_user.bitbucket_access_token_secret) + @client ||= Gitlab::BitbucketImport::Client.new(session[:bitbucket_access_token], + session[:bitbucket_access_token_secret]) end def verify_bitbucket_import_enabled @@ -66,7 +66,7 @@ class Import::BitbucketController < Import::BaseController end def bitbucket_auth - if current_user.bitbucket_access_token.blank? + if session[:bitbucket_access_token].blank? go_to_bitbucket_for_permissions end end @@ -81,4 +81,13 @@ class Import::BitbucketController < Import::BaseController def bitbucket_unauthorized go_to_bitbucket_for_permissions end + + private + + def access_params + { + bitbucket_access_token: session[:bitbucket_access_token], + bitbucket_access_token_secret: session[:bitbucket_access_token_secret] + } + end end diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb index b9f99c1b88a..f21fbd9ecca 100644 --- a/app/controllers/import/github_controller.rb +++ b/app/controllers/import/github_controller.rb @@ -5,9 +5,7 @@ class Import::GithubController < Import::BaseController rescue_from Octokit::Unauthorized, with: :github_unauthorized def callback - token = client.get_token(params[:code]) - current_user.github_access_token = token - current_user.save + session[:github_access_token] = client.get_token(params[:code]) redirect_to status_import_github_url end @@ -39,13 +37,13 @@ class Import::GithubController < Import::BaseController namespace = get_or_create_namespace || (render and return) - @project = Gitlab::GithubImport::ProjectCreator.new(repo, namespace, current_user).execute + @project = Gitlab::GithubImport::ProjectCreator.new(repo, namespace, current_user, access_params).execute end private def client - @client ||= Gitlab::GithubImport::Client.new(current_user.github_access_token) + @client ||= Gitlab::GithubImport::Client.new(session[:github_access_token]) end def verify_github_import_enabled @@ -53,7 +51,7 @@ class Import::GithubController < Import::BaseController end def github_auth - if current_user.github_access_token.blank? + if session[:github_access_token].blank? go_to_github_for_permissions end end @@ -65,4 +63,10 @@ class Import::GithubController < Import::BaseController def github_unauthorized go_to_github_for_permissions end + + private + + def access_params + { github_access_token: session[:github_access_token] } + end end diff --git a/app/controllers/import/gitlab_controller.rb b/app/controllers/import/gitlab_controller.rb index 1b8962d8924..27af19f5f61 100644 --- a/app/controllers/import/gitlab_controller.rb +++ b/app/controllers/import/gitlab_controller.rb @@ -5,9 +5,7 @@ class Import::GitlabController < Import::BaseController rescue_from OAuth2::Error, with: :gitlab_unauthorized def callback - token = client.get_token(params[:code], callback_import_gitlab_url) - current_user.gitlab_access_token = token - current_user.save + session[:gitlab_access_token] = client.get_token(params[:code], callback_import_gitlab_url) redirect_to status_import_gitlab_url end @@ -36,13 +34,13 @@ class Import::GitlabController < Import::BaseController namespace = get_or_create_namespace || (render and return) - @project = Gitlab::GitlabImport::ProjectCreator.new(repo, namespace, current_user).execute + @project = Gitlab::GitlabImport::ProjectCreator.new(repo, namespace, current_user, access_params).execute end private def client - @client ||= Gitlab::GitlabImport::Client.new(current_user.gitlab_access_token) + @client ||= Gitlab::GitlabImport::Client.new(session[:gitlab_access_token]) end def verify_gitlab_import_enabled @@ -50,7 +48,7 @@ class Import::GitlabController < Import::BaseController end def gitlab_auth - if current_user.gitlab_access_token.blank? + if session[:gitlab_access_token].blank? go_to_gitlab_for_permissions end end @@ -62,4 +60,10 @@ class Import::GitlabController < Import::BaseController def gitlab_unauthorized go_to_gitlab_for_permissions end + + private + + def access_params + { gitlab_access_token: session[:gitlab_access_token] } + end end diff --git a/app/workers/repository_import_worker.rb b/app/workers/repository_import_worker.rb index b546f8777e1..f2ba2e15e7b 100644 --- a/app/workers/repository_import_worker.rb +++ b/app/workers/repository_import_worker.rb @@ -25,9 +25,10 @@ class RepositoryImportWorker end return project.import_fail unless data_import_result + Gitlab::BitbucketImport::KeyDeleter.new(project).execute if project.import_type == 'bitbucket' + project.import_finish project.save ProjectCacheWorker.perform_async(project.id) - Gitlab::BitbucketImport::KeyDeleter.new(project).execute if project.import_type == 'bitbucket' end end diff --git a/db/migrate/20150814065925_remove_oauth_tokens_from_users.rb b/db/migrate/20150814065925_remove_oauth_tokens_from_users.rb new file mode 100644 index 00000000000..de2078a9268 --- /dev/null +++ b/db/migrate/20150814065925_remove_oauth_tokens_from_users.rb @@ -0,0 +1,8 @@ +class RemoveOauthTokensFromUsers < ActiveRecord::Migration + def change + remove_column :users, :github_access_token, :string + remove_column :users, :gitlab_access_token, :string + remove_column :users, :bitbucket_access_token, :string + remove_column :users, :bitbucket_access_token_secret, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 2b9a3e7f011..108c48bf321 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -526,13 +526,9 @@ ActiveRecord::Schema.define(version: 20150818213832) do t.string "unconfirmed_email" t.boolean "hide_no_ssh_key", default: false t.string "website_url", default: "", null: false - t.string "github_access_token" - t.string "gitlab_access_token" t.string "notification_email" t.boolean "hide_no_password", default: false t.boolean "password_automatically_set", default: false - t.string "bitbucket_access_token" - t.string "bitbucket_access_token_secret" t.string "location" t.string "encrypted_otp_secret" t.string "encrypted_otp_secret_iv" diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb index 42c93707caa..d8a7d29f1bf 100644 --- a/lib/gitlab/bitbucket_import/importer.rb +++ b/lib/gitlab/bitbucket_import/importer.rb @@ -5,7 +5,10 @@ module Gitlab def initialize(project) @project = project - @client = Client.new(project.creator.bitbucket_access_token, project.creator.bitbucket_access_token_secret) + import_data = project.import_data.try(:data) + bb_session = import_data["bb_session"] if import_data + @client = Client.new(bb_session["bitbucket_access_token"], + bb_session["bitbucket_access_token_secret"]) @formatter = Gitlab::ImportFormatter.new end @@ -16,12 +19,12 @@ module Gitlab #Issues && Comments issues = client.issues(project_identifier) - + issues["issues"].each do |issue| body = @formatter.author_line(issue["reported_by"]["username"], issue["content"]) - + comments = client.issue_comments(project_identifier, issue["local_id"]) - + if comments.any? body += @formatter.comments_header end @@ -31,13 +34,13 @@ module Gitlab end project.issues.create!( - description: body, + description: body, title: issue["title"], state: %w(resolved invalid duplicate wontfix).include?(issue["status"]) ? 'closed' : 'opened', author_id: gl_user_id(project, issue["reported_by"]["username"]) ) end - + true end diff --git a/lib/gitlab/bitbucket_import/key_adder.rb b/lib/gitlab/bitbucket_import/key_adder.rb index 9931aa7e029..0b63f025d0a 100644 --- a/lib/gitlab/bitbucket_import/key_adder.rb +++ b/lib/gitlab/bitbucket_import/key_adder.rb @@ -3,14 +3,15 @@ module Gitlab class KeyAdder attr_reader :repo, :current_user, :client - def initialize(repo, current_user) + def initialize(repo, current_user, access_params) @repo, @current_user = repo, current_user - @client = Client.new(current_user.bitbucket_access_token, current_user.bitbucket_access_token_secret) + @client = Client.new(access_params[:bitbucket_access_token], + access_params[:bitbucket_access_token_secret]) end def execute return false unless BitbucketImport.public_key.present? - + project_identifier = "#{repo["owner"]}/#{repo["slug"]}" client.add_deploy_key(project_identifier, BitbucketImport.public_key) diff --git a/lib/gitlab/bitbucket_import/key_deleter.rb b/lib/gitlab/bitbucket_import/key_deleter.rb index 1a24a86fc37..f4dd393ad29 100644 --- a/lib/gitlab/bitbucket_import/key_deleter.rb +++ b/lib/gitlab/bitbucket_import/key_deleter.rb @@ -6,12 +6,15 @@ module Gitlab def initialize(project) @project = project @current_user = project.creator - @client = Client.new(current_user.bitbucket_access_token, current_user.bitbucket_access_token_secret) + import_data = project.import_data.try(:data) + bb_session = import_data["bb_session"] if import_data + @client = Client.new(bb_session["bitbucket_access_token"], + bb_session["bitbucket_access_token_secret"]) end def execute return false unless BitbucketImport.public_key.present? - + client.delete_deploy_key(project.import_source, BitbucketImport.public_key) true diff --git a/lib/gitlab/bitbucket_import/project_creator.rb b/lib/gitlab/bitbucket_import/project_creator.rb index 54420e62c90..35e34d033e0 100644 --- a/lib/gitlab/bitbucket_import/project_creator.rb +++ b/lib/gitlab/bitbucket_import/project_creator.rb @@ -1,16 +1,17 @@ module Gitlab module BitbucketImport class ProjectCreator - attr_reader :repo, :namespace, :current_user + attr_reader :repo, :namespace, :current_user, :session_data - def initialize(repo, namespace, current_user) + def initialize(repo, namespace, current_user, session_data) @repo = repo @namespace = namespace @current_user = current_user + @session_data = session_data end def execute - ::Projects::CreateService.new(current_user, + project = ::Projects::CreateService.new(current_user, name: repo["name"], path: repo["slug"], description: repo["description"], @@ -18,8 +19,11 @@ module Gitlab visibility_level: repo["is_private"] ? Gitlab::VisibilityLevel::PRIVATE : Gitlab::VisibilityLevel::PUBLIC, import_type: "bitbucket", import_source: "#{repo["owner"]}/#{repo["slug"]}", - import_url: "ssh://git@bitbucket.org/#{repo["owner"]}/#{repo["slug"]}.git" + import_url: "ssh://git@bitbucket.org/#{repo["owner"]}/#{repo["slug"]}.git", ).execute + + project.create_import_data(data: { "bb_session" => session_data } ) + project end end end diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index 98039a76dcd..8c106a61735 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -5,7 +5,9 @@ module Gitlab def initialize(project) @project = project - @client = Client.new(project.creator.github_access_token) + import_data = project.import_data.try(:data) + github_session = import_data["github_session"] if import_data + @client = Client.new(github_session["github_access_token"]) @formatter = Gitlab::ImportFormatter.new end diff --git a/lib/gitlab/github_import/project_creator.rb b/lib/gitlab/github_import/project_creator.rb index 2723eec933e..8c27ebd1ce8 100644 --- a/lib/gitlab/github_import/project_creator.rb +++ b/lib/gitlab/github_import/project_creator.rb @@ -1,16 +1,18 @@ module Gitlab module GithubImport class ProjectCreator - attr_reader :repo, :namespace, :current_user + attr_reader :repo, :namespace, :current_user, :session_data - def initialize(repo, namespace, current_user) + def initialize(repo, namespace, current_user, session_data) @repo = repo @namespace = namespace @current_user = current_user + @session_data = session_data end def execute - ::Projects::CreateService.new(current_user, + project = ::Projects::CreateService.new( + current_user, name: repo.name, path: repo.name, description: repo.description, @@ -18,8 +20,11 @@ module Gitlab visibility_level: repo.private ? Gitlab::VisibilityLevel::PRIVATE : Gitlab::VisibilityLevel::PUBLIC, import_type: "github", import_source: repo.full_name, - import_url: repo.clone_url.sub("https://", "https://#{current_user.github_access_token}@") + import_url: repo.clone_url.sub("https://", "https://#{@session_data[:github_access_token]}@") ).execute + + project.create_import_data(data: { "github_session" => session_data } ) + project end end end diff --git a/lib/gitlab/gitlab_import/importer.rb b/lib/gitlab/gitlab_import/importer.rb index c5304a0699b..50594d2b24f 100644 --- a/lib/gitlab/gitlab_import/importer.rb +++ b/lib/gitlab/gitlab_import/importer.rb @@ -5,7 +5,9 @@ module Gitlab def initialize(project) @project = project - @client = Client.new(project.creator.gitlab_access_token) + import_data = project.import_data.try(:data) + gitlab_session = import_data["gitlab_session"] if import_data + @client = Client.new(gitlab_session["gitlab_access_token"]) @formatter = Gitlab::ImportFormatter.new end @@ -14,12 +16,12 @@ module Gitlab #Issues && Comments issues = client.issues(project_identifier) - + issues.each do |issue| body = @formatter.author_line(issue["author"]["name"], issue["description"]) - + comments = client.issue_comments(project_identifier, issue["id"]) - + if comments.any? body += @formatter.comments_header end @@ -29,13 +31,13 @@ module Gitlab end project.issues.create!( - description: body, + description: body, title: issue["title"], state: issue["state"], author_id: gl_user_id(project, issue["author"]["id"]) ) end - + true end diff --git a/lib/gitlab/gitlab_import/project_creator.rb b/lib/gitlab/gitlab_import/project_creator.rb index f0d7141bf56..d9452de6a50 100644 --- a/lib/gitlab/gitlab_import/project_creator.rb +++ b/lib/gitlab/gitlab_import/project_creator.rb @@ -1,16 +1,17 @@ module Gitlab module GitlabImport class ProjectCreator - attr_reader :repo, :namespace, :current_user + attr_reader :repo, :namespace, :current_user, :session_data - def initialize(repo, namespace, current_user) + def initialize(repo, namespace, current_user, session_data) @repo = repo @namespace = namespace @current_user = current_user + @session_data = session_data end def execute - ::Projects::CreateService.new(current_user, + project = ::Projects::CreateService.new(current_user, name: repo["name"], path: repo["path"], description: repo["description"], @@ -18,8 +19,11 @@ module Gitlab visibility_level: repo["visibility_level"], import_type: "gitlab", import_source: repo["path_with_namespace"], - import_url: repo["http_url_to_repo"].sub("://", "://oauth2:#{current_user.gitlab_access_token}@") + import_url: repo["http_url_to_repo"].sub("://", "://oauth2:#{@session_data[:gitlab_access_token]}@") ).execute + + project.create_import_data(data: { "gitlab_session" => session_data } ) + project end end end diff --git a/spec/controllers/import/bitbucket_controller_spec.rb b/spec/controllers/import/bitbucket_controller_spec.rb index 89e595121a7..81c03c9059b 100644 --- a/spec/controllers/import/bitbucket_controller_spec.rb +++ b/spec/controllers/import/bitbucket_controller_spec.rb @@ -4,7 +4,15 @@ require_relative 'import_spec_helper' describe Import::BitbucketController do include ImportSpecHelper - let(:user) { create(:user, bitbucket_access_token: 'asd123', bitbucket_access_token_secret: "sekret") } + let(:user) { create(:user) } + let(:token) { "asdasd12345" } + let(:secret) { "sekrettt" } + let(:access_params) { { bitbucket_access_token: token, bitbucket_access_token_secret: secret } } + + def assign_session_tokens + session[:bitbucket_access_token] = token + session[:bitbucket_access_token_secret] = secret + end before do sign_in(user) @@ -17,8 +25,6 @@ describe Import::BitbucketController do end it "updates access token" do - token = "asdasd12345" - secret = "sekrettt" access_token = double(token: token, secret: secret) allow_any_instance_of(Gitlab::BitbucketImport::Client). to receive(:get_token).and_return(access_token) @@ -26,8 +32,8 @@ describe Import::BitbucketController do get :callback - expect(user.reload.bitbucket_access_token).to eq(token) - expect(user.reload.bitbucket_access_token_secret).to eq(secret) + expect(session[:bitbucket_access_token]).to eq(token) + expect(session[:bitbucket_access_token_secret]).to eq(secret) expect(controller).to redirect_to(status_import_bitbucket_url) end end @@ -35,6 +41,7 @@ describe Import::BitbucketController do describe "GET status" do before do @repo = OpenStruct.new(slug: 'vim', owner: 'asd') + assign_session_tokens end it "assigns variables" do @@ -73,17 +80,18 @@ describe Import::BitbucketController do before do allow(Gitlab::BitbucketImport::KeyAdder). - to receive(:new).with(bitbucket_repo, user). + to receive(:new).with(bitbucket_repo, user, access_params). and_return(double(execute: true)) stub_client(user: bitbucket_user, project: bitbucket_repo) + assign_session_tokens end context "when the repository owner is the Bitbucket user" do context "when the Bitbucket user and GitLab user's usernames match" do it "takes the current user's namespace" do expect(Gitlab::BitbucketImport::ProjectCreator). - to receive(:new).with(bitbucket_repo, user.namespace, user). + to receive(:new).with(bitbucket_repo, user.namespace, user, access_params). and_return(double(execute: true)) post :create, format: :js @@ -95,7 +103,7 @@ describe Import::BitbucketController do it "takes the current user's namespace" do expect(Gitlab::BitbucketImport::ProjectCreator). - to receive(:new).with(bitbucket_repo, user.namespace, user). + to receive(:new).with(bitbucket_repo, user.namespace, user, access_params). and_return(double(execute: true)) post :create, format: :js @@ -116,7 +124,7 @@ describe Import::BitbucketController do context "when the namespace is owned by the GitLab user" do it "takes the existing namespace" do expect(Gitlab::BitbucketImport::ProjectCreator). - to receive(:new).with(bitbucket_repo, existing_namespace, user). + to receive(:new).with(bitbucket_repo, existing_namespace, user, access_params). and_return(double(execute: true)) post :create, format: :js @@ -150,7 +158,7 @@ describe Import::BitbucketController do it "takes the new namespace" do expect(Gitlab::BitbucketImport::ProjectCreator). - to receive(:new).with(bitbucket_repo, an_instance_of(Group), user). + to receive(:new).with(bitbucket_repo, an_instance_of(Group), user, access_params). and_return(double(execute: true)) post :create, format: :js diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index 0bc14059a35..766be578f7f 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -4,7 +4,13 @@ require_relative 'import_spec_helper' describe Import::GithubController do include ImportSpecHelper - let(:user) { create(:user, github_access_token: 'asd123') } + let(:user) { create(:user) } + let(:token) { "asdasd12345" } + let(:access_params) { { github_access_token: token } } + + def assign_session_token + session[:github_access_token] = token + end before do sign_in(user) @@ -20,7 +26,7 @@ describe Import::GithubController do get :callback - expect(user.reload.github_access_token).to eq(token) + expect(session[:github_access_token]).to eq(token) expect(controller).to redirect_to(status_import_github_url) end end @@ -30,6 +36,7 @@ describe Import::GithubController do @repo = OpenStruct.new(login: 'vim', full_name: 'asd/vim') @org = OpenStruct.new(login: 'company') @org_repo = OpenStruct.new(login: 'company', full_name: 'company/repo') + assign_session_token end it "assigns variables" do @@ -66,13 +73,14 @@ describe Import::GithubController do before do stub_client(user: github_user, repo: github_repo) + assign_session_token end context "when the repository owner is the GitHub user" do context "when the GitHub user and GitLab user's usernames match" do it "takes the current user's namespace" do expect(Gitlab::GithubImport::ProjectCreator). - to receive(:new).with(github_repo, user.namespace, user). + to receive(:new).with(github_repo, user.namespace, user, access_params). and_return(double(execute: true)) post :create, format: :js @@ -84,7 +92,7 @@ describe Import::GithubController do it "takes the current user's namespace" do expect(Gitlab::GithubImport::ProjectCreator). - to receive(:new).with(github_repo, user.namespace, user). + to receive(:new).with(github_repo, user.namespace, user, access_params). and_return(double(execute: true)) post :create, format: :js @@ -97,6 +105,7 @@ describe Import::GithubController do before do github_repo.owner = OpenStruct.new(login: other_username) + assign_session_token end context "when a namespace with the GitHub user's username already exists" do @@ -105,7 +114,7 @@ describe Import::GithubController do context "when the namespace is owned by the GitLab user" do it "takes the existing namespace" do expect(Gitlab::GithubImport::ProjectCreator). - to receive(:new).with(github_repo, existing_namespace, user). + to receive(:new).with(github_repo, existing_namespace, user, access_params). and_return(double(execute: true)) post :create, format: :js @@ -139,7 +148,7 @@ describe Import::GithubController do it "takes the new namespace" do expect(Gitlab::GithubImport::ProjectCreator). - to receive(:new).with(github_repo, an_instance_of(Group), user). + to receive(:new).with(github_repo, an_instance_of(Group), user, access_params). and_return(double(execute: true)) post :create, format: :js diff --git a/spec/controllers/import/gitlab_controller_spec.rb b/spec/controllers/import/gitlab_controller_spec.rb index 4bc67c86703..198d006af76 100644 --- a/spec/controllers/import/gitlab_controller_spec.rb +++ b/spec/controllers/import/gitlab_controller_spec.rb @@ -4,7 +4,13 @@ require_relative 'import_spec_helper' describe Import::GitlabController do include ImportSpecHelper - let(:user) { create(:user, gitlab_access_token: 'asd123') } + let(:user) { create(:user) } + let(:token) { "asdasd12345" } + let(:access_params) { { gitlab_access_token: token } } + + def assign_session_token + session[:gitlab_access_token] = token + end before do sign_in(user) @@ -13,14 +19,13 @@ describe Import::GitlabController do describe "GET callback" do it "updates access token" do - token = "asdasd12345" allow_any_instance_of(Gitlab::GitlabImport::Client). to receive(:get_token).and_return(token) stub_omniauth_provider('gitlab') get :callback - expect(user.reload.gitlab_access_token).to eq(token) + expect(session[:gitlab_access_token]).to eq(token) expect(controller).to redirect_to(status_import_gitlab_url) end end @@ -28,6 +33,7 @@ describe Import::GitlabController do describe "GET status" do before do @repo = OpenStruct.new(path: 'vim', path_with_namespace: 'asd/vim') + assign_session_token end it "assigns variables" do @@ -67,13 +73,14 @@ describe Import::GitlabController do before do stub_client(user: gitlab_user, project: gitlab_repo) + assign_session_token end context "when the repository owner is the GitLab.com user" do context "when the GitLab.com user and GitLab server user's usernames match" do it "takes the current user's namespace" do expect(Gitlab::GitlabImport::ProjectCreator). - to receive(:new).with(gitlab_repo, user.namespace, user). + to receive(:new).with(gitlab_repo, user.namespace, user, access_params). and_return(double(execute: true)) post :create, format: :js @@ -85,7 +92,7 @@ describe Import::GitlabController do it "takes the current user's namespace" do expect(Gitlab::GitlabImport::ProjectCreator). - to receive(:new).with(gitlab_repo, user.namespace, user). + to receive(:new).with(gitlab_repo, user.namespace, user, access_params). and_return(double(execute: true)) post :create, format: :js @@ -98,6 +105,7 @@ describe Import::GitlabController do before do gitlab_repo["namespace"]["path"] = other_username + assign_session_token end context "when a namespace with the GitLab.com user's username already exists" do @@ -106,7 +114,7 @@ describe Import::GitlabController do context "when the namespace is owned by the GitLab server user" do it "takes the existing namespace" do expect(Gitlab::GitlabImport::ProjectCreator). - to receive(:new).with(gitlab_repo, existing_namespace, user). + to receive(:new).with(gitlab_repo, existing_namespace, user, access_params). and_return(double(execute: true)) post :create, format: :js @@ -140,7 +148,7 @@ describe Import::GitlabController do it "takes the new namespace" do expect(Gitlab::GitlabImport::ProjectCreator). - to receive(:new).with(gitlab_repo, an_instance_of(Group), user). + to receive(:new).with(gitlab_repo, an_instance_of(Group), user, access_params). and_return(double(execute: true)) post :create, format: :js diff --git a/spec/lib/gitlab/bitbucket_import/project_creator_spec.rb b/spec/lib/gitlab/bitbucket_import/project_creator_spec.rb index f8958c9bab8..0e826a319e0 100644 --- a/spec/lib/gitlab/bitbucket_import/project_creator_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/project_creator_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::BitbucketImport::ProjectCreator do - let(:user) { create(:user, bitbucket_access_token: "asdffg", bitbucket_access_token_secret: "sekret") } + let(:user) { create(:user) } let(:repo) do { name: 'Vim', @@ -11,6 +11,9 @@ describe Gitlab::BitbucketImport::ProjectCreator do }.with_indifferent_access end let(:namespace){ create(:group, owner: user) } + let(:token) { "asdasd12345" } + let(:secret) { "sekrettt" } + let(:access_params) { { bitbucket_access_token: token, bitbucket_access_token_secret: secret } } before do namespace.add_owner(user) @@ -19,7 +22,7 @@ describe Gitlab::BitbucketImport::ProjectCreator do it 'creates project' do allow_any_instance_of(Project).to receive(:add_import_job) - project_creator = Gitlab::BitbucketImport::ProjectCreator.new(repo, namespace, user) + project_creator = Gitlab::BitbucketImport::ProjectCreator.new(repo, namespace, user, access_params) project = project_creator.execute expect(project.import_url).to eq("ssh://git@bitbucket.org/asd/vim.git") diff --git a/spec/lib/gitlab/github_import/project_creator_spec.rb b/spec/lib/gitlab/github_import/project_creator_spec.rb index 4fe7bd3b77d..ca61d3c5234 100644 --- a/spec/lib/gitlab/github_import/project_creator_spec.rb +++ b/spec/lib/gitlab/github_import/project_creator_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::GithubImport::ProjectCreator do - let(:user) { create(:user, github_access_token: "asdffg") } + let(:user) { create(:user) } let(:repo) do OpenStruct.new( login: 'vim', @@ -13,6 +13,8 @@ describe Gitlab::GithubImport::ProjectCreator do ) end let(:namespace){ create(:group, owner: user) } + let(:token) { "asdffg" } + let(:access_params) { { github_access_token: token } } before do namespace.add_owner(user) @@ -21,7 +23,7 @@ describe Gitlab::GithubImport::ProjectCreator do it 'creates project' do allow_any_instance_of(Project).to receive(:add_import_job) - project_creator = Gitlab::GithubImport::ProjectCreator.new(repo, namespace, user) + project_creator = Gitlab::GithubImport::ProjectCreator.new(repo, namespace, user, access_params) project = project_creator.execute expect(project.import_url).to eq("https://asdffg@gitlab.com/asd/vim.git") diff --git a/spec/lib/gitlab/gitlab_import/project_creator_spec.rb b/spec/lib/gitlab/gitlab_import/project_creator_spec.rb index 938d08396fd..2d8923d14bb 100644 --- a/spec/lib/gitlab/gitlab_import/project_creator_spec.rb +++ b/spec/lib/gitlab/gitlab_import/project_creator_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::GitlabImport::ProjectCreator do - let(:user) { create(:user, gitlab_access_token: "asdffg") } + let(:user) { create(:user) } let(:repo) do { name: 'vim', @@ -13,6 +13,8 @@ describe Gitlab::GitlabImport::ProjectCreator do }.with_indifferent_access end let(:namespace){ create(:group, owner: user) } + let(:token) { "asdffg" } + let(:access_params) { { gitlab_access_token: token } } before do namespace.add_owner(user) @@ -21,7 +23,7 @@ describe Gitlab::GitlabImport::ProjectCreator do it 'creates project' do allow_any_instance_of(Project).to receive(:add_import_job) - project_creator = Gitlab::GitlabImport::ProjectCreator.new(repo, namespace, user) + project_creator = Gitlab::GitlabImport::ProjectCreator.new(repo, namespace, user, access_params) project = project_creator.execute expect(project.import_url).to eq("https://oauth2:asdffg@gitlab.com/asd/vim.git")