apply codestyle and implementation changes to the respective feature code

app/assets/stylesheets/pages/settings.scss

@@ -25,7 +25,7 @@
   padding-top: 0;
 }
 
-.impersonation-token-token-container {
+.token-token-container {
   #impersonation-token-token {
     width: 80%;
     display: inline;

app/controllers/admin/impersonation_tokens_controller.rb

 class Admin::ImpersonationTokensController < Admin::ApplicationController
-  before_action :user, :finder
+  before_action :user
 
   def index
     set_index_vars
   end
 
   def create
-    @impersonation_token = finder.execute.build(impersonation_token_params)
+    @impersonation_token = finder.build(impersonation_token_params)
 
     if @impersonation_token.save
       flash[:impersonation_token] = @impersonation_token.token
@@ -18,7 +18,7 @@ class Admin::ImpersonationTokensController < Admin::ApplicationController
   end
 
   def revoke
-    @impersonation_token = finder.execute(id: params[:id])
+    @impersonation_token = finder.find(params[:id])
 
     if @impersonation_token.revoke!
       flash[:notice] = "Revoked impersonation token #{@impersonation_token.name}!"
@@ -35,8 +35,8 @@ class Admin::ImpersonationTokensController < Admin::ApplicationController
     @user ||= User.find_by!(username: params[:user_id])
   end
 
-  def finder
-    @finder ||= PersonalAccessTokensFinder.new(user: user, impersonation: true)
+  def finder(options = {})
+    PersonalAccessTokensFinder.new({ user: user, impersonation: true }.merge(options))
   end
 
   def impersonation_token_params
@@ -44,12 +44,10 @@ class Admin::ImpersonationTokensController < Admin::ApplicationController
   end
 
   def set_index_vars
-    finder.params[:state] = 'active'
-    @impersonation_token ||= finder.execute.build
     @scopes = Gitlab::Auth::SCOPES
-    finder.params[:order] = :expires_at
-    @active_impersonation_tokens = finder.execute
-    finder.params[:state] = 'inactive'
-    @inactive_impersonation_tokens = finder.execute
+
+    @impersonation_token ||= finder.build
+    @inactive_impersonation_tokens = finder(state: 'inactive').execute
+    @active_impersonation_tokens = finder(state: 'active').execute.order(:expires_at)
   end
 end

app/controllers/profiles/personal_access_tokens_controller.rb

 class Profiles::PersonalAccessTokensController < Profiles::ApplicationController
-  before_action :finder
-
   def index
     set_index_vars
   end
 
   def create
-    @personal_access_token = finder.execute.build(personal_access_token_params)
+    @personal_access_token = finder.build(personal_access_token_params)
 
     if @personal_access_token.save
       flash[:personal_access_token] = @personal_access_token.token
@@ -18,7 +16,7 @@ class Profiles::PersonalAccessTokensController < Profiles::ApplicationController
   end
 
   def revoke
-    @personal_access_token = finder.execute(id: params[:id])
+    @personal_access_token = finder.find(params[:id])
 
     if @personal_access_token.revoke!
       flash[:notice] = "Revoked personal access token #{@personal_access_token.name}!"
@@ -31,8 +29,8 @@ class Profiles::PersonalAccessTokensController < Profiles::ApplicationController
 
   private
 
-  def finder
-    @finder ||= PersonalAccessTokensFinder.new(user: current_user, impersonation: false)
+  def finder(options = {})
+    PersonalAccessTokensFinder.new({ user: current_user, impersonation: false }.merge(options))
   end
 
   def personal_access_token_params
@@ -40,12 +38,10 @@ class Profiles::PersonalAccessTokensController < Profiles::ApplicationController
   end
 
   def set_index_vars
-    finder.params[:state] = 'active'
-    @personal_access_token ||= finder.execute.build
     @scopes = Gitlab::Auth::SCOPES
-    finder.params[:order] = :expires_at
-    @active_personal_access_tokens = finder.execute
-    finder.params[:state] = 'inactive'
-    @inactive_personal_access_tokens = finder.execute
+
+    @personal_access_token = finder.build
+    @inactive_personal_access_tokens = finder(state: 'inactive').execute
+    @active_personal_access_tokens = finder(state: 'active').execute.order(:expires_at)
   end
 end

app/finders/personal_access_tokens_finder.rb

 class PersonalAccessTokensFinder
   attr_accessor :params
 
+  delegate :build, :find, :find_by, to: :execute
+
   def initialize(params = {})
     @params = params
   end
 
-  def execute(token: nil, id: nil)
-    tokens = by_impersonation
-
-    return tokens.find_by_token(token) if token
-    return tokens.find_by_id(id) if id
-
-    tokens = by_state(tokens)
-    tokens.order(@params[:order]) if @params[:order]
-
-    tokens
+  def execute
+    tokens = PersonalAccessToken.all
+    tokens = by_user(tokens)
+    tokens = by_impersonation(tokens)
+    by_state(tokens)
   end
 
   private
 
-  def personal_access_tokens
-    @params[:user] ? @params[:user].personal_access_tokens : PersonalAccessToken.all
+  def by_user(tokens)
+    return tokens unless @params[:user]
+    tokens.where(user: @params[:user])
   end
 
-  def by_impersonation
+  def by_impersonation(tokens)
     case @params[:impersonation]
     when true
-      personal_access_tokens.with_impersonation
+      tokens.with_impersonation
     when false
-      personal_access_tokens.without_impersonation
+      tokens.without_impersonation
     else
-      personal_access_tokens
+      tokens
     end
   end
 

app/models/user.rb

@@ -322,8 +322,7 @@ class User < ActiveRecord::Base
     end
 
     def find_by_personal_access_token(token_string)
-      personal_access_token = PersonalAccessToken.active.find_by_token(token_string) if token_string
-      personal_access_token&.user
+      PersonalAccessTokensFinder.new(state: 'active').find_by(token: token_string)&.user
     end
 
     # Returns a user for the given SSH key.

app/views/admin/impersonation_tokens/index.html.haml

@@ -3,63 +3,6 @@
 
 .row.prepend-top-default
   .col-lg-12
-    %h5.prepend-top-0
-      Add a Impersonation Token
-    %p.profile-settings-content
-      Pick a name for the application, and we'll give the respective user a unique token.
-    = render "shared/personal_access_tokens_form", path: admin_user_impersonation_tokens_path, impersonation: true, personal_access_token: @impersonation_token, scopes: @scopes
+    = render "shared/personal_access_tokens_form", path: admin_user_impersonation_tokens_path, impersonation: true, token: @impersonation_token, scopes: @scopes
 
-    %hr
-
-    %h5 Active Impersonation Tokens (#{@active_impersonation_tokens.length})
-    %p.profile-settings-content
-      To see all the user's personal access tokens you must impersonate first
-    - if @active_impersonation_tokens.present?
-      .table-responsive
-        %table.table.active-impersonation-tokens
-          %thead
-            %tr
-              %th Name
-              %th Created
-              %th Expires
-              %th Scopes
-              %th Token
-              %th
-          %tbody
-            - @active_impersonation_tokens.each do |impersonation_token|
-              %tr
-                %td= impersonation_token.name
-                %td= impersonation_token.created_at.to_date.to_s(:medium)
-                %td
-                  - if impersonation_token.expires?
-                    %span{ class: ('text-warning' if impersonation_token.expires_soon?) }
-                      In #{distance_of_time_in_words_to_now(impersonation_token.expires_at)}
-                  - else
-                    %span.impersonation_tokens-never-expires-label Never
-                %td= impersonation_token.scopes.present? ? impersonation_token.scopes.join(", ") : "<no scopes selected>"
-                %td.impersonation-token-token-container
-                  = text_field_tag 'impersonation-token-token', impersonation_token.token, readonly: true, class: "form-control"
-                  = clipboard_button(clipboard_text: impersonation_token.token)
-                %td= link_to "Revoke", revoke_admin_user_impersonation_token_path(id: impersonation_token.id, user_id: impersonation_token.user.username), method: :put, class: "btn btn-danger pull-right", data: { confirm: "Are you sure you want to revoke this impersonation token? This action cannot be undone." }
-    - else
-      .settings-message.text-center
-        This user has no active impersonation tokens.
-
-    %hr
-
-    %h5 Inactive Impersonation Tokens (#{@inactive_impersonation_tokens.length})
-    - if @inactive_impersonation_tokens.present?
-      .table-responsive
-        %table.table.inactive-impersonation-tokens
-          %thead
-            %tr
-              %th Name
-              %th Created
-          %tbody
-            - @inactive_impersonation_tokens.each do |token|
-              %tr
-                %td= token.name
-                %td= token.created_at.to_date.to_s(:medium)
-    - else
-      .settings-message.text-center
-        This user has no inactive impersonation tokens.
+    = render "shared/personal_access_tokens_table", impersonation: true, active_tokens: @active_impersonation_tokens, inactive_tokens: @inactive_impersonation_tokens

app/views/admin/users/_head.html.haml

@@ -22,5 +22,5 @@
   = nav_link(controller: :identities) do
     = link_to "Identities", admin_user_identities_path(@user)
   = nav_link(controller: :impersonation_tokens) do
-    = link_to "Access Tokens", admin_user_impersonation_tokens_path(@user)
+    = link_to "Impersonation Tokens", admin_user_impersonation_tokens_path(@user)
 .append-bottom-default

app/views/profiles/personal_access_tokens/index.html.haml

@@ -24,66 +24,9 @@
 
       %hr
 
-    %h5.prepend-top-0
-      Add a Personal Access Token
-    %p.profile-settings-content
-      Pick a name for the application, and we'll give you a unique token.
-
-    = render "shared/personal_access_tokens_form", path: profile_personal_access_tokens_path, personal_access_token: @personal_access_token, scopes: @scopes
-
-    %hr
-
-    %h5 Active Personal Access Tokens (#{@active_personal_access_tokens.length})
-
-    - if @active_personal_access_tokens.present?
-      .table-responsive
-        %table.table.active-personal-access-tokens
-          %thead
-            %tr
-              %th Name
-              %th Created
-              %th Expires
-              %th Scopes
-              %th
-          %tbody
-            - @active_personal_access_tokens.each do |token|
-              %tr
-                %td= token.name
-                %td= token.created_at.to_date.to_s(:medium)
-                %td
-                  - if token.expires?
-                    %span{ class: ('text-warning' if token.expires_soon?) }
-                      In #{distance_of_time_in_words_to_now(token.expires_at)}
-                  - else
-                    %span.personal-access-tokens-never-expires-label Never
-                %td= token.scopes.present? ? token.scopes.join(", ") : "<no scopes selected>"
-                %td= link_to "Revoke", revoke_profile_personal_access_token_path(token), method: :put, class: "btn btn-danger pull-right", data: { confirm: "Are you sure you want to revoke this token? This action cannot be undone." }
-
-    - else
-      .settings-message.text-center
-        You don't have any active tokens yet.
-
-    %hr
-
-    %h5 Inactive Personal Access Tokens (#{@inactive_personal_access_tokens.length})
-
-    - if @inactive_personal_access_tokens.present?
-      .table-responsive
-        %table.table.inactive-personal-access-tokens
-          %thead
-            %tr
-              %th Name
-              %th Created
-          %tbody
-            - @inactive_personal_access_tokens.each do |token|
-              %tr
-                %td= token.name
-                %td= token.created_at.to_date.to_s(:medium)
-
-    - else
-      .settings-message.text-center
-        There are no inactive tokens.
+    = render "shared/personal_access_tokens_form", path: profile_personal_access_tokens_path, impersonation: false, token: @personal_access_token, scopes: @scopes
 
+    = render "shared/personal_access_tokens_table", impersonation: false, active_tokens: @active_personal_access_tokens, inactive_tokens: @inactive_personal_access_tokens
 
 :javascript
   $("#created-personal-access-token").click(function() {

app/views/shared/_personal_access_tokens_form.html.haml

-- impersonation = impersonation || false
-- personal_access_token = local_assigns.fetch(:personal_access_token)
-- scopes = local_assigns.fetch(:scopes)
+- type = impersonation ? "Impersonation" : "Personal Access"
 
-= form_for personal_access_token, url: path, method: :post, html: { class: 'js-requires-input' } do |f|
+%h5.prepend-top-0
+  Add a #{type} Token
+%p.profile-settings-content
+  Pick a name for the application, and we'll give you a unique #{type} Token.
 
-  = form_errors(personal_access_token)
+= form_for token, url: path, method: :post, html: { class: 'js-requires-input' } do |f|
+
+  = form_errors(token)
 
   .form-group
     = f.label :name, class: 'label-light'
@@ -16,10 +19,9 @@
 
   .form-group
     = f.label :scopes, class: 'label-light'
-    = render 'shared/tokens/scopes_form', prefix: 'personal_access_token', token: personal_access_token, scopes: scopes
+    = render 'shared/tokens/scopes_form', prefix: 'personal_access_token', token: token, scopes: scopes
 
   .prepend-top-default
-    - type = impersonation ? "Impersonation" : "Personal Access"
     = f.submit "Create #{type} Token", class: "btn btn-create"
 
 :javascript

app/views/shared/_personal_access_tokens_table.html.haml

+- type = impersonation ? "Impersonation" : "Personal Access"
+%hr
+
+%h5 Active #{type} Tokens (#{active_tokens.length})
+- if impersonation
+  %p.profile-settings-content
+    To see all the user's personal access tokens you must impersonate them first.
+
+- if active_tokens.present?
+  .table-responsive
+    %table.table.active-tokens
+      %thead
+        %tr
+          %th Name
+          %th Created
+          %th Expires
+          %th Scopes
+          - if impersonation
+            %th Token
+          %th
+      %tbody
+        - active_tokens.each do |token|
+          %tr
+            %td= token.name
+            %td= token.created_at.to_date.to_s(:medium)
+            %td
+              - if token.expires?
+                %span{ class: ('text-warning' if token.expires_soon?) }
+                  In #{distance_of_time_in_words_to_now(token.expires_at)}
+              - else
+                %span.token-never-expires-label Never
+            %td= token.scopes.present? ? token.scopes.join(", ") : "<no scopes selected>"
+            - if impersonation
+              %td.token-token-container
+                = text_field_tag 'impersonation-token-token', token.token, readonly: true, class: "form-control"
+                = clipboard_button(clipboard_text: token.token)
+            - path = impersonation ? revoke_admin_user_impersonation_token_path(token.user, token) : revoke_profile_personal_access_token_path(token)
+            %td= link_to "Revoke", path, method: :put, class: "btn btn-danger pull-right", data: { confirm: "Are you sure you want to revoke this #{type} Token? This action cannot be undone." }
+- else
+  .settings-message.text-center
+    This user has no active #{type} Tokens.
+
+%hr
+
+%h5 Inactive #{type} Tokens (#{inactive_tokens.length})
+- if inactive_tokens.present?
+  .table-responsive
+    %table.table.inactive-tokens
+      %thead
+        %tr
+          %th Name
+          %th Created
+      %tbody
+        - inactive_tokens.each do |token|
+          %tr
+            %td= token.name
+            %td= token.created_at.to_date.to_s(:medium)
+- else
+  .settings-message.text-center
+    This user has no inactive #{type} Tokens.

db/migrate/20161228135550_add_impersonation_to_personal_access_tokens.rb

@@ -9,7 +9,7 @@ class AddImpersonationToPersonalAccessTokens < ActiveRecord::Migration
   DOWNTIME = false
 
   def up
-    add_column_with_default :personal_access_tokens, :impersonation, :boolean, default: false, null: false
+    add_column_with_default :personal_access_tokens, :impersonation, :boolean, default: false, allow_null: false
   end
 
   def down

doc/api/README.md

@@ -8,7 +8,6 @@ under [`/lib/api`](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/lib/api).
 Documentation for various API resources can be found separately in the
 following locations:
 
-- [Personal Access Tokens](personal_access_tokens.md)
 - [Award Emoji](award_emoji.md)
 - [Branches](branches.md)
 - [Broadcast Messages](broadcast_messages.md)
@@ -222,6 +221,14 @@ GET /projects?private_token=9koXpg98eAheJpvBs5tK&sudo=23
 curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --header "SUDO: 23" "https://gitlab.example.com/api/v3/projects"
 ```
 
+## Impersonation Tokens
+
+Impersonation Tokens are a type of Personal Access Token that can only be created by an admin for a specific user. These can be used by automated tools
+to authenticate with the API as a specific user, as a better alternative to using the user's password or private token directly, which may change over time,
+and to using the [Sudo](#sudo) feature, which requires the tool to know an admin's password or private token, which can change over time as well and are extremely powerful.
+
+For more information about the usage please refer to the [Users](users.md) page
+
 ## Pagination
 
 Sometimes the returned result will span across many pages. When listing

doc/api/personal_access_tokens.md

-# Personal Access Token
-
-## List
-
-This function takes pagination parameters `page` and `per_page` to restrict the list of personal access tokens.
-
-```
-GET /personal_access_tokens
-```
-
-Parameters:
-
-| Attribute | Type | Required | Description |
-| --------- | ---- | -------- | ----------- |
-| `state`   | string | no | filter tokens based on state (all, active, inactive) |
-
-Example response:
-```json
-[
-  {
-    "id": 1,
-    "name": "mytoken",
-    "revoked": false,
-    "expires_at": "2017-01-04",
-    "scopes": ["api"],
-    "active": true
-  }
-]
-```
-
-## Show
-
-```
-GET /personal_access_tokens/:personal_access_token_id
-```
-
-Parameters:
-
-| Attribute | Type | Required | Description |
-| --------- | ---- | -------- | ----------- |
-| `personal_access_token_id` | integer | yes | The ID of the personal access token |
-
-## Create
-
-```
-POST /personal_access_tokens
-```
-
-It responds with the new personal access token for the current user.
-
-Parameters:
-
-| Attribute | Type | Required | Description |
-| --------- | ---- | -------- | ----------- |
-| `name` | string | yes | The name of the personal access token |
-| `expires_at` | date | no | The expiration date of the personal access token |
-| `scopes` | array | no | The array of scopes of the personal access token |
-
-## Revoke
-
-```
-DELETE /personal_access_tokens/:personal_access_token_id
-```
-
-Parameters:
-
-| Attribute | Type | Required | Description |
-| --------- | ---- | -------- | ----------- |
-| `personal_access_token_id` | integer | yes | The ID of the personal access token |

doc/api/users.md

@@ -828,22 +828,21 @@ Example response:
 ]
 ```
 
-## Retrieve user personal access tokens
+## Retrieve user impersonation tokens
 
-It retrieves every personal access token of the user. Note that only administrators can do this.
-This function takes pagination parameters `page` and `per_page` to restrict the list of personal access tokens.
+It retrieves every impersonation token of the user. Note that only administrators can do this.
+This function takes pagination parameters `page` and `per_page` to restrict the list of impersonation tokens.
 
 ```
-GET /users/:id/personal_access_tokens
+GET /users/:user_id/impersonation_tokens
 ```
 
 Parameters:
 
 | Attribute | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
-| `id` | integer | yes | The ID of the user |
+| `user_id` | integer | yes | The ID of the user |
 | `state`   | string | no | filter tokens based on state (all, active, inactive) |
-| `impersonation` | boolean | no | The impersonation flag of the personal access token |
 
 Example response:
 ```json
@@ -861,53 +860,66 @@ Example response:
 ]
 ```
 
-## Show a user personal access token
+## Show a user's impersonation token
 
-It shows a user's personal access token. Note that only administrators can do this.
+It shows a user's impersonation token. Note that only administrators can do this.
 
 ```
-GET /users/:id/personal_access_tokens/:personal_access_token_id
+GET /users/:user_id/impersonation_tokens/:impersonation_token_id
 ```
 
 Parameters:
 
 | Attribute | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
-| `id` | integer | yes | The ID of the user |
-| `personal_access_token_id` | integer | yes | The ID of the personal access token |
+| `user_id` | integer | yes | The ID of the user |
+| `impersonation_token_id` | integer | yes | The ID of the impersonation token |
 
-## Create a personal access token
+## Create a impersonation token
 
-It creates a new personal access token. Note that only administrators can do this.
+It creates a new impersonation token. Note that only administrators can do this.
 You are only able to create impersonation tokens to impersonate the user and perform
 both API calls and Git reads and writes. The user will not see these tokens in his profile
 settings page.
 
 ```
-POST /users/:id/personal_access_tokens
+POST /users/:user_id/impersonation_tokens
 ```
 
 Parameters:
 
 | Attribute | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
-| `id` | integer | yes | The ID of the user |
-| `name` | string | yes | The name of the personal access token |
-| `expires_at` | date | no | The expiration date of the personal access token |
-| `scopes` | array | no | The array of scopes of the personal access token |
-| `impersonation` | boolean | no | The impersonation flag of the personal access token |
+| `user_id` | integer | yes | The ID of the user |
+| `name` | string | yes | The name of the impersonation token |
+| `expires_at` | date | no | The expiration date of the impersonation token |
+| `scopes` | array | no | The array of scopes of the impersonation token (api, read_user) |
+
+Example response:
+```json
+{
+  "id": 1,
+  "name": "mytoken",
+  "revoked": false,
+  "expires_at": "2017-01-04",
+  "scopes": ['api'],
+  "active": true,
+  "impersonation": true,
+  "token": "9koXpg98eAheJpvBs5tK"
+}
+```
 
-## Revoke a personal access token
+## Revoke an impersonation token
 
-It revokes a personal access token. Note that only administrators can revoke impersonation tokens.
+It revokes an impersonation token. Note that only administrators can revoke impersonation tokens.
 
 ```
-DELETE /users/:id/personal_access_tokens/:personal_access_token_id
+DELETE /users/:user_id/impersonation_tokens/:impersonation_token_id
 ```
 
 Parameters:
 
 | Attribute | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
-| `id` | integer | yes | The ID of the user |
-| `personal_access_token_id` | integer | yes | The ID of the personal access token |
+| `user_id` | integer | yes | The ID of the user |
+| `impersonation_token_id` | integer | yes | The ID of the impersonation token |

lib/api/api.rb

@@ -93,7 +93,6 @@ module API
     mount ::API::Namespaces
     mount ::API::Notes
     mount ::API::NotificationSettings
-    mount ::API::PersonalAccessTokens
     mount ::API::Pipelines
     mount ::API::ProjectHooks
     mount ::API::Projects

lib/api/personal_access_tokens.rb

-module API
-  class PersonalAccessTokens < Grape::API
-    include PaginationParams
-
-    before do
-      authenticate!
-      @finder = PersonalAccessTokensFinder.new(user: current_user, impersonation: false)
-    end
-
-    resource :personal_access_tokens do
-      desc 'Retrieve personal access tokens' do
-        detail 'This feature was introduced in GitLab 9.0'
-        success Entities::PersonalAccessToken
-      end
-      params do
-        optional :state, type: String, default: 'all', values: %w[all active inactive], desc: 'Filters (all|active|inactive) personal_access_tokens'
-        use :pagination
-      end
-      get do
-        @finder.params.merge!(declared_params(include_missing: false))
-        present paginate(@finder.execute), with: Entities::PersonalAccessToken
-      end
-
-      desc 'Retrieve personal access token' do
-        detail 'This feature was introduced in GitLab 9.0'
-        success Entities::PersonalAccessToken
-      end
-      params do
-        requires :personal_access_token_id, type: Integer, desc: 'The ID of the personal access token'
-      end
-      get ':personal_access_token_id' do
-        personal_access_token = @finder.execute(id: declared_params[:personal_access_token_id])
-        not_found!('Personal Access Token') unless personal_access_token
-
-        present personal_access_token, with: Entities::PersonalAccessToken
-      end
-
-      desc 'Create a personal access token' do
-        detail 'This feature was introduced in GitLab 9.0'
-        success Entities::PersonalAccessTokenWithToken
-      end
-      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
-        personal_access_token = @finder.execute.build(declared_params(include_missing: false))
-
-        if personal_access_token.save
-          present personal_access_token, with: Entities::PersonalAccessTokenWithToken
-        else
-          render_validation_error!(personal_access_token)
-        end
-      end
-
-      desc 'Revoke a personal access token' do
-        detail 'This feature was introduced in GitLab 9.0'
-      end
-      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 = @finder.execute(id: declared_params[:personal_access_token_id])
-        not_found!('Personal Access Token') unless personal_access_token
-
-        personal_access_token.revoke!
-      end
-    end
-  end
-end

lib/api/users.rb

@@ -10,8 +10,8 @@ module API
     resource :users, requirements: { uid: /[0-9]*/, id: /[0-9]*/ } do
       helpers do
         def find_user(params)
-          user = User.find_by(id: params[:id])
-          user ? user : not_found!('User')
+          id = params[:user_id] || params[:id]
+          User.find_by(id: id) || not_found!('User')
         end
 
         params :optional_attributes do
@@ -369,75 +369,71 @@ module API
       end
 
       params do
-        requires :id, type: Integer, desc: 'The ID of the user'
+        requires :user_id, type: Integer, desc: 'The ID of the user'
       end
-      segment ':id' do
-        resource :personal_access_tokens do
-          before do
-            authenticated_as_admin!
+      segment ':user_id' do
+        resource :impersonation_tokens do
+          helpers do
+            def finder(options = {})
               user = find_user(params)
-            @finder = PersonalAccessTokensFinder.new(user: user)
+              PersonalAccessTokensFinder.new({ user: user, impersonation: true }.merge(options))
             end
 
-          desc 'Retrieve personal access tokens. Available only for admins.' do
+            def find_impersonation_token
+              finder.find_by(id: declared_params[:impersonation_token_id]) || not_found!('Impersonation Token')
+            end
+          end
+
+          before { authenticated_as_admin! }
+
+          desc 'Retrieve impersonation tokens. Available only for admins.' do
             detail 'This feature was introduced in GitLab 9.0'
             success Entities::ImpersonationToken
           end
           params do
-            optional :state, type: String, default: 'all', values: %w[all active inactive], desc: 'Filters (all|active|inactive) personal_access_tokens'
-            optional :impersonation, type: Boolean, desc: 'Filters only impersonation personal_access_tokens'
             use :pagination
+            optional :state, type: String, default: 'all', values: %w[all active inactive], desc: 'Filters (all|active|inactive) impersonation_tokens'
           end
-          get do
-            @finder.params.merge!(declared_params(include_missing: false))
-            present paginate(@finder.execute), with: Entities::ImpersonationToken
-          end
+          get { present paginate(finder(declared_params(include_missing: false)).execute), with: Entities::ImpersonationToken }
 
-          desc 'Create a personal access token. Available only for admins.' do
+          desc 'Create a impersonation token. Available only for admins.' do
             detail 'This feature was introduced in GitLab 9.0'
             success Entities::ImpersonationToken
           end
           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'
-            optional :impersonation, type: Boolean, desc: 'The impersonation flag of the personal access token'
+            requires :name, type: String, desc: 'The name of the impersonation token'
+            optional :expires_at, type: Date, desc: 'The expiration date in the format YEAR-MONTH-DAY of the impersonation token'
+            optional :scopes, type: Array, desc: 'The array of scopes of the impersonation token'
           end
           post do
-            personal_access_token = @finder.execute.build(declared_params(include_missing: false))
+            impersonation_token = finder.build(declared_params(include_missing: false))
 
-            if personal_access_token.save
-              present personal_access_token, with: Entities::ImpersonationToken
+            if impersonation_token.save
+              present impersonation_token, with: Entities::ImpersonationToken
             else
-              render_validation_error!(personal_access_token)
+              render_validation_error!(impersonation_token)
             end
           end
 
-          desc 'Retrieve personal access token. Available only for admins.' do
+          desc 'Retrieve impersonation token. Available only for admins.' do
             detail 'This feature was introduced in GitLab 9.0'
             success Entities::ImpersonationToken
           end
           params do
-            requires :personal_access_token_id, type: Integer, desc: 'The ID of the personal access token'
+            requires :impersonation_token_id, type: Integer, desc: 'The ID of the impersonation token'
           end
-          get ':personal_access_token_id' do
-            personal_access_token = @finder.execute(id: declared_params[:personal_access_token_id])
-            not_found!('Personal Access Token') unless personal_access_token
-
-            present personal_access_token, with: Entities::ImpersonationToken
+          get ':impersonation_token_id' do
+            present find_impersonation_token, with: Entities::ImpersonationToken
           end
 
-          desc 'Revoke a personal access token. Available only for admins.' do
+          desc 'Revoke a impersonation token. Available only for admins.' do
             detail 'This feature was introduced in GitLab 9.0'
           end
           params do
-            requires :personal_access_token_id, type: Integer, desc: 'The ID of the personal access token'
+            requires :impersonation_token_id, type: Integer, desc: 'The ID of the impersonation token'
           end
-          delete ':personal_access_token_id' do
-            personal_access_token = @finder.execute(id: declared_params[:personal_access_token_id])
-            not_found!('Personal Access Token') unless personal_access_token
-
-            personal_access_token.revoke!
+          delete ':impersonation_token_id' do
+            find_impersonation_token.revoke!
           end
         end
       end

lib/gitlab/auth.rb

@@ -105,7 +105,7 @@ module Gitlab
       def personal_access_token_check(password)
         return unless password.present?
 
-        token = PersonalAccessTokensFinder.new(state: 'active').execute(token: password)
+        token = PersonalAccessTokensFinder.new(state: 'active').find_by(token: password)
 
         if token && valid_api_token?(token)
           Gitlab::Auth::Result.new(token.user, nil, :personal_token, full_authentication_abilities)

spec/controllers/profiles/personal_access_tokens_spec.rb

@@ -49,8 +49,8 @@ describe Profiles::PersonalAccessTokensController do
 
   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) }
+    let!(:inactive_personal_access_token) { create(:personal_access_token, :revoked, user: user) }
+    let!(:impersonation_personal_access_token) { create(:personal_access_token, :impersonation, user: user) }
 
     before { get :index }
 

spec/factories/personal_access_tokens.rb

@@ -8,16 +8,20 @@ FactoryGirl.define do
     scopes ['api']
     impersonation false
 
-    factory :revoked_personal_access_token do
+    trait :impersonation do
+      impersonation true
+    end
+
+    trait :revoked do
       revoked true
     end
 
-    factory :expired_personal_access_token do
+    trait :expired do
       expires_at { 1.day.ago }
     end
 
-    factory :impersonation_personal_access_token do
-      impersonation true
+    trait :invalid do
+      token nil
     end
   end
 end

spec/features/admin/admin_users_impersonation_tokens_spec.rb

@@ -5,11 +5,11 @@ describe 'Admin > Users > Impersonation Tokens', feature: true, js: true do
   let!(:user) { create(:user) }
 
   def active_impersonation_tokens
-    find(".table.active-impersonation-tokens")
+    find(".table.active-tokens")
   end
 
   def inactive_impersonation_tokens
-    find(".table.inactive-impersonation-tokens")
+    find(".table.inactive-tokens")
   end
 
   before { login_as(admin) }
@@ -30,7 +30,7 @@ describe 'Admin > Users > Impersonation Tokens', feature: true, js: true do
       check "api"
       check "read_user"
 
-      expect { click_on "Create Impersonation Token" }.to change { PersonalAccessToken.with_impersonation.count }
+      expect { click_on "Create Impersonation Token" }.to change { PersonalAccessTokensFinder.new(impersonation: true).execute.count }
       expect(active_impersonation_tokens).to have_text(name)
       expect(active_impersonation_tokens).to have_text('In')
       expect(active_impersonation_tokens).to have_text('api')
@@ -39,7 +39,7 @@ describe 'Admin > Users > Impersonation Tokens', feature: true, js: true do
   end
 
   describe 'active tokens' do
-    let!(:impersonation_token) { create(:impersonation_personal_access_token, user: user) }
+    let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) }
     let!(:personal_access_token) { create(:personal_access_token, user: user) }
 
     it 'only shows impersonation tokens' do
@@ -51,7 +51,7 @@ describe 'Admin > Users > Impersonation Tokens', feature: true, js: true do
   end
 
   describe "inactive tokens" do
-    let!(:impersonation_token) { create(:impersonation_personal_access_token, user: user) }
+    let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) }
 
     it "allows revocation of an active impersonation token" do
       visit admin_user_impersonation_tokens_path(user_id: user.username)

spec/features/profiles/personal_access_tokens_spec.rb

@@ -4,11 +4,11 @@ describe 'Profile > Personal Access Tokens', feature: true, js: true do
   let(:user) { create(:user) }
 
   def active_personal_access_tokens
-    find(".table.active-personal-access-tokens")
+    find(".table.active-tokens")
   end
 
   def inactive_personal_access_tokens
-    find(".table.inactive-personal-access-tokens")
+    find(".table.inactive-tokens")
   end
 
   def created_personal_access_token
@@ -26,7 +26,7 @@ describe 'Profile > Personal Access Tokens', feature: true, js: true do
   end
 
   describe "token creation" do
-    it "allows creation of a non impersonation token" do
+    it "allows creation of a personal access token" do
       name = FFaker::Product.brand
 
       visit profile_personal_access_tokens_path
@@ -61,10 +61,10 @@ describe 'Profile > Personal Access Tokens', feature: true, js: true do
   end
 
   describe 'active tokens' do
-    let!(:impersonation_token) { create(:impersonation_personal_access_token, user: user) }
+    let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) }
     let!(:personal_access_token) { create(:personal_access_token, user: user) }
 
