From a2021e0bb3ca7de217ba3547909a5f8db84831df Mon Sep 17 00:00:00 2001
From: Igor Drozdov <idrozdov@gitlab.com>
Date: Fri, 15 Dec 2023 17:19:50 +0100
Subject: [PATCH] Refactor find certificate logic into a service
This service/logic is going to be reused
---
.../groups/ssh_certificates/find_service.rb | 35 +++++++
ee/lib/ee/api/internal/base.rb | 10 +-
ee/spec/requests/api/internal/base_spec.rb | 10 +-
.../ssh_certificates/find_service_spec.rb | 92 +++++++++++++++++++
4 files changed, 135 insertions(+), 12 deletions(-)
create mode 100644 ee/app/services/groups/ssh_certificates/find_service.rb
create mode 100644 ee/spec/services/groups/ssh_certificates/find_service_spec.rb
diff --git a/ee/app/services/groups/ssh_certificates/find_service.rb b/ee/app/services/groups/ssh_certificates/find_service.rb
new file mode 100644
index 000000000000..4b70f09017cf
--- /dev/null
+++ b/ee/app/services/groups/ssh_certificates/find_service.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+module Groups
+ module SshCertificates
+ class FindService
+ def initialize(ca_fingerprint, user_identifier)
+ @ca_fingerprint = ca_fingerprint
+ @user_identifier = user_identifier
+ end
+
+ def execute
+ certificate = ::Groups::SshCertificate.find_by_fingerprint(ca_fingerprint)
+ return error('Certificate Not Found', :not_found) unless certificate
+
+ group = certificate.group
+ return error('Feature is not available', :forbidden) unless group.licensed_feature_available?(:ssh_certificates)
+
+ user = group.users.find_by_login(user_identifier)
+
+ return error('User Not Found', :not_found) unless user
+ return error('Not an Enterprise User of the group', :forbidden) unless user.enterprise_user_of_group?(group)
+
+ ServiceResponse.success(payload: { user: user, group: group })
+ end
+
+ private
+
+ attr_reader :ca_fingerprint, :user_identifier
+
+ def error(message, reason)
+ ServiceResponse.error(message: message, reason: reason)
+ end
+ end
+ end
+end
diff --git a/ee/lib/ee/api/internal/base.rb b/ee/lib/ee/api/internal/base.rb
index 53d27576d7ea..0a212d470cc8 100644
--- a/ee/lib/ee/api/internal/base.rb
+++ b/ee/lib/ee/api/internal/base.rb
@@ -88,15 +88,11 @@ def two_factor_push_otp_check
namespace 'internal' do
get '/authorized_certs', feature_category: :source_code_management, urgency: :high do
- certificate = ::Groups::SshCertificate.find_by_fingerprint!(params[:key])
- group = certificate.group
+ response = ::Groups::SshCertificates::FindService.new(params[:key], params[:user_identifier]).execute
- forbidden!('Feature is not available') unless group.licensed_feature_available?(:ssh_certificates)
+ render_api_error!(response.message, response.reason) if response.error?
- user = group.users.find_by_login(params[:user_identifier])
-
- not_found!('User') unless user
- forbidden!('Not an Enterprise User of the group') unless user.enterprise_user_of_group?(group)
+ group, user = response.payload.values_at(:group, :user)
status 200
diff --git a/ee/spec/requests/api/internal/base_spec.rb b/ee/spec/requests/api/internal/base_spec.rb
index 0baa1d98ad80..581919183822 100644
--- a/ee/spec/requests/api/internal/base_spec.rb
+++ b/ee/spec/requests/api/internal/base_spec.rb
@@ -1038,7 +1038,7 @@ def lfs_auth_user(user_id, project)
get(api('/internal/authorized_certs'), params: params, headers: gitlab_shell_internal_api_request_header)
expect(response).to have_gitlab_http_status(:forbidden)
- expect(json_response['message']).to eq('403 Forbidden - Not an Enterprise User of the group')
+ expect(json_response['message']).to eq('Not an Enterprise User of the group')
end
end
@@ -1062,7 +1062,7 @@ def lfs_auth_user(user_id, project)
get(api('/internal/authorized_certs'), params: params, headers: gitlab_shell_internal_api_request_header)
expect(response).to have_gitlab_http_status(:not_found)
- expect(json_response['message']).to eq('404 Not found')
+ expect(json_response['message']).to eq('Certificate Not Found')
end
end
@@ -1073,7 +1073,7 @@ def lfs_auth_user(user_id, project)
get(api('/internal/authorized_certs'), params: params, headers: gitlab_shell_internal_api_request_header)
expect(response).to have_gitlab_http_status(:not_found)
- expect(json_response['message']).to eq('404 User Not Found')
+ expect(json_response['message']).to eq('User Not Found')
end
end
end
@@ -1083,7 +1083,7 @@ def lfs_auth_user(user_id, project)
get(api('/internal/authorized_certs'), params: params, headers: gitlab_shell_internal_api_request_header)
expect(response).to have_gitlab_http_status(:not_found)
- expect(json_response['message']).to eq('404 User Not Found')
+ expect(json_response['message']).to eq('User Not Found')
end
end
@@ -1094,7 +1094,7 @@ def lfs_auth_user(user_id, project)
get(api('/internal/authorized_certs'), params: params, headers: gitlab_shell_internal_api_request_header)
expect(response).to have_gitlab_http_status(:forbidden)
- expect(json_response['message']).to eq('403 Forbidden - Feature is not available')
+ expect(json_response['message']).to eq('Feature is not available')
end
end
end
diff --git a/ee/spec/services/groups/ssh_certificates/find_service_spec.rb b/ee/spec/services/groups/ssh_certificates/find_service_spec.rb
new file mode 100644
index 000000000000..31478438a34c
--- /dev/null
+++ b/ee/spec/services/groups/ssh_certificates/find_service_spec.rb
@@ -0,0 +1,92 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Groups::SshCertificates::FindService, feature_category: :source_code_management do
+ let_it_be(:ssh_certificate) { create(:group_ssh_certificate) }
+ let_it_be(:group) { ssh_certificate.group }
+ let_it_be(:user) { create(:user, :enterprise_user, enterprise_group: group) }
+
+ let(:ca_fingerprint) { ssh_certificate.fingerprint }
+ let(:user_identifier) { user.username }
+ let(:service) { described_class.new(ca_fingerprint, user_identifier) }
+
+ before_all do
+ group.add_developer(user)
+ end
+
+ before do
+ stub_licensed_features(ssh_certificates: true)
+ end
+
+ describe '#execute' do
+ it 'returns successful response with payload' do
+ response = service.execute
+
+ expect(response).to be_success
+ expect(response.payload).to eq({ user: user, group: group })
+ end
+
+ context 'when a certificate not found' do
+ let(:ca_fingerprint) { 'does not exist' }
+
+ it 'returns not found error' do
+ response = service.execute
+
+ expect(response).to be_error
+ expect(response.message).to eq('Certificate Not Found')
+ expect(response.reason).to eq(:not_found)
+ end
+ end
+
+ context 'when ssh_certificates feature is not available' do
+ it 'returns forbidden error' do
+ stub_licensed_features(ssh_certificates: false)
+
+ response = service.execute
+
+ expect(response).to be_error
+ expect(response.message).to eq('Feature is not available')
+ expect(response.reason).to eq(:forbidden)
+ end
+ end
+
+ context 'when a user is not found' do
+ let(:user_identifier) { 'does not exist' }
+
+ it 'returns not found error' do
+ response = service.execute
+
+ expect(response).to be_error
+ expect(response.message).to eq('User Not Found')
+ expect(response.reason).to eq(:not_found)
+ end
+ end
+
+ context 'when a user is not a member' do
+ let_it_be(:user) { create(:user) }
+
+ it 'returns not found error' do
+ response = service.execute
+
+ expect(response).to be_error
+ expect(response.message).to eq('User Not Found')
+ expect(response.reason).to eq(:not_found)
+ end
+ end
+
+ context 'when a user is not an enterprise user' do
+ let_it_be(:user) { create(:user) }
+
+ it 'returns not found error' do
+ group.add_developer(user)
+
+ response = service.execute
+
+ expect(response).to be_error
+ expect(response.message).to eq('Not an Enterprise User of the group')
+ expect(response.reason).to eq(:forbidden)
+ end
+ end
+ end
+end
--
GitLab