Move profiles/gpg_keys#get_keys to users#gpg_keys

This simplifies the basic logic as well as domains in controllers.
Signed-off-by: Takuya Noguchi <takninnovationresearch@gmail.com>

app/controllers/profiles/gpg_keys_controller.rb

@@ -2,7 +2,6 @@
 
 class Profiles::GpgKeysController < Profiles::ApplicationController
   before_action :set_gpg_key, only: [:destroy, :revoke]
-  skip_before_action :authenticate_user!, only: [:get_keys]
 
   feature_category :users
 
@@ -40,24 +39,6 @@ class Profiles::GpgKeysController < Profiles::ApplicationController
     end
   end
 
-  # Get all gpg keys of a user(params[:username]) in a text format
-  def get_keys
-    if params[:username].present?
-      begin
-        user = UserFinder.new(params[:username]).find_by_username
-        if user.present?
-          render plain: user.gpg_keys.select(&:verified?).map(&:key).join("\n")
-        else
-          render_404
-        end
-      rescue => e
-        render html: e.message
-      end
-    else
-      render_404
-    end
-  end
-
   private
 
   def gpg_key_params

app/controllers/users_controller.rb

@@ -58,6 +58,11 @@ class UsersController < ApplicationController
     end
   end
 
+  # Get all gpg keys of a user(params[:username]) in a text format
+  def gpg_keys
+    render plain: user.gpg_keys.select(&:verified?).map(&:key).join("\n")
+  end
+
   def groups
     load_groups
 

changelogs/unreleased/290762-move-the-action-from-profiles-gpgkeyscontroller-get_keys-to-usersc.yml

+---
+title: Move profiles/gpg_keys#get_keys to users#gpg_keys
+merge_request: 49448
+author: Takuya Noguchi
+type: other

config/routes/user.rb

@@ -58,7 +58,7 @@ constraints(::Constraints::UserUrlConstrainer.new) do
   get ':username.keys' => 'users#ssh_keys', constraints: { username: Gitlab::PathRegex.root_namespace_route_regex }
 
   # Get all GPG keys of user
