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

Display impersonation token value only after creation

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

Author: stanhu

app/controllers/admin/impersonation_tokens_controller.rb

@@ -11,6 +11,7 @@ def create
     @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 @@ def set_index_vars
     @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

@@ -0,0 +1,14 @@
+- 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

@@ -0,0 +1,5 @@
+---
+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 @@ class PersonalAccessTokenWithToken < PersonalAccessToken
       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 @@ def find_impersonation_token
 
           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 @@ def find_impersonation_token
             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 @@ class PersonalAccessTokens < Page::Base
           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 @@ def created_token
     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 @@ def created_token
       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 @@ def no_personal_access_tokens_message
     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 @@ def no_personal_access_tokens_message
       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 @@ def disallow_personal_access_token_saves!
       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 @@ def disallow_personal_access_token_saves!
 
         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 @@
       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