diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 34857e2e1..042504915 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -196,7 +196,7 @@ race: - .go-matrix-job - .test-job script: - - make test_golang_race + - make test_race code_quality: stage: lint diff --git a/.ruby-version b/.ruby-version deleted file mode 100644 index 5f6fc5edc..000000000 --- a/.ruby-version +++ /dev/null @@ -1 +0,0 @@ -3.3.10 diff --git a/.tool-versions b/.tool-versions index bd35a1981..3f271094c 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,3 +1,2 @@ -ruby 3.3.10 golang 1.24.5 golangci-lint 2.3.1 diff --git a/Gemfile b/Gemfile index ba8431642..ab6f974ba 100644 --- a/Gemfile +++ b/Gemfile @@ -1,11 +1,5 @@ source 'https://rubygems.org' -group :development, :test do - gem 'base64', '~> 0.3.0' - gem 'rspec', '~> 3.13.1' - gem 'webrick', '~> 1.9', '>= 1.9.1' -end - group :development, :danger do gem 'gitlab-dangerfiles', '~> 4.9.2' end diff --git a/Gemfile.lock b/Gemfile.lock index 16f99a22c..3f998e03c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -31,7 +31,6 @@ GEM danger-gitlab (8.0.0) danger gitlab (~> 4.2, >= 4.2.0) - diff-lcs (1.5.1) faraday (2.13.1) faraday-net_http (>= 2.0, < 3.5) json @@ -75,19 +74,6 @@ GEM rake (13.2.1) rchardet (1.9.0) rexml (3.4.1) - rspec (3.13.1) - rspec-core (~> 3.13.0) - rspec-expectations (~> 3.13.0) - rspec-mocks (~> 3.13.0) - rspec-core (3.13.4) - rspec-support (~> 3.13.0) - rspec-expectations (3.13.5) - diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.13.0) - rspec-mocks (3.13.5) - diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.13.0) - rspec-support (3.13.4) sawyer (0.9.2) addressable (>= 2.3.5) faraday (>= 0.17.3, < 3) @@ -95,16 +81,12 @@ GEM unicode-display_width (>= 1.1.1, < 3) unicode-display_width (2.6.0) uri (0.13.2) - webrick (1.9.1) PLATFORMS ruby DEPENDENCIES - base64 (~> 0.3.0) gitlab-dangerfiles (~> 4.9.2) - rspec (~> 3.13.1) - webrick (~> 1.9, >= 1.9.1) BUNDLED WITH 2.6.5 diff --git a/Makefile b/Makefile index 0782aaeef..6bd424711 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: validate verify verify_ruby verify_golang test test_ruby test_golang test_fancy test_golang_fancy coverage coverage_golang setup _script_install make_necessary_dirs build compile check clean install lint +.PHONY: validate verify verify_ruby verify_golang test test_fancy coverage coverage_golang setup _script_install make_necessary_dirs build compile check clean install lint FIPS_MODE ?= 0 OS := $(shell uname | tr A-Z a-z) @@ -56,26 +56,16 @@ build: compile validate: verify test -verify: verify_golang - -verify_golang: +verify: gofmt -s -l $(GO_SOURCES) | awk '{ print } END { if (NR > 0) { print "Please run make fmt"; exit 1 } }' fmt: gofmt -w -s $(GO_SOURCES) -test: test_ruby test_golang - -test_fancy: test_ruby test_golang_fancy - -# The Ruby tests are now all integration specs that test the Go implementation. -test_ruby: - bundle exec rspec --color --format d spec - -test_golang: +test: go test -cover -coverprofile=cover.out -count 1 -tags "$(GO_TAGS)" ./... -test_golang_fancy: ${GOTESTSUM_FILE} +test_fancy: ${GOTESTSUM_FILE} @${GOTESTSUM_FILE} --version @${GOTESTSUM_FILE} --junitfile ./cover.xml --format pkgname -- -coverprofile=./cover.out -covermode=atomic -count 1 -tags "$(GO_TAGS)" ./... @@ -83,12 +73,10 @@ ${GOTESTSUM_FILE}: mkdir -p $(shell dirname ${GOTESTSUM_FILE}) curl -L https://github.com/gotestyourself/gotestsum/releases/download/v${GOTESTSUM_VERSION}/gotestsum_${GOTESTSUM_VERSION}_${OS}_${ARCH}.tar.gz | tar -zOxf - gotestsum > ${GOTESTSUM_FILE} && chmod +x ${GOTESTSUM_FILE} -test_golang_race: +test_race: go test -race -count 1 ./... -coverage: coverage_golang - -coverage_golang: +coverage: [ -f cover.out ] && go tool cover -func cover.out lint: diff --git a/README.md b/README.md index 983bbfc6d..40fd98c8b 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,6 @@ Development documentation for GitLab Shell [has moved into the `gitlab` reposito | `client/` | HTTP and GitLab client logic that is used internally and by other modules, e.g. Gitaly. | | `bin/` | Compiled binaries are created here. | | `support/` | Scripts and tools that assist in development and/or testing. | -| `spec/` | Ruby based integration tests. | ## Building diff --git a/cmd/gitlab-sshd/acceptance_test.go b/cmd/gitlab-sshd/acceptance_test.go index 506e27df4..6f40e5a7c 100644 --- a/cmd/gitlab-sshd/acceptance_test.go +++ b/cmd/gitlab-sshd/acceptance_test.go @@ -473,23 +473,45 @@ func TwoFactorAuthVerifySuccess(t *testing.T) { } func TestGitLfsAuthenticateSuccess(t *testing.T) { - handler := customHandler{ - url: "/api/v4/internal/lfs_authenticate", - caller: func(w http.ResponseWriter, _ *http.Request) { - fmt.Fprint(w, `{"username": "test-user", "lfs_token": "testlfstoken", "repo_path": "foo", "expires_in": 7200}`) - }, - } - client := runSSHD(t, "ed25519", successAPI(t, handler)) + t.Run("lfs is successfully authed when a correct user is provided", func(t *testing.T) { + handler := customHandler{ + url: "/api/v4/internal/lfs_authenticate", + caller: func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `{"username": "test-user", "lfs_token": "testlfstoken", "repo_path": "foo", "expires_in": 7200}`) + }, + } + client := runSSHD(t, "ed25519", successAPI(t, handler)) - session, err := client.NewSession() - require.NoError(t, err) - defer session.Close() + session, err := client.NewSession() + require.NoError(t, err) + defer session.Close() - output, err := session.Output("git-lfs-authenticate test-user/repo.git download") + output, err := session.Output("git-lfs-authenticate test-user/repo.git download") - require.NoError(t, err) - require.JSONEq(t, `{"header":{"Authorization":"Basic dGVzdC11c2VyOnRlc3RsZnN0b2tlbg=="},"href":"/info/lfs","expires_in":7200} + require.NoError(t, err) + require.JSONEq(t, `{"header":{"Authorization":"Basic dGVzdC11c2VyOnRlc3RsZnN0b2tlbg=="},"href":"/info/lfs","expires_in":7200} `, string(output)) + }) + + t.Run("lfs is not authenticated when a user is not allowed to perform an action", func(t *testing.T) { + handler := customHandler{ + url: "/api/v4/internal/lfs_authenticate", + caller: func(w http.ResponseWriter, _ *http.Request) { + http.Error(w, "forbidden", http.StatusForbidden) + }, + } + client := runSSHD(t, "ed25519", successAPI(t, handler)) + session, err := client.NewSession() + require.NoError(t, err) + defer session.Close() + + output, err := session.Output("git-lfs-authenticate test-user/repo.git download") + + // we don't send back an error + require.NoError(t, err) + // we also ensure that we don't send back any output + require.Empty(t, string(output)) + }) } func TestGitReceivePackSuccess(t *testing.T) { diff --git a/lefthook.yml b/lefthook.yml index e3e1f6860..4cf1df6de 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -2,6 +2,5 @@ pre-push: commands: test: run: make test - # Disabled for now as there's _lots_ of linter errors - #lint: - # run: make lint + lint: + run: make lint diff --git a/spec/gitlab_shell_lfs_authentication_spec.rb b/spec/gitlab_shell_lfs_authentication_spec.rb deleted file mode 100644 index 437005c3d..000000000 --- a/spec/gitlab_shell_lfs_authentication_spec.rb +++ /dev/null @@ -1,128 +0,0 @@ -require_relative 'spec_helper' - -require 'open3' - -describe 'bin/gitlab-shell git-lfs-authentication' do - include_context 'gitlab shell' - - let(:path) { "https://gitlab.com/repo/path" } - let(:env) { {'SSH_CONNECTION' => 'fake', 'SSH_ORIGINAL_COMMAND' => 'git-lfs-authenticate project/repo download' } } - - before(:context) do - write_config("gitlab_url" => "http+unix://#{CGI.escape(tmp_socket_path)}") - end - - def mock_server(server) - server.mount_proc('/api/v4/internal/lfs_authenticate') do |req, res| - res.content_type = 'application/json' - - key_id = req.query['key_id'] || req.query['user_id'] - - unless key_id - body = JSON.parse(req.body) - key_id = body['key_id'] || body['user_id'].to_s - end - - if key_id == '100' - res.status = 200 - res.body = %{{"username":"john","lfs_token":"sometoken","repository_http_path":"#{path}","expires_in":1800}} - else - res.status = 403 - end - end - - server.mount_proc('/api/v4/internal/allowed') do |req, res| - res.content_type = 'application/json' - - key_id = req.query['key_id'] || req.query['username'] - - unless key_id - body = JSON.parse(req.body) - key_id = body['key_id'] || body['username'].to_s - end - - case key_id - when '100', 'someone' then - res.status = 200 - res.body = '{"gl_id":"user-100", "status":true}' - when '101' then - res.status = 200 - res.body = '{"gl_id":"user-101", "status":true}' - else - res.status = 403 - end - end - end - - describe 'lfs authentication command' do - def successful_response - { - "header" => { - "Authorization" => "Basic am9objpzb21ldG9rZW4=" - }, - "href" => "#{path}/info/lfs", - "expires_in" => 1800 - }.to_json + "\n" - end - - context 'when the command is allowed' do - context 'when key is provided' do - let(:cmd) { "#{gitlab_shell_path} key-100" } - - it 'lfs is successfully authenticated' do - output, stderr, status = Open3.capture3(env, cmd) - - expect(output).to eq(successful_response) - expect(status).to be_success - end - end - - context 'when username is provided' do - let(:cmd) { "#{gitlab_shell_path} username-someone" } - - it 'lfs is successfully authenticated' do - output, stderr, status = Open3.capture3(env, cmd) - - expect(output).to eq(successful_response) - expect(status).to be_success - end - end - end - - context 'when a user is not allowed to perform an action' do - let(:cmd) { "#{gitlab_shell_path} key-102" } - - it 'lfs is not authenticated' do - _, stderr, status = Open3.capture3(env, cmd) - - expect(stderr).not_to be_empty - expect(status).not_to be_success - end - end - - context 'when lfs authentication is forbidden for a user' do - let(:cmd) { "#{gitlab_shell_path} key-101" } - - it 'lfs is not authenticated' do - output, stderr, status = Open3.capture3(env, cmd) - - expect(stderr).to be_empty - expect(output).to be_empty - expect(status).to be_success - end - end - - context 'when an action for lfs authentication is unknown' do - let(:cmd) { "#{gitlab_shell_path} key-100" } - let(:env) { {'SSH_CONNECTION' => 'fake', 'SSH_ORIGINAL_COMMAND' => 'git-lfs-authenticate project/repo unknown' } } - - it 'the command is disallowed' do - divider = "remote: \nremote: ========================================================================\nremote: \n" - _, stderr, status = Open3.capture3(env, cmd) - - expect(stderr).to eq("#{divider}remote: Disallowed command\n#{divider}") - expect(status).not_to be_success - end - end - end -end diff --git a/spec/gitlab_shell_personal_access_token_spec.rb b/spec/gitlab_shell_personal_access_token_spec.rb deleted file mode 100644 index ba528a12e..000000000 --- a/spec/gitlab_shell_personal_access_token_spec.rb +++ /dev/null @@ -1,120 +0,0 @@ -require_relative 'spec_helper' - -require 'json' -require 'open3' -require 'date' - -describe 'bin/gitlab-shell personal_access_token' do - include_context 'gitlab shell' - - before(:context) do - write_config("gitlab_url" => "http+unix://#{CGI.escape(tmp_socket_path)}") - end - - def mock_server(server) - server.mount_proc('/api/v4/internal/personal_access_token') do |req, res| - params = JSON.parse(req.body) - - res.content_type = 'application/json' - res.status = 200 - - if params['key_id'] == '000' - res.body = { success: false, message: "Something wrong!"}.to_json - else - res.body = { - success: true, - token: 'aAY1G3YPeemECgUvxuXY', - scopes: params['scopes'], - expires_at: params['expires_at'] - }.to_json - end - end - - server.mount_proc('/api/v4/internal/discover') do |req, res| - res.status = 200 - res.content_type = 'application/json' - res.body = '{"id":100, "name": "Some User", "username": "someuser"}' - end - end - - describe 'command' do - let(:key_id) { 'key-100' } - - let(:output) do - env = { - 'SSH_CONNECTION' => 'fake', - 'SSH_ORIGINAL_COMMAND' => "personal_access_token #{args}" - } - Open3.popen2e(env, "#{gitlab_shell_path} #{key_id}")[1].read() - end - - let(:help_message) do - <<~OUTPUT - remote: - remote: ======================================================================== - remote: - remote: Usage: personal_access_token [ttl_days] - remote: - remote: ======================================================================== - remote: - OUTPUT - end - - context 'without any arguments' do - let(:args) { '' } - - it 'prints the help message' do - expect(output).to eq(help_message) - end - end - - context 'with only the name argument' do - let(:args) { 'newtoken' } - - it 'prints the help message' do - expect(output).to eq(help_message) - end - end - - context 'without a ttl argument' do - let(:args) { 'newtoken api' } - - it 'prints a token with a 30 day expiration date' do - expect(output).to eq(<<~OUTPUT) - Token: aAY1G3YPeemECgUvxuXY - Scopes: api - Expires: #{(Date.today + 30).iso8601} - OUTPUT - end - end - - context 'with a ttl argument' do - let(:args) { 'newtoken read_api,read_user 60' } - - it 'prints a token with an expiration date' do - expect(output).to eq(<<~OUTPUT) - Token: aAY1G3YPeemECgUvxuXY - Scopes: read_api,read_user - Expires: #{(Date.today + 61).iso8601} - OUTPUT - end - end - - context 'with an API error response' do - let(:args) { 'newtoken api' } - let(:key_id) { 'key-000' } - - it 'prints the error response' do - expect(output).to eq(<<~OUTPUT) - remote: - remote: ======================================================================== - remote: - remote: Something wrong! - remote: - remote: ======================================================================== - remote: - OUTPUT - end - end - end -end diff --git a/spec/gitlab_shell_two_factor_verify_spec.rb b/spec/gitlab_shell_two_factor_verify_spec.rb deleted file mode 100644 index b39a5e0a7..000000000 --- a/spec/gitlab_shell_two_factor_verify_spec.rb +++ /dev/null @@ -1,125 +0,0 @@ -require_relative 'spec_helper' - -require 'open3' -require 'json' - -describe 'bin/gitlab-shell 2fa_verify' do - include_context 'gitlab shell' - - let(:env) do - { 'SSH_CONNECTION' => 'fake', - 'SSH_ORIGINAL_COMMAND' => '2fa_verify' } - end - - let(:correct_otp) { '123456' } - - before(:context) do - write_config('gitlab_url' => "http+unix://#{CGI.escape(tmp_socket_path)}") - end - - def mock_server(server) - server.mount_proc('/api/v4/internal/two_factor_manual_otp_check') do |req, res| - res.content_type = 'application/json' - res.status = 200 - - params = JSON.parse(req.body) - - res.body = if params['otp_attempt'] == correct_otp - { success: true }.to_json - else - { success: false, message: 'boom!' }.to_json - end - end - - server.mount_proc('/api/v4/internal/two_factor_push_otp_check') do |req, res| - res.content_type = 'application/json' - res.status = 200 - - params = JSON.parse(req.body) - id = params['key_id'] || params['user_id'].to_s - - if id == '100' - res.body = { success: false, message: 'boom!' }.to_json - else - res.body = { success: true }.to_json - end - end - - server.mount_proc('/api/v4/internal/discover') do |req, res| - res.status = 200 - res.content_type = 'application/json' - - if req.query['username'] == 'someone' - res.body = { id: 100, name: 'Some User', username: 'someuser' }.to_json - else - res.body = { id: 101, name: 'Another User', username: 'another' }.to_json - end - end - end - - describe 'entering OTP manually' do - let(:cmd) { "#{gitlab_shell_path} key-100" } - - context 'when key is provided' do - it 'asks a user for a correct OTP' do - verify_successful_otp_verification!(cmd) - end - end - - context 'when username is provided' do - let(:cmd) { "#{gitlab_shell_path} username-someone" } - - it 'asks a user for a correct OTP' do - verify_successful_otp_verification!(cmd) - end - end - - it 'shows an error when an invalid otp is provided' do - Open3.popen2(env, cmd) do |stdin, stdout| - asks_for_otp(stdout) - stdin.puts('000000') - - expect(stdout.flush.read).to eq("\nOTP validation failed: boom!\n") - end - end - end - - describe 'authorizing via push' do - context 'when key is provided' do - let(:cmd) { "#{gitlab_shell_path} key-101" } - - it 'asks a user for a correct OTP' do - verify_successful_push_verification!(cmd) - end - end - - context 'when username is provided' do - let(:cmd) { "#{gitlab_shell_path} username-another" } - - it 'asks a user for a correct OTP' do - verify_successful_push_verification!(cmd) - end - end - end - - def verify_successful_otp_verification!(cmd) - Open3.popen2(env, cmd) do |stdin, stdout| - asks_for_otp(stdout) - stdin.puts(correct_otp) - - expect(stdout.flush.read).to eq("\nOTP validation successful. Git operations are now allowed.\n") - end - end - - def verify_successful_push_verification!(cmd) - Open3.popen2(env, cmd) do |stdin, stdout| - asks_for_otp(stdout) - - expect(stdout.flush.read).to eq("\nOTP has been validated by Push Authentication. Git operations are now allowed.\n") - end - end - - def asks_for_otp(stdout) - expect(stdout.gets(5)).to eq('OTP: ') - end -end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb deleted file mode 100644 index 093988305..000000000 --- a/spec/spec_helper.rb +++ /dev/null @@ -1,8 +0,0 @@ -ROOT_PATH = File.expand_path('..', __dir__) - -Dir[File.expand_path('support/**/*.rb', __dir__)].each { |f| require f } - -RSpec.configure do |config| - config.run_all_when_everything_filtered = true - config.filter_run :focus -end diff --git a/spec/support/gitlab_shell_setup.rb b/spec/support/gitlab_shell_setup.rb deleted file mode 100644 index 6982aaa5f..000000000 --- a/spec/support/gitlab_shell_setup.rb +++ /dev/null @@ -1,62 +0,0 @@ -require 'yaml' -require 'tempfile' - -RSpec.shared_context 'gitlab shell', shared_context: :metadata do - def original_root_path - ROOT_PATH - end - - def config_path - File.join(tmp_root_path, 'config.yml') - end - - def write_config(config) - config['log_file'] ||= Tempfile.new.path - - File.open(config_path, 'w') do |f| - f.write(config.to_yaml) - end - end - - def tmp_root_path - @tmp_root_path ||= File.realpath(Dir.mktmpdir) - end - - def mock_server(server) - raise NotImplementedError.new( - 'mock_server method must be implemented in order to include gitlab shell context' - ) - end - - # This has to be a relative path shorter than 100 bytes due to - # limitations in how Unix sockets work. - def tmp_socket_path - 'tmp/gitlab-shell-socket' - end - - let(:gitlab_shell_path) { File.join(tmp_root_path, 'bin', 'gitlab-shell') } - - before(:all) do - FileUtils.mkdir_p(File.dirname(tmp_socket_path)) - FileUtils.touch(File.join(tmp_root_path, '.gitlab_shell_secret')) - - @server = HTTPUNIXServer.new(BindAddress: tmp_socket_path) - - mock_server(@server) - - @webrick_thread = Thread.new { @server.start } - - sleep(0.1) while @webrick_thread.alive? && @server.status != :Running - raise "Couldn't start stub GitlabNet server" unless @server.status == :Running - system(original_root_path, 'bin/compile') - - FileUtils.rm_rf(File.join(tmp_root_path, 'bin')) - FileUtils.cp_r('bin', tmp_root_path) - end - - after(:all) do - @server.shutdown if @server - @webrick_thread.join if @webrick_thread - FileUtils.rm_rf(tmp_root_path) - end -end diff --git a/spec/support/http_unix_server.rb b/spec/support/http_unix_server.rb deleted file mode 100644 index 113df57bc..000000000 --- a/spec/support/http_unix_server.rb +++ /dev/null @@ -1,35 +0,0 @@ -require 'webrick' - -# like WEBrick::HTTPServer, but listens on UNIX socket -class HTTPUNIXServer < WEBrick::HTTPServer - def initialize(config = {}) - null_log = WEBrick::Log.new(IO::NULL, 7) - - super(config.merge(Logger: null_log, AccessLog: null_log)) - end - - def listen(address, port) - socket = Socket.unix_server_socket(address) - socket.autoclose = false - server = UNIXServer.for_fd(socket.fileno) - socket.close - @listeners << server - end - - # Workaround: - # https://bugs.ruby-lang.org/issues/10956 - # Affecting Ruby 2.2 - # Fix for 2.2 is at https://github.com/ruby/ruby/commit/ab0a64e1 - # However, this doesn't work with 2.1. The following should work for both: - def start(&block) - @shutdown_pipe = IO.pipe - shutdown_pipe = @shutdown_pipe - super(&block) - end - - def cleanup_shutdown_pipe(shutdown_pipe) - @shutdown_pipe = nil - return if !shutdown_pipe - super(shutdown_pipe) - end -end