Merge branch 'if-53347_fix_impersonation_tokens' into 'master'

Display impersonation token value only after creation

See merge request gitlab-org/gitlab-ce!22916

app/controllers/admin/impersonation_tokens_controller.rb

@@ -11,6 +11,7 @@ class Admin::ImpersonationTokensController < Admin::ApplicationController
     @impersonation_token = finder.build(impersonation_token_params)
 
     if @impersonation_token.save
+      PersonalAccessToken.redis_store!(current_user.id, @impersonation_token.token)
       redirect_to admin_user_impersonation_tokens_path, notice: "A new impersonation token has been created."
     else
       set_index_vars
@@ -53,6 +54,8 @@ class Admin::ImpersonationTokensController < Admin::ApplicationController
     @impersonation_token ||= finder.build
     @inactive_impersonation_tokens = finder(state: 'inactive').execute
     @active_impersonation_tokens = finder(state: 'active').execute.order(:expires_at)
+
+    @new_impersonation_token = PersonalAccessToken.redis_getdel(current_user.id)
   end
   # rubocop: enable CodeReuse/ActiveRecord
 end

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

@@ -5,6 +5,11 @@
 
 .row.prepend-top-default
   .col-lg-12
+    - if @new_impersonation_token
+      = render "shared/personal_access_tokens_created_container", new_token_value: @new_impersonation_token,
+                                                                 container_title: 'Your New Impersonation Token',
+                                                                 clipboard_button_title: 'Copy impersonation token to clipboard'
+
     = render "shared/personal_access_tokens_form", path: admin_user_impersonation_tokens_path, impersonation: true, token: @impersonation_token, scopes: @scopes
 
     = render "shared/personal_access_tokens_table", impersonation: true, active_tokens: @active_impersonation_tokens, inactive_tokens: @inactive_impersonation_tokens

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

@@ -14,17 +14,7 @@
 
   .col-lg-8
     - if @new_personal_access_token
-      .created-personal-access-token-container
-        %h5.prepend-top-0
-          Your New Personal Access Token
-        .form-group
-          .input-group
-            = text_field_tag 'created-personal-access-token', @new_personal_access_token, readonly: true, class: "form-control js-select-on-focus", 'aria-describedby' => "created-personal-access-token-help-block"
-            %span.input-group-append
-              = clipboard_button(text: @new_personal_access_token, title: "Copy personal access token to clipboard", placement: "left", class: "input-group-text btn-default btn-clipboard")
-          %span#created-personal-access-token-help-block.form-text.text-muted.text-danger Make sure you save it - you won't be able to access it again.
-
-      %hr
+      = render "shared/personal_access_tokens_created_container", new_token_value: @new_personal_access_token
 
     = render "shared/personal_access_tokens_form", path: profile_personal_access_tokens_path, impersonation: false, token: @personal_access_token, scopes: @scopes
 

app/views/shared/_personal_access_tokens_created_container.html.haml

+- container_title = local_assigns.fetch(:container_title, 'Your New Personal Access Token')
+- clipboard_button_title = local_assigns.fetch(:clipboard_button_title, 'Copy personal access token to clipboard')
+
+.created-personal-access-token-container
+  %h5.prepend-top-0
+    = container_title
+  .form-group
+    .input-group
+      = text_field_tag 'created-personal-access-token', new_token_value, readonly: true, class: "form-control js-select-on-focus", 'aria-describedby' => "created-token-help-block"
+      %span.input-group-append
+        = clipboard_button(text: new_token_value, title: clipboard_button_title, placement: "left", class: "input-group-text btn-default btn-clipboard")
+    %span#created-token-help-block.form-text.text-muted.text-danger Make sure you save it - you won't be able to access it again.
+
+%hr

app/views/shared/_personal_access_tokens_table.html.haml

@@ -15,8 +15,6 @@
           %th Created
           %th Expires
           %th Scopes
-          - if impersonation
-            %th Token
           %th
       %tbody
         - active_tokens.each do |token|
