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(", ") : "" - %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(", ") : "" - %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(", ") : "" + - 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' + 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! - user = find_user(params) - @finder = PersonalAccessTokensFinder.new(user: 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 - desc 'Retrieve personal access 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 - end - get do - @finder.params.merge!(declared_params(include_missing: false)) - present paginate(@finder.execute), with: Entities::ImpersonationToken - end - - desc 'Create a personal access 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' - 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::ImpersonationToken - else - render_validation_error!(personal_access_token) + def find_impersonation_token + finder.find_by(id: declared_params[:impersonation_token_id]) || not_found!('Impersonation Token') end end - desc 'Retrieve personal access token. 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 - requires :personal_access_token_id, type: Integer, desc: 'The ID of the personal access token' + use :pagination + optional :state, type: String, default: 'all', values: %w[all active inactive], desc: 'Filters (all|active|inactive) impersonation_tokens' 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 + get { present paginate(finder(declared_params(include_missing: false)).execute), with: Entities::ImpersonationToken } - present personal_access_token, with: Entities::ImpersonationToken + 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 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 + impersonation_token = finder.build(declared_params(include_missing: false)) + + if impersonation_token.save + present impersonation_token, with: Entities::ImpersonationToken + else + render_validation_error!(impersonation_token) + end end - desc 'Revoke a 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 :impersonation_token_id, type: Integer, desc: 'The ID of the impersonation token' + end + get ':impersonation_token_id' do + present find_impersonation_token, with: Entities::ImpersonationToken + end + + 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) + 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.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) - - 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 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 + 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 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