-    it 'only shows non impersonated tokens' do
+    it 'only shows personal access tokens' do
       visit profile_personal_access_tokens_path
 
       expect(active_personal_access_tokens).to have_text(personal_access_token.name)

spec/finders/personal_access_tokens_finder_spec.rb

 require 'spec_helper'
 
 describe PersonalAccessTokensFinder do
+  def finder(options = {})
+    described_class.new(options)
+  end
+
   describe '#execute' do
     let(:user) { create(:user) }
+    let(:params) { {} }
     let!(:active_personal_access_token) { create(:personal_access_token, user: user) }
-    let!(:expired_personal_access_token) { create(:expired_personal_access_token, user: user) }
-    let!(:revoked_personal_access_token) { create(:revoked_personal_access_token, user: user) }
-    let!(:active_impersonation_token) { create(:impersonation_personal_access_token, user: user, impersonation: true) }
-    let!(:expired_impersonation_token) { create(:expired_personal_access_token, user: user, impersonation: true) }
-    let!(:revoked_impersonation_token) { create(:revoked_personal_access_token, user: user, impersonation: true) }
+    let!(:expired_personal_access_token) { create(:personal_access_token, :expired, user: user) }
+    let!(:revoked_personal_access_token) { create(:personal_access_token, :revoked, user: user) }
+    let!(:active_impersonation_token) { create(:personal_access_token, :impersonation, user: user) }
+    let!(:expired_impersonation_token) { create(:personal_access_token, :expired, :impersonation, user: user) }
+    let!(:revoked_impersonation_token) { create(:personal_access_token, :revoked, :impersonation, user: user) }
 