@@ -30,10 +28,6 @@
               - 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(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 float-right", data: { confirm: "Are you sure you want to revoke this #{type} Token? This action cannot be undone." }
 - else

changelogs/unreleased/ce-53347_fix_impersonation_tokens.yml

+---
+title: Display impersonation token value only after creation
+merge_request: 22916
+author:
+type: fixed

doc/api/users.md

@@ -1072,7 +1072,6 @@ Example response:
 [
    {
       "active" : true,
-      "token" : "EsMo-vhKfXGwX9RKrwiy",
       "scopes" : [
          "api"
       ],
@@ -1089,7 +1088,6 @@ Example response:
          "read_user"
       ],
       "revoked" : true,
-      "token" : "ZcZRpLeEuQRprkRjYydY",
       "name" : "mytoken2",
       "created_at" : "2017-03-17T17:19:28.697Z",
       "id" : 3,
@@ -1125,7 +1123,6 @@ Example response:
 ```json
 {
    "active" : true,
-   "token" : "EsMo-vhKfXGwX9RKrwiy",
    "scopes" : [
       "api"
    ],
@@ -1142,6 +1139,8 @@ Example response:
 
 > Requires admin permissions.
 
+> Token values are returned once. Make sure you save it - you won't be able to access it again.
+
 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 their profile

lib/api/entities.rb

@@ -1263,7 +1263,11 @@ module API
       expose :token
     end
 
-    class ImpersonationToken < PersonalAccessTokenWithToken
+    class ImpersonationToken < PersonalAccessToken
+      expose :impersonation
+    end
+
+    class ImpersonationTokenWithToken < PersonalAccessTokenWithToken
       expose :impersonation
     end
 

lib/api/users.rb

@@ -531,7 +531,7 @@ module API
 
           desc 'Create a impersonation token. Available only for admins.' do
             detail 'This feature was introduced in GitLab 9.0'
-            success Entities::ImpersonationToken
+            success Entities::ImpersonationTokenWithToken
           end
           params do
             requires :name, type: String, desc: 'The name of the impersonation token'
@@ -542,7 +542,7 @@ module API
             impersonation_token = finder.build(declared_params(include_missing: false))
 
             if impersonation_token.save
-              present impersonation_token, with: Entities::ImpersonationToken
+              present impersonation_token, with: Entities::ImpersonationTokenWithToken
             else
               render_validation_error!(impersonation_token)
             end

qa/qa/page/profile/personal_access_tokens.rb

@@ -8,7 +8,7 @@ module QA
           element :scopes_api_radios, "label :scopes" # rubocop:disable QA/ElementWithPattern
         end
 
-        view 'app/views/profiles/personal_access_tokens/index.html.haml' do
+        view 'app/views/shared/_personal_access_tokens_created_container.html.haml' do
           element :create_token_field, "text_field_tag 'created-personal-access-token'" # rubocop:disable QA/ElementWithPattern
         end
 

spec/controllers/profiles/personal_access_tokens_controller_spec.rb

@@ -39,8 +39,10 @@ describe Profiles::PersonalAccessTokensController do
     let!(:active_personal_access_token) { create(: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) }
+    let(:token_value) { 's3cr3t' }
 
     before do
+      PersonalAccessToken.redis_store!(user.id, token_value)
       get :index
     end
 
@@ -56,5 +58,9 @@ describe Profiles::PersonalAccessTokensController do
       expect(assigns(:active_personal_access_tokens)).not_to include(impersonation_personal_access_token)
       expect(assigns(:inactive_personal_access_tokens)).not_to include(impersonation_personal_access_token)
     end
+
+    it "retrieves newly created personal access token value" do
+      expect(assigns(:new_personal_access_token)).to eql(token_value)
+    end
   end
 end

spec/features/admin/admin_users_impersonation_tokens_spec.rb

@@ -12,6 +12,10 @@ describe 'Admin > Users > Impersonation Tokens', :js do
     find(".settings-message")
   end
 
+  def created_impersonation_token
+    find("#created-personal-access-token").value
+  end
+
   before do
     sign_in(admin)
   end
@@ -39,6 +43,7 @@ describe 'Admin > Users > Impersonation Tokens', :js do
       expect(active_impersonation_tokens).to have_text('api')
       expect(active_impersonation_tokens).to have_text('read_user')
       expect(PersonalAccessTokensFinder.new(impersonation: true).execute.count).to equal(1)
+      expect(created_impersonation_token).not_to be_empty
     end
   end
 

spec/features/profiles/personal_access_tokens_spec.rb

@@ -43,10 +43,12 @@ describe 'Profile > Personal Access Tokens', :js do
       check "read_user"
 
       click_on "Create personal access token"
+
       expect(active_personal_access_tokens).to have_text(name)
       expect(active_personal_access_tokens).to have_text('In')
       expect(active_personal_access_tokens).to have_text('api')
       expect(active_personal_access_tokens).to have_text('read_user')
+      expect(created_personal_access_token).not_to be_empty
     end
 
     context "when creation fails" do
@@ -57,6 +59,7 @@ describe 'Profile > Personal Access Tokens', :js do
 
         expect { click_on "Create personal access token" }.not_to change { PersonalAccessToken.count }
         expect(page).to have_content("Name cannot be nil")
+        expect(page).not_to have_selector("#created-personal-access-token")
       end
     end
   end

spec/requests/api/users_spec.rb

@@ -2018,11 +2018,11 @@ describe API::Users do
       expect(json_response['message']).to eq('403 Forbidden')
     end
 
-    it 'returns a personal access token' do
+    it 'returns an impersonation token' do
       get api("/users/#{user.id}/impersonation_tokens/#{impersonation_token.id}", admin)
 
       expect(response).to have_gitlab_http_status(200)
-      expect(json_response['token']).to be_present
+      expect(json_response['token']).not_to be_present
       expect(json_response['impersonation']).to be_truthy
     end
   end