apply codestyle and implementation changes to the respective feature codepersonal_access_token_api_and_impersonation_token

27 files changed, 313 insertions, 649 deletions

diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss
index 4a8e4344851..3889deee21a 100644
--- a/app/assets/stylesheets/pages/settings.scss
+++ b/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;
diff --git a/app/controllers/admin/impersonation_tokens_controller.rb b/app/controllers/admin/impersonation_tokens_controller.rb
index 448f2c881a1..d26004539b5 100644
--- a/app/controllers/admin/impersonation_tokens_controller.rb
+++ b/app/controllers/admin/impersonation_tokens_controller.rb
@@ -1,12 +1,12 @@
 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
diff --git a/app/controllers/profiles/personal_access_tokens_controller.rb b/app/controllers/profiles/personal_access_tokens_controller.rb
index 2188350f2fd..d1f2374e9eb 100644
--- a/app/controllers/profiles/personal_access_tokens_controller.rb
+++ b/app/controllers/profiles/personal_access_tokens_controller.rb
@@ -1,12 +1,10 @@
 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
diff --git a/app/finders/personal_access_tokens_finder.rb b/app/finders/personal_access_tokens_finder.rb
index 7b9a2f6c0bb..760166b453f 100644
--- a/app/finders/personal_access_tokens_finder.rb
+++ b/app/finders/personal_access_tokens_finder.rb
@@ -1,36 +1,34 @@
 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
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 6fb5ac4a4ef..187627247d2 100644
--- a/app/models/user.rb
+++ b/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.
diff --git a/app/views/admin/impersonation_tokens/index.html.haml b/app/views/admin/impersonation_tokens/index.html.haml
index 9116384e322..1378dde52ab 100644
--- a/app/views/admin/impersonation_tokens/index.html.haml
+++ b/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
diff --git a/app/views/admin/users/_head.html.haml b/app/views/admin/users/_head.html.haml
index 1ded8fa6086..d20be373564 100644
--- a/app/views/admin/users/_head.html.haml
+++ b/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
diff --git a/app/views/profiles/personal_access_tokens/index.html.haml b/app/views/profiles/personal_access_tokens/index.html.haml
index 7b5121ed550..0645ecad496 100644
--- a/app/views/profiles/personal_access_tokens/index.html.haml
+++ b/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() {
diff --git a/app/views/shared/_personal_access_tokens_form.html.haml b/app/views/shared/_personal_access_tokens_form.html.haml
index 074eeb7d038..af4cc90f4a7 100644
--- a/app/views/shared/_personal_access_tokens_form.html.haml
+++ b/app/views/shared/_personal_access_tokens_form.html.haml
@@ -1,10 +1,13 @@
-- 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
diff --git a/app/views/shared/_personal_access_tokens_table.html.haml b/app/views/shared/_personal_access_tokens_table.html.haml
new file mode 100644
index 00000000000..67a49815478
--- /dev/null
+++ b/app/views/shared/_personal_access_tokens_table.html.haml
@@ -0,0 +1,60 @@
+- 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.
diff --git a/db/migrate/20161228135550_add_impersonation_to_personal_access_tokens.rb b/db/migrate/20161228135550_add_impersonation_to_personal_access_tokens.rb
index 48648043dbb..ea9caceaa2c 100644
--- a/db/migrate/20161228135550_add_impersonation_to_personal_access_tokens.rb
+++ b/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
diff --git a/doc/api/README.md b/doc/api/README.md
index 759ec253a1f..c790e000945 100644
--- a/doc/api/README.md
+++ b/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
diff --git a/doc/api/personal_access_tokens.md b/doc/api/personal_access_tokens.md
deleted file mode 100644
index ea156d92dc8..00000000000
--- a/doc/api/personal_access_tokens.md
+++ /dev/null
@@ -1,69 +0,0 @@
-# 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 |
diff --git a/doc/api/users.md b/doc/api/users.md
index 91168c05dbe..4b7618d1019 100644
--- a/doc/api/users.md
+++ b/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) |
 