-    subject { finder.execute }
+    subject { finder(params).execute }
 
     describe 'without user' do
-      let(:finder) { described_class.new }
-
       it do
         is_expected.to contain_exactly(active_personal_access_token, active_impersonation_token,
           revoked_personal_access_token, expired_personal_access_token,
@@ -22,49 +25,49 @@ describe PersonalAccessTokensFinder do
       end
 
       describe 'without impersonation' do
-        before { finder.params.merge!(impersonation: false) }
+        before { params[:impersonation] = false }
 
         it { is_expected.to contain_exactly(active_personal_access_token, revoked_personal_access_token, expired_personal_access_token) }
 
         describe 'with active state' do
-          before { finder.params.merge!(state: 'active') }
+          before { params[:state] = 'active' }
 
           it { is_expected.to contain_exactly(active_personal_access_token) }
         end
 
         describe 'with inactive state' do
-          before { finder.params.merge!(state: 'inactive') }
+          before { params[:state] = 'inactive' }
 
           it { is_expected.to contain_exactly(revoked_personal_access_token, expired_personal_access_token) }
         end
       end
 
       describe 'with impersonation' do
-        before { finder.params.merge!(impersonation: true) }
+        before { params[:impersonation] = true }
 
         it { is_expected.to contain_exactly(active_impersonation_token, revoked_impersonation_token, expired_impersonation_token) }
 
         describe 'with active state' do
-          before { finder.params.merge!(state: 'active') }
+          before { params[:state] = 'active' }
 
           it { is_expected.to contain_exactly(active_impersonation_token) }
         end
 
         describe 'with inactive state' do
-          before { finder.params.merge!(state: 'inactive') }
+          before { params[:state] = 'inactive' }
 
           it { is_expected.to contain_exactly(revoked_impersonation_token, expired_impersonation_token) }
         end
       end
 
       describe 'with active state' do
-        before { finder.params.merge!(state: 'active') }
+        before { params[:state] = 'active' }
 
         it { is_expected.to contain_exactly(active_personal_access_token, active_impersonation_token) }
       end
 
       describe 'with inactive state' do
-        before { finder.params.merge!(state: 'inactive') }
+        before { params[:state] = 'inactive' }
 
         it do
           is_expected.to contain_exactly(expired_personal_access_token, revoked_personal_access_token,
@@ -73,24 +76,24 @@ describe PersonalAccessTokensFinder do
       end
 
       describe 'with id' do
-        subject { finder.execute(id: active_personal_access_token.id) }
+        subject { finder(params).find_by(id: active_personal_access_token.id) }
 
         it { is_expected.to eq(active_personal_access_token) }
 
         describe 'with impersonation' do
-          before { finder.params.merge!(impersonation: true) }
+          before { params[:impersonation] = true }
 
           it { is_expected.to be_nil }
         end
       end
 
       describe 'with token' do
-        subject { finder.execute(token: active_personal_access_token.token) }
+        subject { finder(params).find_by(token: active_personal_access_token.token) }
 
         it { is_expected.to eq(active_personal_access_token) }
 
         describe 'with impersonation' do
-          before { finder.params.merge!(impersonation: true) }
+          before { params[:impersonation] = true }
 
           it { is_expected.to be_nil }
         end
@@ -99,13 +102,14 @@ describe PersonalAccessTokensFinder do
 
     describe 'with user' do
       let(:user2) { create(:user) }
-      let(:finder) { described_class.new(user: user) }
       let!(:other_user_active_personal_access_token) { create(:personal_access_token, user: user2) }
-      let!(:other_user_expired_personal_access_token) { create(:expired_personal_access_token, user: user2) }
-      let!(:other_user_revoked_personal_access_token) { create(:revoked_personal_access_token, user: user2) }
-      let!(:other_user_active_impersonation_token) { create(:impersonation_personal_access_token, user: user2, impersonation: true) }
-      let!(:other_user_expired_impersonation_token) { create(:expired_personal_access_token, user: user2, impersonation: true) }
-      let!(:other_user_revoked_impersonation_token) { create(:revoked_personal_access_token, user: user2, impersonation: true) }
+      let!(:other_user_expired_personal_access_token) { create(:personal_access_token, :expired, user: user2) }
+      let!(:other_user_revoked_personal_access_token) { create(:personal_access_token, :revoked, user: user2) }
+      let!(:other_user_active_impersonation_token) { create(:personal_access_token, :impersonation, user: user2) }
+      let!(:other_user_expired_impersonation_token) { create(:personal_access_token, :expired, :impersonation, user: user2) }
+      let!(:other_user_revoked_impersonation_token) { create(:personal_access_token, :revoked, :impersonation, user: user2) }
+
+      before { params[:user] = user }
 
       it do
         is_expected.to contain_exactly(active_personal_access_token, active_impersonation_token,
@@ -114,49 +118,49 @@ describe PersonalAccessTokensFinder do
       end
 
       describe 'without impersonation' do
-        before { finder.params.merge!(impersonation: false) }
+        before { params[:impersonation] = false }
 
         it { is_expected.to contain_exactly(active_personal_access_token, revoked_personal_access_token, expired_personal_access_token) }
 
         describe 'with active state' do
-          before { finder.params.merge!(state: 'active') }
+          before { params[:state] = 'active' }
 
           it { is_expected.to contain_exactly(active_personal_access_token) }
         end
 
         describe 'with inactive state' do
-          before { finder.params.merge!(state: 'inactive') }
+          before { params[:state] = 'inactive' }
 
           it { is_expected.to contain_exactly(revoked_personal_access_token, expired_personal_access_token) }
         end
       end
 
       describe 'with impersonation' do
-        before { finder.params.merge!(impersonation: true) }
+        before { params[:impersonation] = true }
 
         it { is_expected.to contain_exactly(active_impersonation_token, revoked_impersonation_token, expired_impersonation_token) }
 
         describe 'with active state' do
-          before { finder.params.merge!(state: 'active') }
+          before { params[:state] = 'active' }
 
           it { is_expected.to contain_exactly(active_impersonation_token) }
         end
 
         describe 'with inactive state' do
-          before { finder.params.merge!(state: 'inactive') }
+          before { params[:state] = 'inactive' }
 
           it { is_expected.to contain_exactly(revoked_impersonation_token, expired_impersonation_token) }
         end
       end
 
       describe 'with active state' do
-        before { finder.params.merge!(state: 'active') }
+        before { params[:state] = 'active' }
 
         it { is_expected.to contain_exactly(active_personal_access_token, active_impersonation_token) }
       end
 
       describe 'with inactive state' do
-        before { finder.params.merge!(state: 'inactive') }
+        before { params[:state] = 'inactive' }
 
         it do
           is_expected.to contain_exactly(expired_personal_access_token, revoked_personal_access_token,
@@ -165,24 +169,24 @@ describe PersonalAccessTokensFinder do
       end
 
       describe 'with id' do
-        subject { finder.execute(id: active_personal_access_token.id) }
+        subject { finder(params).find_by(id: active_personal_access_token.id) }
 
         it { is_expected.to eq(active_personal_access_token) }
 
         describe 'with impersonation' do
-          before { finder.params.merge!(impersonation: true) }
+          before { params[:impersonation] = true }
 
           it { is_expected.to be_nil }
         end
       end
 
       describe 'with token' do
-        subject { finder.execute(token: active_personal_access_token.token) }
+        subject { finder(params).find_by(token: active_personal_access_token.token) }
 
         it { is_expected.to eq(active_personal_access_token) }
 
         describe 'with impersonation' do
-          before { finder.params.merge!(impersonation: true) }
+          before { params[:impersonation] = true }
 
           it { is_expected.to be_nil }
         end

spec/lib/gitlab/auth_spec.rb

@@ -118,7 +118,7 @@ describe Gitlab::Auth, lib: true do
       end
 
       it 'succeeds if it is an impersonation token' do
-        impersonation_token = create(:personal_access_token, impersonation: true, scopes: ['api'])
+        impersonation_token = create(:personal_access_token, :impersonation, scopes: ['api'])
 
         expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '')
         expect(gl_auth.find_for_git_client('', impersonation_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(impersonation_token.user, nil, :personal_token, full_authentication_abilities))

spec/models/personal_access_token_spec.rb

@@ -3,7 +3,7 @@ require 'spec_helper'
 describe PersonalAccessToken, models: true do
   describe '.build' do
     let(:personal_access_token) { build(:personal_access_token) }
-    let(:invalid_personal_access_token) { build(:personal_access_token, token: nil) }
+    let(:invalid_personal_access_token) { build(:personal_access_token, :invalid) }
 
     it 'is a valid personal access token' do
       expect(personal_access_token).to be_valid
@@ -19,8 +19,8 @@ describe PersonalAccessToken, models: true do
 
   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) }
+    let(:revoked_personal_access_token) { build(:personal_access_token, :revoked) }
+    let(:expired_personal_access_token) { build(:personal_access_token, :expired) }
 
     it "returns false if the personal_access_token is revoked" do
       expect(revoked_personal_access_token).not_to be_active

spec/requests/api/personal_access_tokens_spec.rb

-require 'spec_helper'
-
-describe API::PersonalAccessTokens, api: true  do
-  include ApiHelpers
-
-  let(:user)  { create(:user) }
-  let(:not_found_token) { (PersonalAccessToken.maximum('id') || 0) + 10 }
-  let(:finder) { PersonalAccessTokensFinder.new(user: user, impersonation: false) }
-
-  describe "GET /personal_access_tokens" do
-    let!(:active_impersonation_token) { create(:impersonation_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!(: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(finder.execute.count)
-
-      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
-      finder.params[:state] = 'active'
-
-      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.size).to eq(finder.execute.count)
-      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
-      finder.params[:state] = 'inactive'
-
-      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.size).to eq(finder.execute.count)
-      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) { %w(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(json_response['impersonation']).not_to be_present
-      expect(finder.execute(id: personal_access_token_id)).not_to be_nil
-    end
-  end
-
-  describe 'GET /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
-      get api("/personal_access_tokens/#{not_found_token}", user)
-
-      expect(response).to have_http_status(404)
-      expect(json_response['message']).to eq('404 Personal Access Token Not Found')
-    end
-
-    it 'returns a 404 error if personal access token exists but it is a personal access tokens of another user' do
-      get 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 Personal Access Token Not Found')
-    end
-
-    it 'returns a personal access token and does not expose token in the json response' do
-      get api("/personal_access_tokens/#{personal_access_token.id}", user)
-
-      expect(response).to have_http_status(200)
-      expect(json_response['token']).not_to be_present
-    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/#{not_found_token}", user)
-
-      expect(response).to have_http_status(404)
-      expect(json_response['message']).to eq('404 Personal Access Token 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 Personal Access Token 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(204)
-      expect(personal_access_token.revoked).to eq(false)
-      expect(personal_access_token.reload.revoked).to eq(true)
-    end
-  end
-end

spec/requests/api/users_spec.rb

@@ -12,7 +12,6 @@ describe API::Users, api: true  do
   let(:ldap_blocked_user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') }
   let(:not_existing_user_id) { (User.maximum('id') || 0 ) + 10 }
   let(:not_existing_pat_id) { (PersonalAccessToken.maximum('id') || 0 ) + 10 }
-  let(:finder) { PersonalAccessTokensFinder.new(user: user) }
 
   describe "GET /users" do
     context "when unauthenticated" do
@@ -1159,99 +1158,71 @@ describe API::Users, api: true  do
     end
   end
 
-  describe 'GET /users/:id/personal_access_tokens' do
+  describe 'GET /users/:user_id/impersonation_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) }
-    let!(:impersonation_personal_access_token) { create(:impersonation_personal_access_token, user: user) }
+    let!(:revoked_personal_access_token) { create(:personal_access_token, :revoked, user: user) }
+    let!(:expired_personal_access_token) { create(:personal_access_token, :expired, user: user) }
+    let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) }
+    let!(:revoked_impersonation_token) { create(:personal_access_token, :impersonation, :revoked, user: user) }
 
     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}/impersonation_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)
+      get api("/users/#{not_existing_user_id}/impersonation_tokens", user)
 
       expect(response).to have_http_status(403)
       expect(json_response['message']).to eq('403 Forbidden')
     end
 
-    it 'returns an array of all impersonated and non-impersonated tokens' do
-      get api("/users/#{user.id}/personal_access_tokens", admin)
+    it 'returns an array of all impersonated tokens' do
+      get api("/users/#{user.id}/impersonation_tokens", admin)
 
       expect(response).to have_http_status(200)
       expect(response).to include_pagination_headers
       expect(json_response).to be_an Array
-      expect(json_response.size).to eq(finder.execute.count)
+      expect(json_response.size).to eq(2)
     end
 
-    it 'returns an array of non impersonated personal access tokens' do
-      finder.params[:impersonation] = false
-
-      get api("/users/#{user.id}/personal_access_tokens?impersonation=false", admin)
-
-      expect(response).to have_http_status(200)
-      expect(response).to include_pagination_headers
-      expect(json_response).to be_an Array
-      expect(json_response.size).to eq(finder.execute.count)
-      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
-      finder.params.merge!(state: 'active', impersonation: false)
-
-      get api("/users/#{user.id}/personal_access_tokens?state=active&impersonation=false", admin)
+    it 'returns an array of active impersonation tokens if state active' do
+      get api("/users/#{user.id}/impersonation_tokens?state=active", admin)
 
       expect(response).to have_http_status(200)
       expect(response).to include_pagination_headers
       expect(json_response).to be_an Array
-      expect(json_response.size).to eq(finder.execute.count)
+      expect(json_response.size).to eq(1)
       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
-      finder.params.merge!(state: 'inactive', impersonation: false)
-      get api("/users/#{user.id}/personal_access_tokens?impersonation=false&state=inactive", admin)
+      get api("/users/#{user.id}/impersonation_tokens?state=inactive", admin)
 
       expect(response).to have_http_status(200)
       expect(json_response).to be_an Array
-      expect(json_response.size).to eq(finder.execute.count)
+      expect(json_response.size).to eq(1)
       expect(json_response).to all(include('active' => false))
     end
-
-    it 'returns an array of impersonation personal access tokens if impersonation is set to true' do
-      finder.params[:impersonation] = true
-
-      get api("/users/#{user.id}/personal_access_tokens?impersonation=true", admin)
-
-      expect(response).to have_http_status(200)
-      expect(response).to include_pagination_headers
-      expect(json_response).to be_an Array
-      expect(json_response.size).to eq(finder.execute.count)
-      expect(json_response).to all(include('impersonation' => true))
-    end
   end
 
-  describe 'POST /users/:id/personal_access_tokens' do
+  describe 'POST /users/:user_id/impersonation_tokens' do
     let(:name) { 'my new pat' }
     let(:expires_at) { '2016-12-28' }
     let(:scopes) { %w(api read_user) }
     let(:impersonation) { true }
 
-    it 'returns validation error if personal access token misses some attributes' do
-      post api("/users/#{user.id}/personal_access_tokens", admin)
+    it 'returns validation error if impersonation token misses some attributes' do
+      post api("/users/#{user.id}/impersonation_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),
+      post api("/users/#{not_existing_user_id}/impersonation_tokens", admin),
         name: name,
         expires_at: expires_at
 
@@ -1260,7 +1231,7 @@ describe API::Users, api: true  do
     end
 
     it 'returns a 403 error when authenticated as normal user' do
-      post api("/users/#{user.id}/personal_access_tokens", user),
+      post api("/users/#{user.id}/impersonation_tokens", user),
         name: name,
         expires_at: expires_at
 
@@ -1268,8 +1239,8 @@ describe API::Users, api: true  do
       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),
+    it 'creates a impersonation token' do
+      post api("/users/#{user.id}/impersonation_tokens", admin),
         name: name,
         expires_at: expires_at,
         scopes: scopes,
@@ -1285,75 +1256,88 @@ describe API::Users, api: true  do
       expect(json_response['revoked']).to be_falsey
       expect(json_response['token']).to be_present
       expect(json_response['impersonation']).to eq(impersonation)
-      expect(finder.execute(id: json_response['id'])).not_to be_nil
     end
   end
 
-  describe 'GET /users/:id/personal_access_tokens/:personal_access_token_id' do
-    let!(:personal_access_token) { create(:personal_access_token, user: user, revoked: false) }
-    let!(:impersonation_token) { create(:impersonation_personal_access_token, user: user, revoked: false) }
+  describe 'GET /users/:user_id/impersonation_tokens/:impersonation_token_id' do
+    let!(:personal_access_token) { create(:personal_access_token, user: user) }
+    let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) }
 
     it 'returns 404 error if user not found' do
