From 6d43c95b7011ec7ec4600e00bdc8df76bb39813c Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 19 Sep 2016 13:38:58 +0200 Subject: [PATCH] Revert all changes introduced by https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6043 --- CHANGELOG | 1 - .../projects/git_http_client_controller.rb | 6 --- app/helpers/lfs_helper.rb | 2 +- doc/workflow/lfs/lfs_administration.md | 4 +- .../lfs/manage_large_binaries_with_git_lfs.md | 8 --- lib/api/internal.rb | 13 ----- lib/gitlab/auth.rb | 25 --------- lib/gitlab/auth/result.rb | 4 -- lib/gitlab/lfs_token.rb | 54 ------------------- spec/lib/gitlab/auth_spec.rb | 18 ------- spec/lib/gitlab/lfs_token_spec.rb | 51 ------------------ spec/requests/api/internal_spec.rb | 46 ---------------- spec/requests/lfs_http_spec.rb | 16 ------ 13 files changed, 3 insertions(+), 245 deletions(-) delete mode 100644 lib/gitlab/lfs_token.rb delete mode 100644 spec/lib/gitlab/lfs_token_spec.rb diff --git a/CHANGELOG b/CHANGELOG index cc54378f9f1..057c4bffda1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -106,7 +106,6 @@ v 8.12.0 (unreleased) - Remove green outline from `New branch unavailable` button on issue page !5858 (winniehell) - Fix repo title alignment (ClemMakesApps) - Change update interval of contacted_at - - Add LFS support to SSH !6043 - Fix branch title trailing space on hover (ClemMakesApps) - Don't include 'Created By' tag line when importing from GitHub if there is a linked GitLab account (EspadaV8) - Award emoji tooltips containing more than 10 usernames are now truncated !4780 (jlogandavison) diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb index ee9ea4bc8b2..d1a2c52d80a 100644 --- a/app/controllers/projects/git_http_client_controller.rb +++ b/app/controllers/projects/git_http_client_controller.rb @@ -132,12 +132,6 @@ class Projects::GitHttpClientController < Projects::ApplicationController authentication_project == project end - def lfs_deploy_key? - authentication_result.lfs_deploy_token? && - actor && - actor.projects.include?(project) - end - def authentication_has_download_access? has_authentication_ability?(:download_code) || has_authentication_ability?(:build_download_code) end diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb index 018ca7d7bba..8e827664681 100644 --- a/app/helpers/lfs_helper.rb +++ b/app/helpers/lfs_helper.rb @@ -25,7 +25,7 @@ module LfsHelper def lfs_download_access? return false unless project.lfs_enabled? - project.public? || ci? || lfs_deploy_key? || user_can_download_code? || build_can_download_code? + project.public? || ci? || user_can_download_code? || build_can_download_code? end def user_can_download_code? diff --git a/doc/workflow/lfs/lfs_administration.md b/doc/workflow/lfs/lfs_administration.md index b3c73e947f0..9dc1e9b47e3 100644 --- a/doc/workflow/lfs/lfs_administration.md +++ b/doc/workflow/lfs/lfs_administration.md @@ -45,5 +45,5 @@ In `config/gitlab.yml`: * Currently, storing GitLab Git LFS objects on a non-local storage (like S3 buckets) is not supported * Currently, removing LFS objects from GitLab Git LFS storage is not supported -* LFS authentications via SSH was added with GitLab 8.12 -* Only compatible with the GitLFS client versions 1.1.0 and up, or 1.0.2. +* LFS authentications via SSH is not supported for the time being +* Only compatible with the GitLFS client versions 1.1.0 or 1.0.2. diff --git a/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md b/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md index 1a4f213a792..9fe065fa680 100644 --- a/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md +++ b/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md @@ -35,10 +35,6 @@ Documentation for GitLab instance administrators is under [LFS administration do credentials store is recommended * Git LFS always assumes HTTPS so if you have GitLab server on HTTP you will have to add the URL to Git config manually (see #troubleshooting) - ->**Note**: With 8.12 GitLab added LFS support to SSH. The Git LFS communication - still goes over HTTP, but now the SSH client passes the correct credentials - to the Git LFS client, so no action is required by the user. ## Using Git LFS @@ -136,10 +132,6 @@ git config --add lfs.url "http://gitlab.example.com/group/project.git/info/lfs" ### Credentials are always required when pushing an object ->**Note**: With 8.12 GitLab added LFS support to SSH. The Git LFS communication - still goes over HTTP, but now the SSH client passes the correct credentials - to the Git LFS client, so no action is required by the user. - Given that Git LFS uses HTTP Basic Authentication to authenticate the user pushing the LFS object on every push for every object, user HTTPS credentials are required. diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 090d04544da..1114fd21784 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -82,19 +82,6 @@ module API response end - post "/lfs_authenticate" do - status 200 - - key = Key.find(params[:key_id]) - token_handler = Gitlab::LfsToken.new(key) - - { - username: token_handler.actor_name, - lfs_token: token_handler.generate, - repository_http_path: project.http_url_to_repo - } - end - get "/merge_request_urls" do ::MergeRequests::GetUrlsService.new(project).execute(params[:changes]) end diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index ca2a0920c00..7464d6082cb 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -11,7 +11,6 @@ module Gitlab build_access_token_check(login, password) || user_with_password_for_git(login, password) || oauth_access_token_check(login, password) || - lfs_token_check(login, password) || personal_access_token_check(login, password) || Result.new @@ -103,30 +102,6 @@ module Gitlab end end - def lfs_token_check(login, password) - deploy_key_matches = login.match(/\Alfs\+deploy-key-(\d+)\z/) - - actor = - if deploy_key_matches - DeployKey.find(deploy_key_matches[1]) - else - User.by_login(login) - end - - if actor - token_handler = Gitlab::LfsToken.new(actor) - - authentication_abilities = - if token_handler.user? - full_authentication_abilities - else - read_authentication_abilities - end - - Result.new(actor, nil, token_handler.type, authentication_abilities) if Devise.secure_compare(token_handler.value, password) - end - end - def build_access_token_check(login, password) return unless login == 'gitlab-ci-token' return unless password diff --git a/lib/gitlab/auth/result.rb b/lib/gitlab/auth/result.rb index e4786b12676..bf625649cbf 100644 --- a/lib/gitlab/auth/result.rb +++ b/lib/gitlab/auth/result.rb @@ -5,10 +5,6 @@ module Gitlab type == :ci end - def lfs_deploy_token? - type == :lfs_deploy_token - end - def success? actor.present? || type == :ci end diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb deleted file mode 100644 index d089a2f9b0b..00000000000 --- a/lib/gitlab/lfs_token.rb +++ /dev/null @@ -1,54 +0,0 @@ -module Gitlab - class LfsToken - attr_accessor :actor - - TOKEN_LENGTH = 50 - EXPIRY_TIME = 1800 - - def initialize(actor) - @actor = - case actor - when DeployKey, User - actor - when Key - actor.user - else - raise 'Bad Actor' - end - end - - def generate - token = Devise.friendly_token(TOKEN_LENGTH) - - Gitlab::Redis.with do |redis| - redis.set(redis_key, token, ex: EXPIRY_TIME) - end - - token - end - - def value - Gitlab::Redis.with do |redis| - redis.get(redis_key) - end - end - - def user? - actor.is_a?(User) - end - - def type - actor.is_a?(User) ? :lfs_token : :lfs_deploy_token - end - - def actor_name - actor.is_a?(User) ? actor.username : "lfs+deploy-key-#{actor.id}" - end - - private - - def redis_key - "gitlab:lfs_token:#{actor.class.name.underscore}_#{actor.id}" if actor - end - end -end diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 21f0d46100e..8807a68a0a2 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -61,24 +61,6 @@ describe Gitlab::Auth, lib: true do expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)) end - it 'recognizes user lfs tokens' do - user = create(:user) - ip = 'ip' - token = Gitlab::LfsToken.new(user).generate - - expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username) - expect(gl_auth.find_for_git_client(user.username, token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :lfs_token, read_authentication_abilities)) - end - - it 'recognizes deploy key lfs tokens' do - key = create(:deploy_key) - ip = 'ip' - token = Gitlab::LfsToken.new(key).generate - - expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: "lfs+deploy-key-#{key.id}") - expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_authentication_abilities)) - end - it 'recognizes OAuth tokens' do user = create(:user) application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) diff --git a/spec/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb deleted file mode 100644 index 9f04f67e0a8..00000000000 --- a/spec/lib/gitlab/lfs_token_spec.rb +++ /dev/null @@ -1,51 +0,0 @@ -require 'spec_helper' - -describe Gitlab::LfsToken, lib: true do - describe '#generate and #value' do - shared_examples 'an LFS token generator' do - it 'returns a randomly generated token' do - token = handler.generate - - expect(token).not_to be_nil - expect(token).to be_a String - expect(token.length).to eq 50 - end - - it 'returns the correct token based on the key' do - token = handler.generate - - expect(handler.value).to eq(token) - end - end - - context 'when the actor is a user' do - let(:actor) { create(:user) } - let(:handler) { described_class.new(actor) } - - it_behaves_like 'an LFS token generator' - - it 'returns the correct username' do - expect(handler.actor_name).to eq(actor.username) - end - - it 'returns the correct token type' do - expect(handler.type).to eq(:lfs_token) - end - end - - context 'when the actor is a deploy key' do - let(:actor) { create(:deploy_key) } - let(:handler) { described_class.new(actor) } - - it_behaves_like 'an LFS token generator' - - it 'returns the correct username' do - expect(handler.actor_name).to eq("lfs+deploy-key-#{actor.id}") - end - - it 'returns the correct token type' do - expect(handler.type).to eq(:lfs_deploy_token) - end - end - end -end diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 46e8e6f1169..46d1b868782 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -100,43 +100,6 @@ describe API::API, api: true do end end - describe "POST /internal/lfs_authenticate" do - before do - project.team << [user, :developer] - end - - context 'user key' do - it 'returns the correct information about the key' do - lfs_auth(key.id, project) - - expect(response).to have_http_status(200) - expect(json_response['username']).to eq(user.username) - expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).value) - - expect(json_response['repository_http_path']).to eq(project.http_url_to_repo) - end - - it 'returns a 404 when the wrong key is provided' do - lfs_auth(nil, project) - - expect(response).to have_http_status(404) - end - end - - context 'deploy key' do - let(:key) { create(:deploy_key) } - - it 'returns the correct information about the key' do - lfs_auth(key.id, project) - - expect(response).to have_http_status(200) - expect(json_response['username']).to eq("lfs+deploy-key-#{key.id}") - expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).value) - expect(json_response['repository_http_path']).to eq(project.http_url_to_repo) - end - end - end - describe "GET /internal/discover" do it do get(api("/internal/discover"), key_id: key.id, secret_token: secret_token) @@ -426,13 +389,4 @@ describe API::API, api: true do protocol: 'ssh' ) end - - def lfs_auth(key_id, project) - post( - api("/internal/lfs_authenticate"), - key_id: key_id, - secret_token: secret_token, - project: project.path_with_namespace - ) - end end diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 09e4e265dd1..b58d410b7a3 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -245,18 +245,6 @@ describe 'Git LFS API and storage' do end end - context 'when deploy key is authorized' do - let(:key) { create(:deploy_key) } - let(:authorization) { authorize_deploy_key } - - let(:update_permissions) do - project.deploy_keys << key - project.lfs_objects << lfs_object - end - - it_behaves_like 'responds with a file' - end - context 'when build is authorized as' do let(:authorization) { authorize_ci_project } @@ -1109,10 +1097,6 @@ describe 'Git LFS API and storage' do ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password) end - def authorize_deploy_key - ActionController::HttpAuthentication::Basic.encode_credentials("lfs+deploy-key-#{key.id}", Gitlab::LfsToken.new(key).generate) - end - def fork_project(project, user, object = nil) allow(RepositoryForkWorker).to receive(:perform_async).and_return(true) Projects::ForkService.new(project, user, {}).execute