-## Revoke a personal access token
+Example response:
+```json
+{
+  "id": 1,
+  "name": "mytoken",
+  "revoked": false,
+  "expires_at": "2017-01-04",
+  "scopes": ['api'],
+  "active": true,
+  "impersonation": true,
+  "token": "9koXpg98eAheJpvBs5tK"
+}
+```
 
-It revokes a personal access token. Note that only administrators can revoke impersonation tokens.
+## Revoke an impersonation token
+
+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 |
diff --git a/lib/api/api.rb b/lib/api/api.rb
index a2ce03a901c..b27ac3f1d15 100644
--- a/lib/api/api.rb
+++ b/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
diff --git a/lib/api/personal_access_tokens.rb b/lib/api/personal_access_tokens.rb
deleted file mode 100644
index 7f1a54ac12f..00000000000
--- a/lib/api/personal_access_tokens.rb
+++ /dev/null
@@ -1,71 +0,0 @@
-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
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 37117049007..549003f576a 100644
--- a/lib/api/users.rb
+++ b/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'
-      end
-      segment ':id' do
-        resource :personal_access_tokens do
-          before do
-            authenticated_as_admin!
-            user = find_user(params)
-            @finder = PersonalAccessTokensFinder.new(user: user)
+        requires :user_id, type: Integer, desc: 'The ID of the user'
+      end
+      segment ':user_id' do
+        resource :impersonation_tokens do
+          helpers do
+            def finder(options = {})
+              user = find_user(params)
+              PersonalAccessTokensFinder.new({ user: user, impersonation: true }.merge(options))
+            end
+
+            def find_impersonation_token
+              finder.find_by(id: declared_params[:impersonation_token_id]) || not_found!('Impersonation Token')
+            end
           end
 
-          desc 'Retrieve personal access tokens. Available only for admins.' do
+          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
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 6f84288654f..587be734f32 100644
--- a/lib/gitlab/auth.rb
+++ b/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)
diff --git a/spec/controllers/profiles/personal_access_tokens_spec.rb b/spec/controllers/profiles/personal_access_tokens_spec.rb
index 714bf8cd441..ae8031a45f6 100644
--- a/spec/controllers/profiles/personal_access_tokens_spec.rb
+++ b/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 }
 
diff --git a/spec/factories/personal_access_tokens.rb b/spec/factories/personal_access_tokens.rb
index 1905b22935b..7b15ba47de1 100644
--- a/spec/factories/personal_access_tokens.rb
+++ b/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
diff --git a/spec/features/admin/admin_users_impersonation_tokens_spec.rb b/spec/features/admin/admin_users_impersonation_tokens_spec.rb
index 804723cea32..9ff5c2f9d40 100644
--- a/spec/features/admin/admin_users_impersonation_tokens_spec.rb
+++ b/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)
diff --git a/spec/features/profiles/personal_access_tokens_spec.rb b/spec/features/profiles/personal_access_tokens_spec.rb
index 74e4e157dc5..0917d4dc3ef 100644
--- a/spec/features/profiles/personal_access_tokens_spec.rb
+++ b/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)
diff --git a/spec/finders/personal_access_tokens_finder_spec.rb b/spec/finders/personal_access_tokens_finder_spec.rb
index 91e746f295a..fd92664ca24 100644
--- a/spec/finders/personal_access_tokens_finder_spec.rb
+++ b/spec/finders/personal_access_tokens_finder_spec.rb
@@ -1,20 +1,23 @@
 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
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index ca297fead4a..8e7af233e6b 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/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))
diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb
index 173136ef899..7c9f4aad836 100644
--- a/spec/models/personal_access_token_spec.rb
+++ b/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
diff --git a/spec/requests/api/personal_access_tokens_spec.rb b/spec/requests/api/personal_access_tokens_spec.rb
deleted file mode 100644
index 4037ce483ea..00000000000
--- a/spec/requests/api/personal_access_tokens_spec.rb
+++ /dev/null
@@ -1,141 +0,0 @@
-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
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 39c94edd44a..c09e5717087 100644
--- a/spec/requests/api/users_spec.rb
+++ b/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