Merge pull request #56 from smashwilson/36-logger

Logger

3 files changed, 131 insertions, 4 deletions

diff --git a/spec/gitlab_keys_spec.rb b/spec/gitlab_keys_spec.rb
index f04d506..09f5872 100644
--- a/spec/gitlab_keys_spec.rb
+++ b/spec/gitlab_keys_spec.rb
@@ -2,6 +2,10 @@ require_relative 'spec_helper'
 require_relative '../lib/gitlab_keys'
 
 describe GitlabKeys do
+  before do
+    $logger = double('logger').as_null_object
+  end
+
   describe :initialize do
     let(:gitlab_keys) { build_gitlab_keys('add-key', 'key-741', 'ssh-rsa AAAAB3NzaDAxx2E') }
 
@@ -18,6 +22,11 @@ describe GitlabKeys do
       gitlab_keys.should_receive(:system).with(valid_cmd)
       gitlab_keys.send :add_key
     end
+
+    it "should log an add-key event" do
+      $logger.should_receive(:info).with('Adding key key-741 => "ssh-rsa AAAAB3NzaDAxx2E"')
+      gitlab_keys.send :add_key
+    end
   end
 
   describe :rm_key do
@@ -28,6 +37,11 @@ describe GitlabKeys do
       gitlab_keys.should_receive(:system).with(valid_cmd)
       gitlab_keys.send :rm_key
     end
+
+    it "should log an rm-key event" do
+      $logger.should_receive(:info).with('Removing key key-741')
+      gitlab_keys.send :rm_key
+    end
   end
 
   describe :exec do
@@ -48,6 +62,13 @@ describe GitlabKeys do
       gitlab_keys.should_receive(:puts).with('not allowed')
       gitlab_keys.exec
     end
+
+    it 'should log a warning on unknown commands' do
+      gitlab_keys = build_gitlab_keys('nooope')
+      gitlab_keys.stub(puts: nil)
+      $logger.should_receive(:warn).with('Attempt to execute invalid gitlab-keys command "nooope".')
+      gitlab_keys.exec
+    end
   end
 
   def build_gitlab_keys(*args)
diff --git a/spec/gitlab_projects_spec.rb b/spec/gitlab_projects_spec.rb
index d0d6764..5bcc5c8 100644
--- a/spec/gitlab_projects_spec.rb
+++ b/spec/gitlab_projects_spec.rb
@@ -4,6 +4,7 @@ require_relative '../lib/gitlab_projects'
 describe GitlabProjects do
   before do
     FileUtils.mkdir_p(tmp_repos_path)
+    $logger = double('logger').as_null_object
   end
 
   after do
@@ -37,10 +38,16 @@ describe GitlabProjects do
       gl_projects.should_receive(:system).with(valid_cmd)
       gl_projects.exec
     end
+
+    it "should log an add-project event" do
+      $logger.should_receive(:info).with("Adding project #{repo_name} at <#{tmp_repo_path}>.")
+      gl_projects.exec
+    end
   end
 
   describe :mv_project do
     let(:gl_projects) { build_gitlab_projects('mv-project', repo_name, 'repo.git') }
+    let(:new_repo_path) { File.join(tmp_repos_path, 'repo.git') }
 
     before do
       FileUtils.mkdir_p(tmp_repo_path)
@@ -50,7 +57,33 @@ describe GitlabProjects do
       File.exists?(tmp_repo_path).should be_true
       gl_projects.exec
       File.exists?(tmp_repo_path).should be_false
-      File.exists?(File.join(tmp_repos_path, 'repo.git')).should be_true
+      File.exists?(new_repo_path).should be_true
+    end
+
+    it "should fail if no destination path is provided" do
+      incomplete = build_gitlab_projects('mv-project', repo_name)
+      $logger.should_receive(:error).with("mv-project failed: no destination path provided.")
+      incomplete.exec.should be_false
+    end
+
+    it "should fail if the source path doesn't exist" do
+      bad_source = build_gitlab_projects('mv-project', 'bad-src.git', 'dest.git')
+      $logger.should_receive(:error).with("mv-project failed: source path <#{tmp_repos_path}/bad-src.git> does not exist.")
+      bad_source.exec.should be_false
+    end
+
+    it "should fail if the destination path already exists" do
+      FileUtils.mkdir_p(File.join(tmp_repos_path, 'already-exists.git'))
+      bad_dest = build_gitlab_projects('mv-project', repo_name, 'already-exists.git')
+      message = "mv-project failed: destination path <#{tmp_repos_path}/already-exists.git> already exists."
+      $logger.should_receive(:error).with(message)
+      bad_dest.exec.should be_false
+    end
+
+    it "should log an mv-project event" do
+      message = "Moving project #{repo_name} from <#{tmp_repo_path}> to <#{new_repo_path}>."
+      $logger.should_receive(:info).with(message)
+      gl_projects.exec
     end
   end
 
@@ -66,6 +99,11 @@ describe GitlabProjects do
       gl_projects.exec
       File.exists?(tmp_repo_path).should be_false
     end
+
+    it "should log an rm-project event" do
+      $logger.should_receive(:info).with("Removing project #{repo_name} from <#{tmp_repo_path}>.")
+      gl_projects.exec
+    end
   end
 
   describe :import_project do
@@ -75,6 +113,12 @@ describe GitlabProjects do
       gl_projects.exec
       File.exists?(File.join(tmp_repo_path, 'HEAD')).should be_true
     end
+
+    it "should log an import-project event" do
+      message = "Importing project #{repo_name} from <https://github.com/randx/six.git> to <#{tmp_repo_path}>."
+      $logger.should_receive(:info).with(message)
+      gl_projects.exec
+    end
   end
 
   describe :fork_project do