-      get api("/users/#{not_existing_user_id}/personal_access_tokens/1", admin)
+      get api("/users/#{not_existing_user_id}/impersonation_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
-      get api("/users/#{user.id}/personal_access_tokens/#{not_existing_pat_id}", admin)
+    it 'returns a 404 error if impersonation token not found' do
+      get api("/users/#{user.id}/impersonation_tokens/#{not_existing_pat_id}", admin)
 
       expect(response).to have_http_status(404)
-      expect(json_response['message']).to eq('404 Personal Access Token Not Found')
+      expect(json_response['message']).to eq('404 Impersonation Token Not Found')
+    end
+
+    it 'returns a 404 error if token is not impersonation token' do
+      get api("/users/#{user.id}/impersonation_tokens/#{personal_access_token.id}", admin)
+
+      expect(response).to have_http_status(404)
+      expect(json_response['message']).to eq('404 Impersonation Token Not Found')
     end
 
     it 'returns a 403 error when authenticated as normal user' do
-      get api("/users/#{user.id}/personal_access_tokens/#{personal_access_token.id}", user)
+      get api("/users/#{user.id}/impersonation_tokens/#{impersonation_token.id}", user)
 
       expect(response).to have_http_status(403)
       expect(json_response['message']).to eq('403 Forbidden')
     end
 
     it 'returns a personal access token' do
-      get api("/users/#{user.id}/personal_access_tokens/#{personal_access_token.id}", admin)
+      get api("/users/#{user.id}/impersonation_tokens/#{impersonation_token.id}", admin)
 
       expect(response).to have_http_status(200)
       expect(json_response['token']).to be_present
-      expect(json_response['impersonation']).to be_falsey
+      expect(json_response['impersonation']).to be_truthy
     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) }
