From 81246e5649a8fb9e73369cbd117505a546d7e807 Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Tue, 27 Dec 2016 17:26:57 +0100 Subject: [PATCH] manage personal_access_tokens through api --- app/models/personal_access_token.rb | 5 + ...es_at_to_date_in_personal_access_tokens.rb | 18 +++ db/schema.rb | 2 +- lib/api/api.rb | 1 + lib/api/entities.rb | 12 ++ lib/api/personal_access_tokens.rb | 56 ++++++++ lib/api/users.rb | 64 +++++++++ .../profiles/personal_access_tokens_spec.rb | 4 +- spec/factories/personal_access_tokens.rb | 8 ++ spec/models/personal_access_token_spec.rb | 18 +++ .../api/personal_access_tokens_spec.rb | 107 ++++++++++++++ spec/requests/api/users_spec.rb | 135 ++++++++++++++++++ 12 files changed, 427 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20161228124936_change_expires_at_to_date_in_personal_access_tokens.rb create mode 100644 lib/api/personal_access_tokens.rb create mode 100644 spec/requests/api/personal_access_tokens_spec.rb diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb index 10a34c42fd8..5da98d7126a 100644 --- a/app/models/personal_access_token.rb +++ b/app/models/personal_access_token.rb @@ -1,4 +1,5 @@ class PersonalAccessToken < ActiveRecord::Base + include Expirable include TokenAuthenticatable add_authentication_token_field :token @@ -19,4 +20,8 @@ class PersonalAccessToken < ActiveRecord::Base self.revoked = true self.save end + + def active? + !revoked? && !expired? + end end diff --git a/db/migrate/20161228124936_change_expires_at_to_date_in_personal_access_tokens.rb b/db/migrate/20161228124936_change_expires_at_to_date_in_personal_access_tokens.rb new file mode 100644 index 00000000000..af1bac897cc --- /dev/null +++ b/db/migrate/20161228124936_change_expires_at_to_date_in_personal_access_tokens.rb @@ -0,0 +1,18 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class ChangeExpiresAtToDateInPersonalAccessTokens < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = true + DOWNTIME_REASON = 'This migration requires downtime because it alters expires_at column from datetime to date' + + def up + change_column :personal_access_tokens, :expires_at, :date + end + + def down + change_column :personal_access_tokens, :expires_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 1d94368f66e..82b4a931069 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -879,7 +879,7 @@ ActiveRecord::Schema.define(version: 20170216141440) do t.string "token", null: false t.string "name", null: false t.boolean "revoked", default: false - t.datetime "expires_at" + t.date "expires_at" t.datetime "created_at", null: false t.datetime "updated_at", null: false t.string "scopes", default: "--- []\n", null: false diff --git a/lib/api/api.rb b/lib/api/api.rb index b27ac3f1d15..a2ce03a901c 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -93,6 +93,7 @@ module API mount ::API::Namespaces mount ::API::Notes mount ::API::NotificationSettings + mount ::API::PersonalAccessTokens mount ::API::Pipelines mount ::API::ProjectHooks mount ::API::Projects diff --git a/lib/api/entities.rb b/lib/api/entities.rb index a99d9cadc8a..211353ef2a9 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -696,5 +696,17 @@ module API expose :id, :message, :starts_at, :ends_at, :color, :font expose :active?, as: :active end + + class BasicPersonalAccessToken < Grape::Entity + expose :id, :name, :revoked, :created_at, :scopes + expose :active?, as: :active + expose :expires_at do |personal_access_token| + personal_access_token.expires_at ? personal_access_token.expires_at.strftime("%Y-%m-%d") : nil + end + end + + class PersonalAccessToken < BasicPersonalAccessToken + expose :token + end end end diff --git a/lib/api/personal_access_tokens.rb b/lib/api/personal_access_tokens.rb new file mode 100644 index 00000000000..56797ddcf74 --- /dev/null +++ b/lib/api/personal_access_tokens.rb @@ -0,0 +1,56 @@ +module API + class PersonalAccessTokens < Grape::API + before { authenticate! } + + resource :personal_access_tokens do + desc 'Retrieve personal access tokens' + params do + optional :state, type: String, default: 'all', values: %w[all active inactive], desc: 'Filters (all|active|inactive) personal_access_tokens' + end + get do + personal_access_tokens = current_user.personal_access_tokens + + case params[:state] + when "active" + personal_access_tokens = personal_access_tokens.active + when "inactive" + personal_access_tokens = personal_access_tokens.inactive + end + + present personal_access_tokens, with: Entities::BasicPersonalAccessToken + end + + desc 'Create a personal access token' + params do + 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 :scopes, type: Array, desc: 'The array of scopes of the personal access token' + end + post do + parameters = declared_params(include_missing: false) + parameters[:user_id] = current_user.id + + personal_access_token = PersonalAccessToken.generate(parameters) + + if personal_access_token.save + present personal_access_token, with: Entities::PersonalAccessToken + else + render_validation_error!(personal_access_token) + end + end + + desc 'Revoke a personal access token' + params do + requires :personal_access_token_id, type: Integer, desc: 'The ID of the personal access token' + end + delete ':personal_access_token_id' do + personal_access_token = PersonalAccessToken.find_by(id: params[:personal_access_token_id], user_id: current_user.id) + not_found!('PersonalAccessToken') unless personal_access_token + + personal_access_token.revoke! + + present personal_access_token, with: Entities::BasicPersonalAccessToken + end + end + end +end diff --git a/lib/api/users.rb b/lib/api/users.rb index 7bb4b76f830..450d678061e 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -362,6 +362,70 @@ module API present paginate(events), with: Entities::Event end + + desc 'Retrieve personal access tokens. Available only for admins.' + params do + requires :user_id, type: Integer + optional :state, type: String, default: 'all', values: %w[all active inactive], desc: 'Filters (all|active|inactive) personal_access_tokens' + end + get ':user_id/personal_access_tokens' do + authenticated_as_admin! + + user = User.find_by(id: params[:user_id]) + not_found!('User') unless user + + personal_access_tokens = user.personal_access_tokens + + case params[:state] + when "active" + personal_access_tokens = personal_access_tokens.active + when "inactive" + personal_access_tokens = personal_access_tokens.inactive + end + + present personal_access_tokens, with: Entities::PersonalAccessToken + end + + desc 'Create a personal access token. Available only for admins.' + params do + requires :user_id, type: Integer, desc: 'The ID of the user' + 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 :scopes, type: Array, desc: 'The array of scopes of the personal access token' + end + post ':user_id/personal_access_tokens' do + authenticated_as_admin! + + user = User.find_by(id: params[:user_id]) + not_found!('User') unless user + + personal_access_token = PersonalAccessToken.generate(declared_params(include_missing: false)) + + if personal_access_token.save + present personal_access_token, with: Entities::PersonalAccessToken + else + render_validation_error!(personal_access_token) + end + end + + desc 'Revoke a personal access token. Available only for admins.' + params do + requires :user_id, type: Integer, desc: 'The ID of the user' + requires :personal_access_token_id, type: Integer, desc: 'The ID of the personal access token' + end + delete ':user_id/personal_access_tokens/:personal_access_token_id' do + authenticated_as_admin! + + user = User.find_by(id: params[:user_id]) + not_found!('User') unless user + + personal_access_token = PersonalAccessToken.find_by(id: params[:personal_access_token_id]) + not_found!('PersonalAccessToken') unless personal_access_token + + personal_access_token.revoke! + + present personal_access_token, with: Entities::PersonalAccessToken + end end resource :user do diff --git a/spec/controllers/profiles/personal_access_tokens_spec.rb b/spec/controllers/profiles/personal_access_tokens_spec.rb index 9d5f4c99f6d..96b3b7d1b24 100644 --- a/spec/controllers/profiles/personal_access_tokens_spec.rb +++ b/spec/controllers/profiles/personal_access_tokens_spec.rb @@ -22,12 +22,12 @@ describe Profiles::PersonalAccessTokensController do end it "allows creation of a token with an expiry date" do - expires_at = 5.days.from_now + expires_at = 5.days.from_now.to_date post :create, personal_access_token: { name: FFaker::Product.brand, expires_at: expires_at } expect(created_token).not_to be_nil - expect(created_token.expires_at.to_i).to eq(expires_at.to_i) + expect(created_token.expires_at).to eq(expires_at) end context "scopes" do diff --git a/spec/factories/personal_access_tokens.rb b/spec/factories/personal_access_tokens.rb index 811eab7e15b..3464d2d5d89 100644 --- a/spec/factories/personal_access_tokens.rb +++ b/spec/factories/personal_access_tokens.rb @@ -6,5 +6,13 @@ FactoryGirl.define do revoked false expires_at { 5.days.from_now } scopes ['api'] + + factory :revoked_personal_access_token do + revoked true + end + + factory :expired_personal_access_token do + expires_at { 1.day.ago } + end end end diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb index 46eb71cef14..c10c3bc3f31 100644 --- a/spec/models/personal_access_token_spec.rb +++ b/spec/models/personal_access_token_spec.rb @@ -12,4 +12,22 @@ describe PersonalAccessToken, models: true do expect(personal_access_token).not_to be_persisted end end + + describe ".active?" do + let(:active_personal_access_token) { build(:personal_access_token) } + let(:revoked_personal_access_token) { build(:revoked_personal_access_token) } + let(:expired_personal_access_token) { build(:expired_personal_access_token) } + + it "returns false if the personal_access_token is revoked" do + expect(revoked_personal_access_token).not_to be_active + end + + it "returns false if the personal_access_token is expired" do + expect(expired_personal_access_token).not_to be_active + end + + it "returns true if the personal_access_token is not revoked and not expired" do + expect(active_personal_access_token).to be_active + end + end end diff --git a/spec/requests/api/personal_access_tokens_spec.rb b/spec/requests/api/personal_access_tokens_spec.rb new file mode 100644 index 00000000000..7ffdabaeeeb --- /dev/null +++ b/spec/requests/api/personal_access_tokens_spec.rb @@ -0,0 +1,107 @@ +require 'spec_helper' + +describe API::PersonalAccessTokens, api: true do + include ApiHelpers + + let(:user) { create(:user) } + + describe "GET /personal_access_tokens" do + let!(:active_personal_access_token) { create(: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) } + + it 'returns an array of personal access tokens without exposing the token' do + get api("/personal_access_tokens", user) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.size).to eq(3) + + json_personal_access_token = json_response.detect do |personal_access_token| + personal_access_token['id'] == active_personal_access_token.id + end + + expect(json_personal_access_token['name']).to eq(active_personal_access_token.name) + expect(json_personal_access_token['token']).not_to be_present + end + + it 'returns an array of active personal access tokens if active is set to true' do + get api("/personal_access_tokens?state=active", user) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response).to all(include('active' => true)) + end + + it 'returns an array of inactive personal access tokens if active is set to false' do + get api("/personal_access_tokens?state=inactive", user) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response).to all(include('active' => false)) + end + end + + describe 'POST /personal_access_tokens' do + let(:name) { 'my new pat' } + let(:expires_at) { '2016-12-28' } + let(:scopes) { ['api', 'read_user'] } + + it 'returns validation error if personal access token miss some attributes' do + post api("/personal_access_tokens", user) + + expect(response).to have_http_status(400) + expect(json_response['error']).to eq('name is missing') + end + + it 'creates a personal access token' do + post api("/personal_access_tokens", user), + name: name, + expires_at: expires_at, + scopes: scopes + + expect(response).to have_http_status(201) + + personal_access_token_id = json_response['id'] + + expect(json_response['name']).to eq(name) + expect(json_response['scopes']).to eq(scopes) + expect(json_response['expires_at']).to eq(expires_at) + expect(json_response['id']).to be_present + expect(json_response['created_at']).to be_present + expect(json_response['active']).to eq(false) + expect(json_response['revoked']).to eq(false) + expect(json_response['token']).to be_present + expect(PersonalAccessToken.find(personal_access_token_id)).not_to eq(nil) + end + end + + describe 'DELETE /personal_access_tokens/:personal_access_token_id' do + let!(:personal_access_token) { create(:personal_access_token, user: user, revoked: false) } + let!(:personal_access_token_of_another_user) { create(:personal_access_token, revoked: false) } + + it 'returns a 404 error if personal access token not found' do + delete api("/personal_access_tokens/42", user) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 PersonalAccessToken Not Found') + end + + it 'returns a 404 error if personal access token exists but it is a personal access tokens of another user' do + delete api("/personal_access_tokens/#{personal_access_token_of_another_user.id}", user) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 PersonalAccessToken Not Found') + end + + it 'revokes a personal access token and does not expose token in the json response' do + delete api("/personal_access_tokens/#{personal_access_token.id}", user) + + expect(response).to have_http_status(200) + expect(personal_access_token.revoked).to eq(false) + expect(personal_access_token.reload.revoked).to eq(true) + expect(json_response['revoked']).to eq(true) + expect(json_response['token']).not_to be_present + end + end +end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index e5e4c84755f..5ed6adc09bc 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -10,6 +10,7 @@ describe API::Users, api: true do let(:omniauth_user) { create(:omniauth_user) } let(:ldap_user) { create(:omniauth_user, provider: 'ldapmain') } let(:ldap_blocked_user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') } + let(:not_existing_user_id) { (User.maximum('id') || 0 ) + 10 } describe "GET /users" do context "when unauthenticated" do @@ -1155,4 +1156,138 @@ describe API::Users, api: true do expect(json_response['message']).to eq('404 User Not Found') end end + + describe 'GET /users/:user_id/personal_access_tokens' do + let!(:active_personal_access_token) { create(: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) } + + it 'returns a 404 error if user not found' do + get api("/users/#{not_existing_user_id}/personal_access_tokens", admin) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 User Not Found') + end + + it 'returns a 403 error when authenticated as normal user' do + get api("/users/#{not_existing_user_id}/personal_access_tokens", user) + + expect(response).to have_http_status(403) + expect(json_response['message']).to eq('403 Forbidden') + end + + it 'returns an array of personal access tokens' do + get api("/users/#{user.id}/personal_access_tokens", admin) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.size).to eq(3) + expect(json_response.detect do |personal_access_token| + personal_access_token['id'] == active_personal_access_token.id + end['token']).to eq(active_personal_access_token.token) + end + + it 'returns an array of active personal access tokens if active is set to true' do + get api("/users/#{user.id}/personal_access_tokens?state=active", admin) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response).to all(include('active' => true)) + end + + it 'returns an array of inactive personal access tokens if active is set to false' do + get api("/users/#{user.id}/personal_access_tokens?state=inactive", admin) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response).to all(include('active' => false)) + end + end + + describe 'POST /users/:user_id/personal_access_tokens' do + let(:name) { 'my new pat' } + let(:expires_at) { '2016-12-28' } + let(:scopes) { ['api', 'read_user'] } + + it 'returns validation error if personal access token miss some attributes' do + post api("/users/#{user.id}/personal_access_tokens", admin) + + expect(response).to have_http_status(400) + expect(json_response['error']).to eq('name is missing') + end + + it 'returns a 404 error if user not found' do + post api("/users/#{not_existing_user_id}/personal_access_tokens", admin), + name: name, + expires_at: expires_at + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 User Not Found') + end + + it 'returns a 403 error when authenticated as normal user' do + post api("/users/#{user.id}/personal_access_tokens", user), + name: name, + expires_at: expires_at + + expect(response).to have_http_status(403) + expect(json_response['message']).to eq('403 Forbidden') + end + + it 'creates a personal access token' do + post api("/users/#{user.id}/personal_access_tokens", admin), + name: name, + expires_at: expires_at, + scopes: scopes + + expect(response).to have_http_status(201) + + personal_access_token_id = json_response['id'] + + expect(json_response['name']).to eq(name) + expect(json_response['scopes']).to eq(scopes) + expect(json_response['expires_at']).to eq(expires_at) + expect(json_response['id']).to be_present + expect(json_response['created_at']).to be_present + expect(json_response['active']).to eq(false) + expect(json_response['revoked']).to eq(false) + expect(json_response['token']).to be_present + expect(PersonalAccessToken.find(personal_access_token_id)).not_to eq(nil) + end + end + + describe 'DELETE /users/:id/personal_access_tokens/:personal_access_token_id' do + let!(:personal_access_token) { create(:personal_access_token, user: user, revoked: false) } + + it 'returns a 404 error if user not found' do + delete api("/users/#{not_existing_user_id}/personal_access_tokens/1", admin) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 User Not Found') + end + + it 'returns a 404 error if personal access token not found' do + delete api("/users/#{user.id}/personal_access_tokens/42", admin) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 PersonalAccessToken Not Found') + end + + it 'returns a 403 error when authenticated as normal user' do + delete api("/users/#{user.id}/personal_access_tokens/#{personal_access_token.id}", user) + + expect(response).to have_http_status(403) + expect(json_response['message']).to eq('403 Forbidden') + end + + it 'revokes a personal access token' do + delete api("/users/#{user.id}/personal_access_tokens/#{personal_access_token.id}", admin) + + expect(response).to have_http_status(200) + expect(personal_access_token.revoked).to eq(false) + expect(personal_access_token.reload.revoked).to eq(true) + expect(json_response['revoked']).to eq(true) + expect(json_response['token']).to be_present + end + end end -- GitLab