-  get ':username.gpg' => 'profiles/gpg_keys#get_keys', constraints: { username: Gitlab::PathRegex.root_namespace_route_regex }
+  get ':username.gpg' => 'users#gpg_keys', constraints: { username: Gitlab::PathRegex.root_namespace_route_regex }
 
   scope(path: ':username',
         as: :user,

spec/controllers/profiles/gpg_keys_controller_spec.rb

@@ -16,121 +16,4 @@ RSpec.describe Profiles::GpgKeysController do
       end.to change { GpgKey.count }.by(1)
     end
   end
-
-  describe "#get_keys" do
-    describe "non existent user" do
-      it "does not generally work" do
-        get :get_keys, params: { username: 'not-existent' }
-
-        expect(response).not_to be_successful
-      end
-    end
-
-    describe "user with no keys" do
-      it "does generally work" do
-        get :get_keys, params: { username: user.username }
-
-        expect(response).to be_successful
-      end
-
-      it "renders all keys separated with a new line" do
-        get :get_keys, params: { username: user.username }
-
-        expect(response.body).to eq("")
-      end
-
-      it "responds with text/plain content type" do
-        get :get_keys, params: { username: user.username }
-
-        expect(response.content_type).to eq("text/plain")
-      end
-    end
-
-    describe "user with keys" do
-      let!(:gpg_key) { create(:gpg_key, user: user) }
-      let!(:another_gpg_key) { create(:another_gpg_key, user: user) }
-
-      describe "while signed in" do
-        before do
-          sign_in(user)
-        end
-
-        it "does generally work" do
-          get :get_keys, params: { username: user.username }
-
-          expect(response).to be_successful
-        end
-
-        it "renders all verified keys separated with a new line" do
-          get :get_keys, params: { username: user.username }
-
-          expect(response.body).not_to eq('')
-          expect(response.body).to eq(user.gpg_keys.select(&:verified?).map(&:key).join("\n"))
-
-          expect(response.body).to include(gpg_key.key)
-          expect(response.body).to include(another_gpg_key.key)
-        end
-
-        it "responds with text/plain content type" do
-          get :get_keys, params: { username: user.username }
-
-          expect(response.content_type).to eq("text/plain")
-        end
-      end
-
-      describe 'when logged out' do
-        before do
-          sign_out(user)
-        end
-
-        it "still does generally work" do
-          get :get_keys, params: { username: user.username }
-
-          expect(response).to be_successful
-        end
-
-        it "renders all verified keys separated with a new line" do
-          get :get_keys, params: { username: user.username }
-
-          expect(response.body).not_to eq('')
-          expect(response.body).to eq(user.gpg_keys.map(&:key).join("\n"))
-
-          expect(response.body).to include(gpg_key.key)
-          expect(response.body).to include(another_gpg_key.key)
-        end
-
-        it "responds with text/plain content type" do
-          get :get_keys, params: { username: user.username }
-
-          expect(response.content_type).to eq("text/plain")
-        end
-      end
-
-      describe 'when revoked' do
-        before do
-          sign_in(user)
-          another_gpg_key.revoke
-        end
-
-        it "doesn't render revoked keys" do
-          get :get_keys, params: { username: user.username }
-
-          expect(response.body).not_to eq('')
-
-          expect(response.body).to include(gpg_key.key)
-          expect(response.body).not_to include(another_gpg_key.key)
-        end
-
-        it "doesn't render revoked keys for non-authorized users" do
-          sign_out(user)
-          get :get_keys, params: { username: user.username }
-
-          expect(response.body).not_to eq('')
-
-          expect(response.body).to include(gpg_key.key)
-          expect(response.body).not_to include(another_gpg_key.key)
-        end
-      end
-    end
-  end
 end

spec/controllers/users_controller_spec.rb

@@ -3,7 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe UsersController do
-  let(:user) { create(:user) }
+  # This user should have the same e-mail address associated with the GPG key prepared for tests
+  let(:user) { create(:user, email: GpgHelpers::User1.emails[0]) }
   let(:private_user) { create(:user, private_profile: true) }
   let(:public_user) { create(:user) }
 
@@ -326,6 +327,123 @@ RSpec.describe UsersController do
     end
   end
 
+  describe "#gpg_keys" do
+    describe "non existent user" do
+      it "does not generally work" do
+        get :gpg_keys, params: { username: 'not-existent' }
+
+        expect(response).not_to be_successful
+      end
+    end
+
+    describe "user with no keys" do
+      it "does generally work" do
+        get :gpg_keys, params: { username: user.username }
+
+        expect(response).to be_successful
+      end
+
+      it "renders all keys separated with a new line" do
+        get :gpg_keys, params: { username: user.username }
+
+        expect(response.body).to eq("")
+      end
+
+      it "responds with text/plain content type" do
+        get :gpg_keys, params: { username: user.username }
+
+        expect(response.content_type).to eq("text/plain")
+      end
+    end
+
+    describe "user with keys" do
+      let!(:gpg_key) { create(:gpg_key, user: user) }
+      let!(:another_gpg_key) { create(:another_gpg_key, user: user) }
+
+      describe "while signed in" do
+        before do
+          sign_in(user)
+        end
+
+        it "does generally work" do
+          get :gpg_keys, params: { username: user.username }
+
+          expect(response).to be_successful
+        end
+
+        it "renders all verified keys separated with a new line" do
+          get :gpg_keys, params: { username: user.username }
+
+          expect(response.body).not_to eq('')
+          expect(response.body).to eq(user.gpg_keys.select(&:verified?).map(&:key).join("\n"))
+
+          expect(response.body).to include(gpg_key.key)
+          expect(response.body).to include(another_gpg_key.key)
+        end
+
+        it "responds with text/plain content type" do
+          get :gpg_keys, params: { username: user.username }
+
+          expect(response.content_type).to eq("text/plain")
+        end
+      end
+
+      describe 'when logged out' do
+        before do
+          sign_out(user)
+        end
+
+        it "still does generally work" do
+          get :gpg_keys, params: { username: user.username }
+
+          expect(response).to be_successful
+        end
+
+        it "renders all verified keys separated with a new line" do
+          get :gpg_keys, params: { username: user.username }
+
+          expect(response.body).not_to eq('')
+          expect(response.body).to eq(user.gpg_keys.map(&:key).join("\n"))
+
+          expect(response.body).to include(gpg_key.key)
+          expect(response.body).to include(another_gpg_key.key)
+        end
+
+        it "responds with text/plain content type" do
+          get :gpg_keys, params: { username: user.username }
+
+          expect(response.content_type).to eq("text/plain")
+        end
+      end
+
+      describe 'when revoked' do
+        before do
+          sign_in(user)
+          another_gpg_key.revoke
+        end
+
+        it "doesn't render revoked keys" do
+          get :gpg_keys, params: { username: user.username }
+
+          expect(response.body).not_to eq('')
+
+          expect(response.body).to include(gpg_key.key)
+          expect(response.body).not_to include(another_gpg_key.key)
+        end
+
+        it "doesn't render revoked keys for non-authorized users" do
+          sign_out(user)
+          get :gpg_keys, params: { username: user.username }
+
+          expect(response.body).not_to eq('')
+
+          expect(response.body).to include(gpg_key.key)
+          expect(response.body).not_to include(another_gpg_key.key)
+        end
+      end
+    end
+  end
+
   describe 'GET #calendar' do
     context 'for user' do
       let(:project) { create(:project) }

spec/routing/routing_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 # user                       GET    /:username
 # user_ssh_keys              GET    /:username.keys
+# user_gpg_keys              GET    /:username.gpg
 # user_groups                GET    /users/:username/groups(.:format)
 # user_projects              GET    /users/:username/projects(.:format)
 # user_contributed_projects  GET    /users/:username/contributed(.:format)
@@ -17,6 +18,12 @@ RSpec.describe UsersController, "routing" do
     expect(get("/User")).to route_to('users#show', username: 'User')
   end
 
+  it "to #gpg_keys" do
+    allow_any_instance_of(::Constraints::UserUrlConstrainer).to receive(:matches?).and_return(true)
+
+    expect(get("/User.gpg")).to route_to('users#gpg_keys', username: 'User')
+  end
+
   it "to #groups" do
     expect(get("/users/User/groups")).to route_to('users#groups', username: 'User')
   end
@@ -197,12 +204,6 @@ RSpec.describe Profiles::GpgKeysController, "routing" do
   it "to #destroy" do
     expect(delete("/profile/gpg_keys/1")).to route_to('profiles/gpg_keys#destroy', id: '1')
   end
-
-  it "to #get_keys" do
-    allow_any_instance_of(::Constraints::UserUrlConstrainer).to receive(:matches?).and_return(true)
-
-    expect(get("/foo.gpg")).to route_to('profiles/gpg_keys#get_keys', username: 'foo')
-  end
 end
 
 #   emails GET    /emails(.:format)        emails#index