-    let!(:impersonation_token) { create(:impersonation_personal_access_token, user: user, revoked: false) }
+  describe 'DELETE /users/:user_id/impersonation_tokens/:impersonation_token_id' do
+    let!(:personal_access_token) { create(:personal_access_token, user: user) }
+    let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) }
 
     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}/impersonation_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/#{not_existing_pat_id}", admin)
+    it 'returns a 404 error if impersonation token not found' do
+      delete api("/users/#{user.id}/impersonation_tokens/#{not_existing_pat_id}", admin)
+
+      expect(response).to have_http_status(404)
+      expect(json_response['message']).to eq('404 Impersonation Token Not Found')
+    end
+
+    it 'returns a 404 error if token is not impersonation token' do
+      delete api("/users/#{user.id}/impersonation_tokens/#{personal_access_token.id}", admin)
 
       expect(response).to have_http_status(404)
-      expect(json_response['message']).to eq('404 Personal Access Token Not Found')
+      expect(json_response['message']).to eq('404 Impersonation Token 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)
+      delete api("/users/#{user.id}/impersonation_tokens/#{impersonation_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)
+    it 'revokes a impersonation token' do
+      delete api("/users/#{user.id}/impersonation_tokens/#{impersonation_token.id}", admin)
 
       expect(response).to have_http_status(204)
-      expect(personal_access_token.revoked).to be_falsey
-      expect(personal_access_token.reload.revoked).to be_truthy
+      expect(impersonation_token.revoked).to be_falsey
+      expect(impersonation_token.reload.revoked).to be_truthy
     end
   end
 end