root
/
gitlab-ce
mirror of https://gitlab.com/free-releases/gitlab-ce.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

Remove authentication using user.private_token

This commit is contained in:
Douwe Maan 2017-10-12 11:01:12 +02:00
parent c03d39df82
commit 294fa6fcdc
15 changed files with 75 additions and 200 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
app/controllers/application_controller.rb
View File

@ -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

2
doc/topics/autodevops/index.md
View File

@ -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

22
lib/api/api_guard.rb
View File

@ -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?

86
spec/controllers/application_controller_spec.rb
View File

@ -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

6
spec/features/atom/dashboard_issues_spec.rb
View File

@ -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")

6
spec/features/atom/dashboard_spec.rb
View File

@ -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

6
spec/features/atom/issues_spec.rb
View File

@ -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')

6
spec/features/atom/users_spec.rb
View File

@ -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

14
spec/features/profile_spec.rb
View File

@ -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

2
spec/models/concerns/token_authenticatable_spec.rb
View File

@ -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

9
spec/models/user_spec.rb
View File

@ -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)

2
spec/requests/api/doorkeeper_access_spec.rb
View File

@ -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)

53
spec/requests/api/helpers_spec.rb
View File

@ -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

26
spec/requests/api/users_spec.rb
View File

@ -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

28
spec/support/api_helpers.rb
View File

@ -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