@@ -87,7 +131,15 @@ describe GitlabProjects do
       gl_projects_import.exec
     end
 
+    it "should not fork without a destination namespace" do
+      missing_arg = build_gitlab_projects('fork-project', source_repo_name)
+      $logger.should_receive(:error).with("fork-project failed: no destination namespace provided.")
+      missing_arg.exec.should be_false
+    end
+
     it "should not fork into a namespace that doesn't exist" do
+      message = "fork-project failed: destination namespace <#{tmp_repos_path}/forked-to-namespace> does not exist."
+      $logger.should_receive(:error).with(message)
       gl_projects_fork.exec.should be_false
     end
 
@@ -101,9 +153,24 @@ describe GitlabProjects do
     end
 
     it "should not fork if a project of the same name already exists" do
-      #trying to fork again should fail as the repo already exists
+      # create a fake project at the intended destination
+      FileUtils.mkdir_p(File.join(tmp_repos_path, 'forked-to-namespace', repo_name))
+
+      # trying to fork again should fail as the repo already exists
+      message = "fork-project failed: destination repository <#{tmp_repos_path}/forked-to-namespace/#{repo_name}> "
+      message << "already exists."
+      $logger.should_receive(:error).with(message)
       gl_projects_fork.exec.should be_false
     end
+
+    it "should log a fork-project event" do
+      message = "Forking project from <#{File.join(tmp_repos_path, source_repo_name)}> to <#{dest_repo}>."
+      $logger.should_receive(:info).with(message)
+
+      # create destination namespace
+      FileUtils.mkdir_p(File.join(tmp_repos_path, 'forked-to-namespace'))
+      gl_projects_fork.exec.should be_true
+    end
   end
 
   describe :exec do
@@ -112,6 +179,12 @@ describe GitlabProjects do
       gitlab_projects.should_receive(:puts).with('not allowed')
       gitlab_projects.exec
     end
+
+    it 'should log a warning for unknown commands' do
+      gitlab_projects = build_gitlab_projects('hurf-durf', repo_name)
+      $logger.should_receive(:warn).with('Attempt to execute invalid gitlab-projects command "hurf-durf".')
+      gitlab_projects.exec
+    end
   end
 
   def build_gitlab_projects(*args)
diff --git a/spec/gitlab_shell_spec.rb b/spec/gitlab_shell_spec.rb
index da91c36..44dca6d 100644
--- a/spec/gitlab_shell_spec.rb
+++ b/spec/gitlab_shell_spec.rb
@@ -11,13 +11,15 @@ describe GitlabShell do
   end
   let(:api) do
     double(GitlabNet).tap do |api|
-      api.stub(discover: 'John Doe')
+      api.stub(discover: { 'name' => 'John Doe' })
       api.stub(allowed?: true)
     end
   end
   let(:key_id) { "key-#{rand(100) + 100}" }
   let(:repository_path) { "/home/git#{rand(100)}/repos" }
-  before { GitlabConfig.any_instance.stub(:repos_path).and_return(repository_path) }
+  before do
+    GitlabConfig.any_instance.stub(repos_path: repository_path, audit_usernames: false)
+  end
 
   describe :initialize do
     before { ssh_cmd 'git-receive-pack' }
@@ -64,6 +66,18 @@ describe GitlabShell do
       it "should set the GL_ID environment variable" do
         ENV.should_receive("[]=").with("GL_ID", key_id)
       end
+
+      it "should log the command execution" do
+        message = "gitlab-shell: executing git command "
+        message << "<git-upload-pack #{File.join(repository_path, 'gitlab-ci.git')}> "
+        message << "for user with key #{key_id}."
+        $logger.should_receive(:info).with(message)
+      end
+
+      it "should use usernames if configured to do so" do
+        GitlabConfig.any_instance.stub(audit_usernames: true)
+        $logger.should_receive(:info) { |msg| msg.should =~ /for John Doe/ }
+      end
     end
 
     context 'git-receive-pack' do
@@ -77,6 +91,13 @@ describe GitlabShell do
       it "should execute the command" do
         subject.should_receive(:exec_cmd).with("git-receive-pack #{File.join(repository_path, 'gitlab-ci.git')}")
       end
+
+      it "should log the command execution" do
+        message = "gitlab-shell: executing git command "
+        message << "<git-receive-pack #{File.join(repository_path, 'gitlab-ci.git')}> "
+        message << "for user with key #{key_id}."
+        $logger.should_receive(:info).with(message)
+      end
     end
 
     context 'arbitrary command' do
@@ -90,6 +111,11 @@ describe GitlabShell do
       it "should not execute the command" do
         subject.should_not_receive(:exec_cmd)
       end
+
+      it "should log the attempt" do
+        message = "gitlab-shell: Attempt to execute disallowed command <arbitrary command> by user with key #{key_id}."
+        $logger.should_receive(:warn).with(message)
+      end
     end
 
     context 'no command' do
@@ -110,6 +136,13 @@ describe GitlabShell do
       api.should_receive(:allowed?).
         with('git-upload-pack', 'gitlab-ci.git', key_id, '_any')
     end
+
+    it "should disallow access and log the attempt if allowed? returns false" do
+      api.stub(allowed?: false)
+      message = "gitlab-shell: Access denied for git command <git-upload-pack gitlab-ci.git> "
+      message << "by user with key #{key_id}."
+      $logger.should_receive(:warn).with(message)
+    end
   end
 
   def ssh_cmd(cmd)