summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@selenight.nl>2017-10-12 11:01:12 +0200
committerDouwe Maan <douwe@selenight.nl>2017-11-02 11:39:02 +0100
commit294fa6fcdcfa7d76bc97b754d2930f3686f54997 (patch)
treedd2093f86c6828fa59a5a4cbd79ffda16e883382
parentc03d39df8234be7fc6c846df05e08f204e6f0456 (diff)
downloadgitlab-ce-294fa6fcdcfa7d76bc97b754d2930f3686f54997.tar.gz
Remove authentication using user.private_token
-rw-r--r--app/controllers/application_controller.rb7
-rw-r--r--doc/topics/autodevops/index.md2
-rw-r--r--lib/api/api_guard.rb22
-rw-r--r--spec/controllers/application_controller_spec.rb86
-rw-r--r--spec/features/atom/dashboard_issues_spec.rb6
-rw-r--r--spec/features/atom/dashboard_spec.rb6
-rw-r--r--spec/features/atom/issues_spec.rb6
-rw-r--r--spec/features/atom/users_spec.rb6
-rw-r--r--spec/features/profile_spec.rb14
-rw-r--r--spec/models/concerns/token_authenticatable_spec.rb2
-rw-r--r--spec/models/user_spec.rb9
-rw-r--r--spec/requests/api/doorkeeper_access_spec.rb2
-rw-r--r--spec/requests/api/helpers_spec.rb53
-rw-r--r--spec/requests/api/users_spec.rb26
-rw-r--r--spec/support/api_helpers.rb28
15 files changed, 75 insertions, 200 deletions
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 391a0519195..3be7aee69bc 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -11,7 +11,7 @@ class ApplicationController < ActionController::Base
include EnforcesTwoFactorAuthentication
include WithPerformanceBar
- before_action :authenticate_user_from_private_token!
+ before_action :authenticate_user_from_personal_access_token!
before_action :authenticate_user_from_rss_token!
before_action :authenticate_user!
before_action :validate_user_service_ticket!
@@ -100,13 +100,12 @@ class ApplicationController < ActionController::Base
return try(:authenticated_user)
end
- # This filter handles both private tokens and personal access tokens
- def authenticate_user_from_private_token!
+ def authenticate_user_from_personal_access_token!
token = params[:private_token].presence || request.headers['PRIVATE-TOKEN'].presence
return unless token.present?
- user = User.find_by_authentication_token(token) || User.find_by_personal_access_token(token)
+ user = User.find_by_personal_access_token(token)
sessionless_sign_in(user)
end
diff --git a/doc/topics/autodevops/index.md b/doc/topics/autodevops/index.md
index 5561784ed0b..042cde3f01e 100644
--- a/doc/topics/autodevops/index.md
+++ b/doc/topics/autodevops/index.md
@@ -517,7 +517,7 @@ Feature.get(:auto_devops_banner_disabled).enable
Or through the HTTP API with an admin access token:
```sh
-curl --data "value=true" --header "PRIVATE-TOKEN: private_token" https://gitlab.example.com/api/v4/features/auto_devops_banner_disabled
+curl --data "value=true" --header "PRIVATE-TOKEN: personal_access_token" https://gitlab.example.com/api/v4/features/auto_devops_banner_disabled
```
[ce-37115]: https://gitlab.com/gitlab-org/gitlab-ce/issues/37115
diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb
index 87b9db66efd..0ff376bbab6 100644
--- a/lib/api/api_guard.rb
+++ b/lib/api/api_guard.rb
@@ -44,7 +44,7 @@ module API
module HelperMethods
def find_current_user
user =
- find_user_from_private_token ||
+ find_user_from_personal_access_token ||
find_user_from_oauth_token ||
find_user_from_warden
@@ -61,13 +61,14 @@ module API
private
- def find_user_from_private_token
+ def find_user_from_personal_access_token
token_string = private_token.to_s
return nil unless token_string.present?
- user =
- find_user_by_authentication_token(token_string) ||
- find_user_by_personal_access_token(token_string)
+ access_token = PersonalAccessToken.find_by_token(token_string)
+ raise UnauthorizedError unless access_token
+
+ user = find_user_by_access_token(access_token)
raise UnauthorizedError unless user
@@ -99,17 +100,6 @@ module API
find_user_by_access_token(access_token)
end
- def find_user_by_authentication_token(token_string)
- User.find_by_authentication_token(token_string)
- end
-
- def find_user_by_personal_access_token(token_string)
- access_token = PersonalAccessToken.find_by_token(token_string)
- return unless access_token
-
- find_user_by_access_token(access_token)
- end
-
# Check the Rails session for valid authentication details
def find_user_from_warden
warden.try(:authenticate) if verified_request?
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index 6802b839eaa..b73ca0c2346 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -50,70 +50,36 @@ describe ApplicationController do
end
end
- describe "#authenticate_user_from_token!" do
- describe "authenticating a user from a private token" do
- controller(described_class) do
- def index
- render text: "authenticated"
- end
- end
-
- context "when the 'private_token' param is populated with the private token" do
- it "logs the user in" do
- get :index, private_token: user.private_token
- expect(response).to have_gitlab_http_status(200)
- expect(response.body).to eq("authenticated")
- end
- end
-
- context "when the 'PRIVATE-TOKEN' header is populated with the private token" do
- it "logs the user in" do
- @request.headers['PRIVATE-TOKEN'] = user.private_token
- get :index
- expect(response).to have_gitlab_http_status(200)
- expect(response.body).to eq("authenticated")
- end
- end
-
- it "doesn't log the user in otherwise" do
- @request.headers['PRIVATE-TOKEN'] = "token"
- get :index, private_token: "token", authenticity_token: "token"
- expect(response.status).not_to eq(200)
- expect(response.body).not_to eq("authenticated")
+ describe "#authenticate_user_from_personal_access_token!" do
+ controller(described_class) do
+ def index
+ render text: 'authenticated'
end
end
- describe "authenticating a user from a personal access token" do
- controller(described_class) do
- def index
- render text: 'authenticated'
- end
- end
-
- let(:personal_access_token) { create(:personal_access_token, user: user) }
+ let(:personal_access_token) { create(:personal_access_token, user: user) }
- context "when the 'personal_access_token' param is populated with the personal access token" do
- it "logs the user in" do
- get :index, private_token: personal_access_token.token
- expect(response).to have_gitlab_http_status(200)
- expect(response.body).to eq('authenticated')
- end
+ context "when the 'personal_access_token' param is populated with the personal access token" do
+ it "logs the user in" do
+ get :index, private_token: personal_access_token.token
+ expect(response).to have_gitlab_http_status(200)
+ expect(response.body).to eq('authenticated')
end
+ end
- context "when the 'PERSONAL_ACCESS_TOKEN' header is populated with the personal access token" do
- it "logs the user in" do
- @request.headers["PRIVATE-TOKEN"] = personal_access_token.token
- get :index
- expect(response).to have_gitlab_http_status(200)
- expect(response.body).to eq('authenticated')
- end
+ context "when the 'PERSONAL_ACCESS_TOKEN' header is populated with the personal access token" do
+ it "logs the user in" do
+ @request.headers["PRIVATE-TOKEN"] = personal_access_token.token
+ get :index
+ expect(response).to have_gitlab_http_status(200)
+ expect(response.body).to eq('authenticated')
end
+ end
- it "doesn't log the user in otherwise" do
- get :index, private_token: "token"
- expect(response.status).not_to eq(200)
- expect(response.body).not_to eq('authenticated')
- end
+ it "doesn't log the user in otherwise" do
+ get :index, private_token: "token"
+ expect(response.status).not_to eq(200)
+ expect(response.body).not_to eq('authenticated')
end
end
@@ -152,11 +118,15 @@ describe ApplicationController do
end
end
+ before do
+ sign_in user
+ end
+
context 'when format is handled' do
let(:requested_format) { :json }
it 'returns 200 response' do
- get :index, private_token: user.private_token, format: requested_format
+ get :index, format: requested_format
expect(response).to have_gitlab_http_status 200
end
@@ -164,7 +134,7 @@ describe ApplicationController do
context 'when format is not handled' do
it 'returns 404 response' do
- get :index, private_token: user.private_token
+ get :index
expect(response).to have_gitlab_http_status 404
end
diff --git a/spec/features/atom/dashboard_issues_spec.rb b/spec/features/atom/dashboard_issues_spec.rb
index 5aae2dbaf91..89c9d377003 100644
--- a/spec/features/atom/dashboard_issues_spec.rb
+++ b/spec/features/atom/dashboard_issues_spec.rb
@@ -13,8 +13,10 @@ describe "Dashboard Issues Feed" do
end
describe "atom feed" do
- it "renders atom feed via private token" do
- visit issues_dashboard_path(:atom, private_token: user.private_token)
+ it "renders atom feed via personal access token" do
+ personal_access_token = create(:personal_access_token, user: user)
+
+ visit issues_dashboard_path(:atom, private_token: personal_access_token.token)
expect(response_headers['Content-Type']).to have_content('application/atom+xml')
expect(body).to have_selector('title', text: "#{user.name} issues")
diff --git a/spec/features/atom/dashboard_spec.rb b/spec/features/atom/dashboard_spec.rb
index 321c8a2a670..2c0c331b6db 100644
--- a/spec/features/atom/dashboard_spec.rb
+++ b/spec/features/atom/dashboard_spec.rb
@@ -4,9 +4,11 @@ describe "Dashboard Feed" do
describe "GET /" do
let!(:user) { create(:user, name: "Jonh") }
- context "projects atom feed via private token" do
+ context "projects atom feed via personal access token" do
it "renders projects atom feed" do
- visit dashboard_projects_path(:atom, private_token: user.private_token)
+ personal_access_token = create(:personal_access_token, user: user)
+
+ visit dashboard_projects_path(:atom, private_token: personal_access_token.token)
expect(body).to have_selector('feed title')
end
end
diff --git a/spec/features/atom/issues_spec.rb b/spec/features/atom/issues_spec.rb
index 3eeb4d35131..4102ac0588a 100644
--- a/spec/features/atom/issues_spec.rb
+++ b/spec/features/atom/issues_spec.rb
@@ -28,10 +28,12 @@ describe 'Issues Feed' do
end
end
- context 'when authenticated via private token' do
+ context 'when authenticated via personal access token' do
it 'renders atom feed' do
+ personal_access_token = create(:personal_access_token, user: user)
+
visit project_issues_path(project, :atom,
- private_token: user.private_token)
+ private_token: personal_access_token.token)
expect(response_headers['Content-Type'])
.to have_content('application/atom+xml')
diff --git a/spec/features/atom/users_spec.rb b/spec/features/atom/users_spec.rb
index 9ce687afb31..2b934d81674 100644
--- a/spec/features/atom/users_spec.rb
+++ b/spec/features/atom/users_spec.rb
@@ -4,9 +4,11 @@ describe "User Feed" do
describe "GET /" do
let!(:user) { create(:user) }
- context 'user atom feed via private token' do
+ context 'user atom feed via personal access token' do
it "renders user atom feed" do
- visit user_path(user, :atom, private_token: user.private_token)
+ personal_access_token = create(:personal_access_token, user: user)
+
+ visit user_path(user, :atom, private_token: personal_access_token.token)
expect(body).to have_selector('feed title')
end
end
diff --git a/spec/features/profile_spec.rb b/spec/features/profile_spec.rb
index 1cddd35fd8a..e05853110fe 100644
--- a/spec/features/profile_spec.rb
+++ b/spec/features/profile_spec.rb
@@ -56,20 +56,6 @@ describe 'Profile account page' do
end
end
- describe 'when I reset private token' do
- before do
- visit profile_account_path
- end
-
- it 'resets private token' do
- previous_token = find("#private-token").value
-
- click_link('Reset private token')
-
- expect(find('#private-token').value).not_to eq(previous_token)
- end
- end
-
describe 'when I reset RSS token' do
before do
visit profile_account_path
diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb
index 882afeccfc6..dfb83578fce 100644
--- a/spec/models/concerns/token_authenticatable_spec.rb
+++ b/spec/models/concerns/token_authenticatable_spec.rb
@@ -12,7 +12,7 @@ shared_examples 'TokenAuthenticatable' do
end
describe User, 'TokenAuthenticatable' do
- let(:token_field) { :authentication_token }
+ let(:token_field) { :rss_token }
it_behaves_like 'TokenAuthenticatable'
describe 'ensures authentication token' do
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 1c3c9068f12..fb03e320734 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -346,7 +346,6 @@ describe User do
describe "Respond to" do
it { is_expected.to respond_to(:admin?) }
it { is_expected.to respond_to(:name) }
- it { is_expected.to respond_to(:private_token) }
it { is_expected.to respond_to(:external?) }
end
@@ -526,14 +525,6 @@ describe User do
end
end
- describe 'authentication token' do
- it "has authentication token" do
- user = create(:user)
-
- expect(user.authentication_token).not_to be_blank
- end
- end
-
describe 'ensure incoming email token' do
it 'has incoming email token' do
user = create(:user)
diff --git a/spec/requests/api/doorkeeper_access_spec.rb b/spec/requests/api/doorkeeper_access_spec.rb
index de7ce848a31..174593593f8 100644
--- a/spec/requests/api/doorkeeper_access_spec.rb
+++ b/spec/requests/api/doorkeeper_access_spec.rb
@@ -25,7 +25,7 @@ describe 'doorkeeper access' do
end
end
- describe "authorization by private token" do
+ describe "authorization by OAuth token" do
it "returns authentication success" do
get api("/user", user)
expect(response).to have_gitlab_http_status(200)
diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb
index 9f3b5a809d7..0ab9f94376c 100644
--- a/spec/requests/api/helpers_spec.rb
+++ b/spec/requests/api/helpers_spec.rb
@@ -28,17 +28,17 @@ describe API::Helpers do
allow_any_instance_of(self.class).to receive(:options).and_return({})
end
- def set_env(user_or_token, identifier)
+ def set_env(token, identifier)
clear_env
clear_param
- env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user_or_token.respond_to?(:private_token) ? user_or_token.private_token : user_or_token
+ env[API::APIGuard::PRIVATE_TOKEN_HEADER] = token
env[API::Helpers::SUDO_HEADER] = identifier.to_s
end
- def set_param(user_or_token, identifier)
+ def set_param(token, identifier)
clear_env
clear_param
- params[API::APIGuard::PRIVATE_TOKEN_PARAM] = user_or_token.respond_to?(:private_token) ? user_or_token.private_token : user_or_token
+ params[API::APIGuard::PRIVATE_TOKEN_PARAM] = token
params[API::Helpers::SUDO_PARAM] = identifier.to_s
end
@@ -160,41 +160,6 @@ describe API::Helpers do
end
end
- describe "when authenticating using a user's private token" do
- it "returns a 401 response for an invalid token" do
- env[API::APIGuard::PRIVATE_TOKEN_HEADER] = 'invalid token'
- allow_any_instance_of(self.class).to receive(:doorkeeper_guard) { false }
-
- expect { current_user }.to raise_error /401/
- end
-
- it "returns a 401 response for a user without access" do
- env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user.private_token
- allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
-
- expect { current_user }.to raise_error /401/
- end
-
- it 'returns a 401 response for a user who is blocked' do
- user.block!
- env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user.private_token
-
- expect { current_user }.to raise_error /401/
- end
-
- it "leaves user as is when sudo not specified" do
- env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user.private_token
-
- expect(current_user).to eq(user)
-
- clear_env
-
- params[API::APIGuard::PRIVATE_TOKEN_PARAM] = user.private_token
-
- expect(current_user).to eq(user)
- end
- end
-
describe "when authenticating using a user's personal access tokens" do
let(:personal_access_token) { create(:personal_access_token, user: user) }
@@ -445,16 +410,6 @@ describe API::Helpers do
expect { sudo? }.to raise_error '403 - {"message"=>"403 Forbidden - Private token must be specified in order to use sudo"}'
end
end
-
- context 'private access token is used' do
- before do
- set_env(admin.private_token, user.id)
- end
-
- it 'returns true' do
- expect(sudo?).to be_truthy
- end
- end
end
end
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 4737f034f21..fc1d055afe2 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -1097,14 +1097,6 @@ describe API::Users do
end
end
- context 'with private token' do
- it 'returns 403 without private token when sudo defined' do
- get api("/user?private_token=#{user.private_token}&sudo=123")
-
- expect(response).to have_gitlab_http_status(403)
- end
- end
-
it 'returns current user without private token when sudo not defined' do
get api("/user", user)
@@ -1139,24 +1131,6 @@ describe API::Users do
expect(json_response['id']).to eq(admin.id)
end
end
-
- context 'with private token' do
- it 'returns sudoed user with private token when sudo defined' do
- get api("/user?private_token=#{admin.private_token}&sudo=#{user.id}")
-
- expect(response).to have_gitlab_http_status(200)
- expect(response).to match_response_schema('public_api/v4/user/login')
- expect(json_response['id']).to eq(user.id)
- end
-
- it 'returns initial current user without private token but with is_admin when sudo not defined' do
- get api("/user?private_token=#{admin.private_token}")
-
- expect(response).to have_gitlab_http_status(200)
- expect(response).to match_response_schema('public_api/v4/user/admin')
- expect(json_response['id']).to eq(admin.id)
- end
- end
end
context 'with unauthenticated user' do
diff --git a/spec/support/api_helpers.rb b/spec/support/api_helpers.rb
index 01aca74274c..ac0c7a9b493 100644
--- a/spec/support/api_helpers.rb
+++ b/spec/support/api_helpers.rb
@@ -18,21 +18,23 @@ module ApiHelpers
#
# Returns the relative path to the requested API resource
def api(path, user = nil, version: API::API.version, personal_access_token: nil, oauth_access_token: nil)
- "/api/#{version}#{path}" +
+ full_path = "/api/#{version}#{path}"
- # Normalize query string
- (path.index('?') ? '' : '?') +
+ if oauth_access_token
+ query_string = "access_token=#{oauth_access_token.token}"
+ elsif personal_access_token
+ query_string = "private_token=#{personal_access_token.token}"
+ elsif user
+ personal_access_token = create(:personal_access_token, user: user)
+ query_string = "private_token=#{personal_access_token.token}"
+ end
- if personal_access_token.present?
- "&private_token=#{personal_access_token.token}"
- elsif oauth_access_token.present?
- "&access_token=#{oauth_access_token.token}"
- # Append private_token if given a User object
- elsif user.respond_to?(:private_token)
- "&private_token=#{user.private_token}"
- else
- ''
- end
+ if query_string
+ full_path << (path.index('?') ? '&' : '?')
+ full_path << query_string
+ end
+
+ full_path
end
# Temporary helper method for simplifying V3 exclusive API specs