diff --git a/app/controllers/profiles/keys_controller.rb b/app/controllers/profiles/keys_controller.rb index b9cb71ae89a003f830c988914db2463f387409fe..99e1b9027fa5f1e16cb5e8b673c6b2639043dc68 100644 --- a/app/controllers/profiles/keys_controller.rb +++ b/app/controllers/profiles/keys_controller.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class Profiles::KeysController < Profiles::ApplicationController - skip_before_action :authenticate_user!, only: [:get_keys] - def index @keys = current_user.keys.order_id_desc @key = Key.new @@ -33,25 +31,6 @@ class Profiles::KeysController < Profiles::ApplicationController end end - # Get all keys of a user(params[:username]) in a text format - # Helpful for sysadmins to put in respective servers - def get_keys - if params[:username].present? - begin - user = UserFinder.new(params[:username]).find_by_username - if user.present? - render plain: user.all_ssh_keys.join("\n") - else - return render_404 - end - rescue => e - render html: e.message - end - else - return render_404 - end - end - private def key_params diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 5ee978850716f207ed473e21a1d9f38500266415..3cc07585c3fbdaa86c448021719f757fbcd46f0e 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -36,6 +36,12 @@ class UsersController < ApplicationController end end + # Get all keys of a user(params[:username]) in a text format + # Helpful for sysadmins to put in respective servers + def ssh_keys + render plain: user.all_ssh_keys.join("\n") + end + def activity respond_to do |format| format.html { render 'show' } diff --git a/changelogs/unreleased/move-profiles-keys-get-keys-to-users.yml b/changelogs/unreleased/move-profiles-keys-get-keys-to-users.yml new file mode 100644 index 0000000000000000000000000000000000000000..d36674be345fd709c078fbbf44e7b9319d751d1e --- /dev/null +++ b/changelogs/unreleased/move-profiles-keys-get-keys-to-users.yml @@ -0,0 +1,5 @@ +--- +title: Move profiles/keys#get_keys to users#ssh_keys +merge_request: 35507 +author: Takuya Noguchi +type: other diff --git a/config/routes/user.rb b/config/routes/user.rb index 9db3a71a2708ce9563a883d7f43229120fce001b..3bf13908d396ead1a2efedffa370789b1c1ed134 100644 --- a/config/routes/user.rb +++ b/config/routes/user.rb @@ -56,7 +56,7 @@ end constraints(::Constraints::UserUrlConstrainer.new) do # Get all keys of user - get ':username.keys' => 'profiles/keys#get_keys', constraints: { username: Gitlab::PathRegex.root_namespace_route_regex } + get ':username.keys', controller: :users, action: :ssh_keys, constraints: { username: Gitlab::PathRegex.root_namespace_route_regex } scope(path: ':username', as: :user, diff --git a/spec/controllers/profiles/keys_controller_spec.rb b/spec/controllers/profiles/keys_controller_spec.rb index 258ed62262ad293049f22f52d4408c1fda553f08..66f6135df1ec56801e10f310f83c711d300fdf12 100644 --- a/spec/controllers/profiles/keys_controller_spec.rb +++ b/spec/controllers/profiles/keys_controller_spec.rb @@ -20,69 +20,4 @@ RSpec.describe Profiles::KeysController do expect(Key.last.expires_at).to be_like_time(expires_at) 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!(:key) { create(:key, user: user) } - let!(:another_key) { create(:another_key, user: user) } - let!(:deploy_key) { create(:deploy_key, user: user) } - - it "does generally work" do - get :get_keys, params: { username: user.username } - - expect(response).to be_successful - end - - it "renders all non deploy 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.all_ssh_keys.join("\n")) - - expect(response.body).to include(key.key.sub(' dummy@gitlab.com', '')) - expect(response.body).to include(another_key.key.sub(' dummy@gitlab.com', '')) - - expect(response.body).not_to include(deploy_key.key) - end - - it "does not render the comment of the key" do - get :get_keys, params: { username: user.username } - - expect(response.body).not_to match(/dummy@gitlab.com/) - 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 - end end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index bec4b24484a395eff00eb5c9cf053cb1feab6ada..99c3b82bd0d68447d15a4534fe0c6344e7dbe53d 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -114,6 +114,71 @@ RSpec.describe UsersController do end end + describe "#ssh_keys" do + describe "non existent user" do + it "does not generally work" do + get :ssh_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 :ssh_keys, params: { username: user.username } + + expect(response).to be_successful + end + + it "renders all keys separated with a new line" do + get :ssh_keys, params: { username: user.username } + + expect(response.body).to eq("") + end + + it "responds with text/plain content type" do + get :ssh_keys, params: { username: user.username } + expect(response.content_type).to eq("text/plain") + end + end + + describe "user with keys" do + let!(:key) { create(:key, user: user) } + let!(:another_key) { create(:another_key, user: user) } + let!(:deploy_key) { create(:deploy_key, user: user) } + + it "does generally work" do + get :ssh_keys, params: { username: user.username } + + expect(response).to be_successful + end + + it "renders all non deploy keys separated with a new line" do + get :ssh_keys, params: { username: user.username } + + expect(response.body).not_to eq('') + expect(response.body).to eq(user.all_ssh_keys.join("\n")) + + expect(response.body).to include(key.key.sub(' dummy@gitlab.com', '')) + expect(response.body).to include(another_key.key.sub(' dummy@gitlab.com', '')) + + expect(response.body).not_to include(deploy_key.key) + end + + it "does not render the comment of the key" do + get :ssh_keys, params: { username: user.username } + + expect(response.body).not_to match(/dummy@gitlab.com/) + end + + it "responds with text/plain content type" do + get :ssh_keys, params: { username: user.username } + + expect(response.content_type).to eq("text/plain") + end + end + end + describe 'GET #calendar' do context 'for user' do let(:project) { create(:project) } diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb index a6f415a995a583ed0dfa03f56470142f3e7a5a8a..1218ae307812a8819b96f6f10acbd7ae45e7cbb9 100644 --- a/spec/routing/routing_spec.rb +++ b/spec/routing/routing_spec.rb @@ -32,6 +32,13 @@ RSpec.describe UsersController, "routing" do expect(get("/users/User/snippets")).to route_to('users#snippets', username: 'User') end + # get all the ssh-keys of a user + it "to #get_keys" do + allow_any_instance_of(::Constraints::UserUrlConstrainer).to receive(:matches?).and_return(true) + + expect(get("/User.keys")).to route_to('users#ssh_keys', username: 'User') + end + it "to #calendar" do expect(get("/users/User/calendar")).to route_to('users#calendar', username: 'User') end @@ -190,13 +197,6 @@ RSpec.describe Profiles::KeysController, "routing" do it "to #destroy" do expect(delete("/profile/keys/1")).to route_to('profiles/keys#destroy', id: '1') end - - # get all the ssh-keys of a user - it "to #get_keys" do - allow_any_instance_of(::Constraints::UserUrlConstrainer).to receive(:matches?).and_return(true) - - expect(get("/foo.keys")).to route_to('profiles/keys#get_keys', username: 'foo') - end end # emails GET /emails(.:format) emails#index