From 5ef06ac7eaf91c7ee4c32ed63c389b87aece5de8 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Thu, 26 Jul 2018 17:52:47 +1000 Subject: Move GL_PROTOCOL into GitlabNet --- lib/gitlab_net.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'lib/gitlab_net.rb') diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index 584dd93..f0696e3 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -16,6 +16,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength class NotFound < StandardError; end CHECK_TIMEOUT = 5 + GL_PROTOCOL = 'ssh'.freeze def check_access(cmd, gl_repository, repo, actor, changes, protocol, env: {}) changes = changes.join("\n") unless changes.is_a?(String) -- cgit v1.2.1 From 25e751e9eec5b5c685e1623fe3527235b75cd051 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Thu, 26 Jul 2018 18:20:23 +1000 Subject: Move HTTP related exceptions into HTTPHelper --- lib/gitlab_net.rb | 3 --- 1 file changed, 3 deletions(-) (limited to 'lib/gitlab_net.rb') diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index f0696e3..4124440 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -12,9 +12,6 @@ require_relative 'http_helper' class GitlabNet # rubocop:disable Metrics/ClassLength include HTTPHelper - class ApiUnreachableError < StandardError; end - class NotFound < StandardError; end - CHECK_TIMEOUT = 5 GL_PROTOCOL = 'ssh'.freeze -- cgit v1.2.1 From 99a65d67df3b77a0ae3bc6abc758708fbcc4de2a Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Thu, 26 Jul 2018 18:55:22 +1000 Subject: Use HTTP status constants in GitLabNet --- lib/gitlab_net.rb | 35 +++++++++++------------------------ 1 file changed, 11 insertions(+), 24 deletions(-) (limited to 'lib/gitlab_net.rb') diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index 4124440..2f704df 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -55,16 +55,10 @@ class GitlabNet # rubocop:disable Metrics/ClassLength end def lfs_authenticate(key, repo) - params = { - project: sanitize_path(repo), - key_id: key.gsub('key-', '') - } - + params = { project: sanitize_path(repo), key_id: key.gsub('key-', '') } resp = post("#{internal_api_endpoint}/lfs_authenticate", params) - if resp.code == '200' - GitlabLfsAuthentication.build_from_json(resp.body) - end + GitlabLfsAuthentication.build_from_json(resp.body) if resp.code == HTTP_SUCCESS end def broadcast_message @@ -79,11 +73,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength url += "&gl_repository=#{URI.escape(gl_repository)}" if gl_repository resp = get(url) - if resp.code == '200' - JSON.parse(resp.body) - else - [] - end + resp.code == HTTP_SUCCESS ? JSON.parse(resp.body) : [] rescue [] end @@ -94,7 +84,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength def authorized_key(key) resp = get("#{internal_api_endpoint}/authorized_keys?key=#{URI.escape(key, '+/=')}") - JSON.parse(resp.body) if resp.code == "200" + JSON.parse(resp.body) if resp.code == HTTP_SUCCESS rescue nil end @@ -102,8 +92,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength def two_factor_recovery_codes(key) key_id = key.gsub('key-', '') resp = post("#{internal_api_endpoint}/two_factor_recovery_codes", key_id: key_id) - - JSON.parse(resp.body) if resp.code == '200' + JSON.parse(resp.body) if resp.code == HTTP_SUCCESS rescue {} end @@ -112,7 +101,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength params = { gl_repository: gl_repository, project: repo_path } resp = post("#{internal_api_endpoint}/notify_post_receive", params) - resp.code == '200' + resp.code == HTTP_SUCCESS rescue false end @@ -124,21 +113,19 @@ class GitlabNet # rubocop:disable Metrics/ClassLength changes: changes } resp = post("#{internal_api_endpoint}/post_receive", params) + raise NotFound if resp.code == HTTP_NOT_FOUND - raise NotFound if resp.code == '404' - - JSON.parse(resp.body) if resp.code == '200' + JSON.parse(resp.body) if resp.code == HTTP_SUCCESS end def pre_receive(gl_repository) resp = post("#{internal_api_endpoint}/pre_receive", gl_repository: gl_repository) + raise NotFound if resp.code == HTTP_NOT_FOUND - raise NotFound if resp.code == '404' - - JSON.parse(resp.body) if resp.code == '200' + JSON.parse(resp.body) if resp.code == HTTP_SUCCESS end - protected + private def sanitize_path(repo) repo.delete("'") -- cgit v1.2.1 From 3087f411ee9f7716a400e80cf6f965265116dd8e Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Thu, 26 Jul 2018 18:58:39 +1000 Subject: actor is always key_id in GitlabNet --- lib/gitlab_net.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'lib/gitlab_net.rb') diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index 2f704df..de95d9d 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -15,24 +15,19 @@ class GitlabNet # rubocop:disable Metrics/ClassLength CHECK_TIMEOUT = 5 GL_PROTOCOL = 'ssh'.freeze - def check_access(cmd, gl_repository, repo, actor, changes, protocol, env: {}) + def check_access(cmd, gl_repository, repo, key_id, changes, protocol, env: {}) changes = changes.join("\n") unless changes.is_a?(String) params = { action: cmd, changes: changes, gl_repository: gl_repository, + key_id: key_id.gsub('key-', ''), project: sanitize_path(repo), protocol: protocol, env: env } - if actor =~ /\Akey\-\d+\Z/ - params[:key_id] = actor.gsub("key-", "") - elsif actor =~ /\Auser\-\d+\Z/ - params[:user_id] = actor.gsub("user-", "") - end - url = "#{internal_api_endpoint}/allowed" resp = post(url, params) -- cgit v1.2.1 From c4d63ab07f83c93b5e80a81db5a6bb926b6930c7 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Thu, 26 Jul 2018 19:01:27 +1000 Subject: Tidy up GitlabNet * Remove HTTP related requires * Make protocol = GL_PROTOCOL the default --- lib/gitlab_net.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'lib/gitlab_net.rb') diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index de95d9d..191ecf5 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -1,12 +1,8 @@ -require 'net/http' -require 'openssl' require 'json' -require_relative 'gitlab_config' require_relative 'gitlab_logger' require_relative 'gitlab_access' require_relative 'gitlab_lfs_authentication' -require_relative 'httpunix' require_relative 'http_helper' class GitlabNet # rubocop:disable Metrics/ClassLength @@ -15,7 +11,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength CHECK_TIMEOUT = 5 GL_PROTOCOL = 'ssh'.freeze - def check_access(cmd, gl_repository, repo, key_id, changes, protocol, env: {}) + def check_access(cmd, gl_repository, repo, key_id, changes, protocol = GL_PROTOCOL, env: {}) changes = changes.join("\n") unless changes.is_a?(String) params = { @@ -28,8 +24,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength env: env } - url = "#{internal_api_endpoint}/allowed" - resp = post(url, params) + resp = post("#{internal_api_endpoint}/allowed", params) if resp.code == '200' GitAccessStatus.create_from_json(resp.body) -- cgit v1.2.1 From ecba183b60eb0798ecf1736659ed7d0a995d0f01 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Thu, 26 Jul 2018 19:06:20 +1000 Subject: Utilise new Actions * Move gitaly, git-lfs and 2FA logic out from gitlab_shell.rb * Streamline parsing of origin_cmd in GitlabShell * Utilise proper HTTP status codes sent from the API * Also support 200 OK with status of true/false (ideally get rid of this) * Use HTTP status constants * Use attr_reader definitions (var over @var) * Rspec deprecation fixes --- lib/gitlab_net.rb | 47 ++++++++++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 17 deletions(-) (limited to 'lib/gitlab_net.rb') diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index 191ecf5..3bc8f2d 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -1,15 +1,18 @@ require 'json' +require_relative 'errors' require_relative 'gitlab_logger' require_relative 'gitlab_access' require_relative 'gitlab_lfs_authentication' require_relative 'http_helper' +require_relative 'action' -class GitlabNet # rubocop:disable Metrics/ClassLength +class GitlabNet include HTTPHelper CHECK_TIMEOUT = 5 GL_PROTOCOL = 'ssh'.freeze + API_INACCESSIBLE_ERROR = 'API is not accessible'.freeze def check_access(cmd, gl_repository, repo, key_id, changes, protocol = GL_PROTOCOL, env: {}) changes = changes.join("\n") unless changes.is_a?(String) @@ -26,22 +29,15 @@ class GitlabNet # rubocop:disable Metrics/ClassLength resp = post("#{internal_api_endpoint}/allowed", params) - if resp.code == '200' - GitAccessStatus.create_from_json(resp.body) - else - GitAccessStatus.new(false, - 'API is not accessible', - gl_repository: nil, - gl_username: nil, - repository_path: nil, - gitaly: nil) - end + determine_action(key_id, resp) end def discover(key) key_id = key.gsub("key-", "") resp = get("#{internal_api_endpoint}/discover?key_id=#{key_id}") - JSON.parse(resp.body) rescue nil + JSON.parse(resp.body) + rescue JSON::ParserError, ApiUnreachableError + nil end def lfs_authenticate(key, repo) @@ -97,11 +93,7 @@ class GitlabNet # rubocop:disable Metrics/ClassLength end def post_receive(gl_repository, identifier, changes) - params = { - gl_repository: gl_repository, - identifier: identifier, - changes: changes - } + params = { gl_repository: gl_repository, identifier: identifier, changes: changes } resp = post("#{internal_api_endpoint}/post_receive", params) raise NotFound if resp.code == HTTP_NOT_FOUND @@ -120,4 +112,25 @@ class GitlabNet # rubocop:disable Metrics/ClassLength def sanitize_path(repo) repo.delete("'") end + + def determine_action(key_id, resp) + json = JSON.parse(resp.body) + message = json['message'] + + case resp.code + when HTTP_SUCCESS + # TODO: This raise can be removed once internal API can respond with correct + # HTTP status codes, instead of relying upon parsing the body and + # accessing the 'status' key. + raise AccessDeniedError, message unless json['status'] + + Action::Gitaly.create_from_json(key_id, json) + when HTTP_UNAUTHORIZED, HTTP_NOT_FOUND + raise AccessDeniedError, message + else + raise UnknownError, "#{API_INACCESSIBLE_ERROR}: #{message}" + end + rescue JSON::ParserError + raise UnknownError, API_INACCESSIBLE_ERROR + end end -- cgit v1.2.1 From be5b38f5338a7a4fc48229990480b144a1499903 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Tue, 31 Jul 2018 10:49:41 +1000 Subject: Rename NotFound -> NotFoundError --- lib/gitlab_net.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/gitlab_net.rb') diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index 3bc8f2d..29a91f4 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -95,14 +95,14 @@ class GitlabNet def post_receive(gl_repository, identifier, changes) params = { gl_repository: gl_repository, identifier: identifier, changes: changes } resp = post("#{internal_api_endpoint}/post_receive", params) - raise NotFound if resp.code == HTTP_NOT_FOUND + raise NotFoundError if resp.code == HTTP_NOT_FOUND JSON.parse(resp.body) if resp.code == HTTP_SUCCESS end def pre_receive(gl_repository) resp = post("#{internal_api_endpoint}/pre_receive", gl_repository: gl_repository) - raise NotFound if resp.code == HTTP_NOT_FOUND + raise NotFoundError if resp.code == HTTP_NOT_FOUND JSON.parse(resp.body) if resp.code == HTTP_SUCCESS end -- cgit v1.2.1 From 4c4d9f5ef4a2e3ac16d0b02e18b19ba513849f57 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Tue, 31 Jul 2018 21:06:56 +1000 Subject: Use actor when we don't know if it's a Key or User * Use gl_id when we don't know if it's a key-X or user-X * Use Actor.new_from(gl_id) which will figure out if it's a Key or User * Use key_str when we're referring to key-X as key_id is confusing --- lib/gitlab_net.rb | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) (limited to 'lib/gitlab_net.rb') diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index 29a91f4..34e2ff6 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -14,34 +14,34 @@ class GitlabNet GL_PROTOCOL = 'ssh'.freeze API_INACCESSIBLE_ERROR = 'API is not accessible'.freeze - def check_access(cmd, gl_repository, repo, key_id, changes, protocol = GL_PROTOCOL, env: {}) + def check_access(cmd, gl_repository, repo, actor, changes, protocol = GL_PROTOCOL, env: {}) changes = changes.join("\n") unless changes.is_a?(String) params = { action: cmd, changes: changes, gl_repository: gl_repository, - key_id: key_id.gsub('key-', ''), project: sanitize_path(repo), protocol: protocol, env: env } + params[actor.class.identifier_key.to_sym] = actor.id + resp = post("#{internal_api_endpoint}/allowed", params) - determine_action(key_id, resp) + determine_action(actor, resp) end - def discover(key) - key_id = key.gsub("key-", "") + def discover(key_id) resp = get("#{internal_api_endpoint}/discover?key_id=#{key_id}") JSON.parse(resp.body) rescue JSON::ParserError, ApiUnreachableError nil end - def lfs_authenticate(key, repo) - params = { project: sanitize_path(repo), key_id: key.gsub('key-', '') } + def lfs_authenticate(key_id, repo) + params = { project: sanitize_path(repo), key_id: key_id } resp = post("#{internal_api_endpoint}/lfs_authenticate", params) GitlabLfsAuthentication.build_from_json(resp.body) if resp.code == HTTP_SUCCESS @@ -68,15 +68,14 @@ class GitlabNet get("#{internal_api_endpoint}/check", options: { read_timeout: CHECK_TIMEOUT }) end - def authorized_key(key) - resp = get("#{internal_api_endpoint}/authorized_keys?key=#{URI.escape(key, '+/=')}") + def authorized_key(full_key) + resp = get("#{internal_api_endpoint}/authorized_keys?key=#{URI.escape(full_key, '+/=')}") JSON.parse(resp.body) if resp.code == HTTP_SUCCESS rescue nil end - def two_factor_recovery_codes(key) - key_id = key.gsub('key-', '') + def two_factor_recovery_codes(key_id) resp = post("#{internal_api_endpoint}/two_factor_recovery_codes", key_id: key_id) JSON.parse(resp.body) if resp.code == HTTP_SUCCESS rescue @@ -92,8 +91,8 @@ class GitlabNet false end - def post_receive(gl_repository, identifier, changes) - params = { gl_repository: gl_repository, identifier: identifier, changes: changes } + def post_receive(gl_repository, actor, changes) + params = { gl_repository: gl_repository, identifier: actor.identifier, changes: changes } resp = post("#{internal_api_endpoint}/post_receive", params) raise NotFoundError if resp.code == HTTP_NOT_FOUND @@ -113,7 +112,7 @@ class GitlabNet repo.delete("'") end - def determine_action(key_id, resp) + def determine_action(actor, resp) json = JSON.parse(resp.body) message = json['message'] @@ -124,7 +123,7 @@ class GitlabNet # accessing the 'status' key. raise AccessDeniedError, message unless json['status'] - Action::Gitaly.create_from_json(key_id, json) + Action::Gitaly.create_from_json(actor, json) when HTTP_UNAUTHORIZED, HTTP_NOT_FOUND raise AccessDeniedError, message else -- cgit v1.2.1 From 5709ea1a3435ca7e1308d2fa54bfbadc61682fc7 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Wed, 1 Aug 2018 13:46:11 +1000 Subject: GitlatNet#discover only parse JSON if a 200 --- lib/gitlab_net.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/gitlab_net.rb') diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index 0a3e383..9ea18aa 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -35,7 +35,7 @@ class GitlabNet def discover(actor) resp = get("#{internal_api_endpoint}/discover?#{actor.identifier_key}=#{actor.id}") - JSON.parse(resp.body) + JSON.parse(resp.body) if resp.code == HTTP_SUCCESS rescue JSON::ParserError, ApiUnreachableError nil end -- cgit v1.2.1