Commit a3dfb58e authored by Simon Vocella's avatar Simon Vocella Committed by Tiago Botelho

add impersonation token

parent 81246e56
...@@ -7,13 +7,19 @@ class PersonalAccessToken < ActiveRecord::Base ...@@ -7,13 +7,19 @@ class PersonalAccessToken < ActiveRecord::Base
belongs_to :user belongs_to :user
default_scope { where(impersonation: false) }
scope :active, -> { where(revoked: false).where("expires_at >= NOW() OR expires_at IS NULL") } scope :active, -> { where(revoked: false).where("expires_at >= NOW() OR expires_at IS NULL") }
scope :inactive, -> { where("revoked = true OR expires_at < NOW()") } scope :inactive, -> { where("revoked = true OR expires_at < NOW()") }
scope :impersonation, -> { where(impersonation: true) }
def self.generate(params) class << self
personal_access_token = self.new(params) alias_method :and_impersonation_tokens, :unscoped
personal_access_token.ensure_token
personal_access_token def generate(params)
personal_access_token = self.new(params)
personal_access_token.ensure_token
personal_access_token
end
end end
def revoke! def revoke!
......
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddImpersonationToPersonalAccessTokens < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
def up
add_column_with_default :personal_access_tokens, :impersonation, :boolean, default: false
end
def down
remove_column :personal_access_tokens, :impersonation
end
end
...@@ -883,6 +883,7 @@ ActiveRecord::Schema.define(version: 20170216141440) do ...@@ -883,6 +883,7 @@ ActiveRecord::Schema.define(version: 20170216141440) do
t.datetime "created_at", null: false t.datetime "created_at", null: false
t.datetime "updated_at", null: false t.datetime "updated_at", null: false
t.string "scopes", default: "--- []\n", null: false t.string "scopes", default: "--- []\n", null: false
t.boolean "impersonation", default: false, null: false
end end
add_index "personal_access_tokens", ["token"], name: "index_personal_access_tokens_on_token", unique: true, using: :btree add_index "personal_access_tokens", ["token"], name: "index_personal_access_tokens_on_token", unique: true, using: :btree
......
...@@ -706,6 +706,7 @@ module API ...@@ -706,6 +706,7 @@ module API
end end
class PersonalAccessToken < BasicPersonalAccessToken class PersonalAccessToken < BasicPersonalAccessToken
expose :impersonation
expose :token expose :token
end end
end end
......
...@@ -367,6 +367,7 @@ module API ...@@ -367,6 +367,7 @@ module API
params do params do
requires :user_id, type: Integer requires :user_id, type: Integer
optional :state, type: String, default: 'all', values: %w[all active inactive], desc: 'Filters (all|active|inactive) personal_access_tokens' optional :state, type: String, default: 'all', values: %w[all active inactive], desc: 'Filters (all|active|inactive) personal_access_tokens'
optional :impersonation, type: Boolean, default: false, desc: 'Filters only impersonation personal_access_token'
end end
get ':user_id/personal_access_tokens' do get ':user_id/personal_access_tokens' do
authenticated_as_admin! authenticated_as_admin!
...@@ -374,7 +375,8 @@ module API ...@@ -374,7 +375,8 @@ module API
user = User.find_by(id: params[:user_id]) user = User.find_by(id: params[:user_id])
not_found!('User') unless user not_found!('User') unless user
personal_access_tokens = user.personal_access_tokens personal_access_tokens = PersonalAccessToken.and_impersonation_tokens.where(user_id: user.id)
personal_access_tokens = personal_access_tokens.impersonation if params[:impersonation]
case params[:state] case params[:state]
when "active" when "active"
...@@ -392,6 +394,7 @@ module API ...@@ -392,6 +394,7 @@ module API
requires :name, type: String, desc: 'The name of the personal access token' requires :name, type: String, desc: 'The name of the personal access token'
optional :expires_at, type: Date, desc: 'The expiration date in the format YEAR-MONTH-DAY of the personal access token' optional :expires_at, type: Date, desc: 'The expiration date in the format YEAR-MONTH-DAY of the personal access token'
optional :scopes, type: Array, desc: 'The array of scopes of the personal access token' optional :scopes, type: Array, desc: 'The array of scopes of the personal access token'
optional :impersonation, type: Boolean, default: false, desc: 'The impersonation flag of the personal access token'
end end
post ':user_id/personal_access_tokens' do post ':user_id/personal_access_tokens' do
authenticated_as_admin! authenticated_as_admin!
...@@ -419,7 +422,7 @@ module API ...@@ -419,7 +422,7 @@ module API
user = User.find_by(id: params[:user_id]) user = User.find_by(id: params[:user_id])
not_found!('User') unless user not_found!('User') unless user
personal_access_token = PersonalAccessToken.find_by(id: params[:personal_access_token_id]) personal_access_token = PersonalAccessToken.and_impersonation_tokens.find_by(user_id: user.id, id: params[:personal_access_token_id])
not_found!('PersonalAccessToken') unless personal_access_token not_found!('PersonalAccessToken') unless personal_access_token
personal_access_token.revoke! personal_access_token.revoke!
......
...@@ -18,8 +18,8 @@ module Gitlab ...@@ -18,8 +18,8 @@ module Gitlab
build_access_token_check(login, password) || build_access_token_check(login, password) ||
lfs_token_check(login, password) || lfs_token_check(login, password) ||
oauth_access_token_check(login, password) || oauth_access_token_check(login, password) ||
personal_access_token_check(login, password) ||
user_with_password_for_git(login, password) || user_with_password_for_git(login, password) ||
personal_access_token_check(password) ||
Gitlab::Auth::Result.new Gitlab::Auth::Result.new
rate_limit!(ip, success: result.success?, login: login) rate_limit!(ip, success: result.success?, login: login)
...@@ -102,14 +102,13 @@ module Gitlab ...@@ -102,14 +102,13 @@ module Gitlab
end end
end end
def personal_access_token_check(login, password) def personal_access_token_check(password)
if login && password return unless password.present?
token = PersonalAccessToken.active.find_by_token(password)
validation = User.by_login(login)
if valid_personal_access_token?(token, validation) token = PersonalAccessToken.and_impersonation_tokens.active.find_by_token(password)
Gitlab::Auth::Result.new(validation, nil, :personal_token, full_authentication_abilities)
end if token && (valid_api_token?(token) || token.impersonation)
Gitlab::Auth::Result.new(token.user, nil, :personal_token, full_authentication_abilities)
end end
end end
...@@ -117,10 +116,6 @@ module Gitlab ...@@ -117,10 +116,6 @@ module Gitlab
token && token.accessible? && valid_api_token?(token) token && token.accessible? && valid_api_token?(token)
end end
def valid_personal_access_token?(token, user)
token && token.user == user && valid_api_token?(token)
end
def valid_api_token?(token) def valid_api_token?(token)
AccessTokenValidationService.new(token).include_any_scope?(['api']) AccessTokenValidationService.new(token).include_any_scope?(['api'])
end end
......
...@@ -3,13 +3,13 @@ require 'spec_helper' ...@@ -3,13 +3,13 @@ require 'spec_helper'
describe Profiles::PersonalAccessTokensController do describe Profiles::PersonalAccessTokensController do
let(:user) { create(:user) } let(:user) { create(:user) }
before { sign_in(user) }
describe '#create' do describe '#create' do
def created_token def created_token
PersonalAccessToken.order(:created_at).last PersonalAccessToken.order(:created_at).last
end end
before { sign_in(user) }
it "allows creation of a token" do it "allows creation of a token" do
name = FFaker::Product.brand name = FFaker::Product.brand
...@@ -46,4 +46,29 @@ describe Profiles::PersonalAccessTokensController do ...@@ -46,4 +46,29 @@ describe Profiles::PersonalAccessTokensController do
end end
end end
end end
describe '#index' do
let!(:active_personal_access_token) { create(:personal_access_token, user: user) }
let!(:inactive_personal_access_token) { create(:revoked_personal_access_token, user: user) }
let!(:impersonation_personal_access_token) { create(:impersonation_personal_access_token, user: user) }
it "retrieves active personal access tokens" do
get :index
expect(assigns(:active_personal_access_tokens)).to include(active_personal_access_token)
end
it "retrieves inactive personal access tokens" do
get :index
expect(assigns(:inactive_personal_access_tokens)).to include(inactive_personal_access_token)
end
it "does not retrieve impersonation personal access tokens" do
get :index
expect(assigns(:active_personal_access_tokens)).not_to include(impersonation_personal_access_token)
expect(assigns(:inactive_personal_access_tokens)).not_to include(impersonation_personal_access_token)
end
end
end end
...@@ -6,6 +6,7 @@ FactoryGirl.define do ...@@ -6,6 +6,7 @@ FactoryGirl.define do
revoked false revoked false
expires_at { 5.days.from_now } expires_at { 5.days.from_now }
scopes ['api'] scopes ['api']
impersonation false
factory :revoked_personal_access_token do factory :revoked_personal_access_token do
revoked true revoked true
...@@ -14,5 +15,9 @@ FactoryGirl.define do ...@@ -14,5 +15,9 @@ FactoryGirl.define do
factory :expired_personal_access_token do factory :expired_personal_access_token do
expires_at { 1.day.ago } expires_at { 1.day.ago }
end end
factory :impersonation_personal_access_token do
impersonation true
end
end end
end end
...@@ -110,25 +110,30 @@ describe Gitlab::Auth, lib: true do ...@@ -110,25 +110,30 @@ describe Gitlab::Auth, lib: true do
end end
context 'while using personal access tokens as passwords' do context 'while using personal access tokens as passwords' do
let(:user) { create(:user) }
let(:token_w_api_scope) { create(:personal_access_token, user: user, scopes: ['api']) }
it 'succeeds for personal access tokens with the `api` scope' do it 'succeeds for personal access tokens with the `api` scope' do
expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: user.email) personal_access_token = create(:personal_access_token, scopes: ['api'])
expect(gl_auth.find_for_git_client(user.email, token_w_api_scope.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :personal_token, full_authentication_abilities))
expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '')
expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(personal_access_token.user, nil, :personal_token, full_authentication_abilities))
end end
it 'fails for personal access tokens with other scopes' do it 'succeeds if it is an impersonation token' do
personal_access_token = create(:personal_access_token, user: user, scopes: ['read_user']) personal_access_token = create(:personal_access_token, impersonation: true, scopes: [])
expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: user.email) expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '')
expect(gl_auth.find_for_git_client(user.email, personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil)) expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(personal_access_token.user, nil, :personal_token, full_authentication_abilities))
end end
it 'does not try password auth before personal access tokens' do it 'fails for personal access tokens with other scopes' do
expect(gl_auth).not_to receive(:find_with_user_password) personal_access_token = create(:personal_access_token, scopes: ['read_user'])
expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '')
expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil))
end
gl_auth.find_for_git_client(user.email, token_w_api_scope.token, project: nil, ip: 'ip') it 'fails if password is nil' do
expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '')
expect(gl_auth.find_for_git_client('', nil, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil))
end end
end end
......
...@@ -1161,6 +1161,7 @@ describe API::Users, api: true do ...@@ -1161,6 +1161,7 @@ describe API::Users, api: true do
let!(:active_personal_access_token) { create(:personal_access_token, user: user) } let!(:active_personal_access_token) { create(:personal_access_token, user: user) }
let!(:revoked_personal_access_token) { create(:revoked_personal_access_token, user: user) } let!(:revoked_personal_access_token) { create(:revoked_personal_access_token, user: user) }
let!(:expired_personal_access_token) { create(:expired_personal_access_token, user: user) } let!(:expired_personal_access_token) { create(:expired_personal_access_token, user: user) }
let!(:impersonation_personal_access_token) { create(:impersonation_personal_access_token, user: user) }
it 'returns a 404 error if user not found' do it 'returns a 404 error if user not found' do
get api("/users/#{not_existing_user_id}/personal_access_tokens", admin) get api("/users/#{not_existing_user_id}/personal_access_tokens", admin)
...@@ -1181,7 +1182,7 @@ describe API::Users, api: true do ...@@ -1181,7 +1182,7 @@ describe API::Users, api: true do
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
expect(json_response).to be_an Array expect(json_response).to be_an Array
expect(json_response.size).to eq(3) expect(json_response.size).to eq(4)
expect(json_response.detect do |personal_access_token| expect(json_response.detect do |personal_access_token|
personal_access_token['id'] == active_personal_access_token.id personal_access_token['id'] == active_personal_access_token.id
end['token']).to eq(active_personal_access_token.token) end['token']).to eq(active_personal_access_token.token)
...@@ -1202,12 +1203,21 @@ describe API::Users, api: true do ...@@ -1202,12 +1203,21 @@ describe API::Users, api: true do
expect(json_response).to be_an Array expect(json_response).to be_an Array
expect(json_response).to all(include('active' => false)) expect(json_response).to all(include('active' => false))
end end
it 'returns an array of impersonation personal access tokens if impersonation is set to true' do
get api("/users/#{user.id}/personal_access_tokens?impersonation=true", admin)
expect(response).to have_http_status(200)
expect(json_response).to be_an Array
expect(json_response).to all(include('impersonation' => true))
end
end end
describe 'POST /users/:user_id/personal_access_tokens' do describe 'POST /users/:user_id/personal_access_tokens' do
let(:name) { 'my new pat' } let(:name) { 'my new pat' }
let(:expires_at) { '2016-12-28' } let(:expires_at) { '2016-12-28' }
let(:scopes) { ['api', 'read_user'] } let(:scopes) { ['api', 'read_user'] }
let(:impersonation) { true }
it 'returns validation error if personal access token miss some attributes' do it 'returns validation error if personal access token miss some attributes' do
post api("/users/#{user.id}/personal_access_tokens", admin) post api("/users/#{user.id}/personal_access_tokens", admin)
...@@ -1238,7 +1248,8 @@ describe API::Users, api: true do ...@@ -1238,7 +1248,8 @@ describe API::Users, api: true do
post api("/users/#{user.id}/personal_access_tokens", admin), post api("/users/#{user.id}/personal_access_tokens", admin),
name: name, name: name,
expires_at: expires_at, expires_at: expires_at,
scopes: scopes scopes: scopes,
impersonation: impersonation
expect(response).to have_http_status(201) expect(response).to have_http_status(201)
...@@ -1252,12 +1263,14 @@ describe API::Users, api: true do ...@@ -1252,12 +1263,14 @@ describe API::Users, api: true do
expect(json_response['active']).to eq(false) expect(json_response['active']).to eq(false)
expect(json_response['revoked']).to eq(false) expect(json_response['revoked']).to eq(false)
expect(json_response['token']).to be_present expect(json_response['token']).to be_present
expect(PersonalAccessToken.find(personal_access_token_id)).not_to eq(nil) expect(json_response['impersonation']).to eq(impersonation)
expect(PersonalAccessToken.and_impersonation_tokens.find(personal_access_token_id)).not_to eq(nil)
end end
end end
describe 'DELETE /users/:id/personal_access_tokens/:personal_access_token_id' do describe 'DELETE /users/:id/personal_access_tokens/:personal_access_token_id' do
let!(:personal_access_token) { create(:personal_access_token, user: user, revoked: false) } let!(:personal_access_token) { create(:personal_access_token, user: user, revoked: false) }
let!(:impersonation_token) { create(:impersonation_personal_access_token, user: user, revoked: false) }
it 'returns a 404 error if user not found' do it 'returns a 404 error if user not found' do
delete api("/users/#{not_existing_user_id}/personal_access_tokens/1", admin) delete api("/users/#{not_existing_user_id}/personal_access_tokens/1", admin)
...@@ -1289,5 +1302,14 @@ describe API::Users, api: true do ...@@ -1289,5 +1302,14 @@ describe API::Users, api: true do
expect(json_response['revoked']).to eq(true) expect(json_response['revoked']).to eq(true)
expect(json_response['token']).to be_present expect(json_response['token']).to be_present
end end
it 'revokes an impersonation token' do
delete api("/users/#{user.id}/personal_access_tokens/#{impersonation_token.id}", admin)
expect(response).to have_http_status(200)
expect(impersonation_token.revoked).to eq(false)
expect(impersonation_token.reload.revoked).to eq(true)
expect(json_response['revoked']).to eq(true)
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment