toil(github_hooks): Log which github event is received

github_hooks/app/controllers/projects_controller.rb

@@ -78,6 +78,21 @@ def repo_host_post_commit_hook
           next
         end
 
+        signature = repo_host_request.headers["X-Hub-Signature-256"]
+
+        # Rubocop insisted on making this a one big if, istead of 2 nested if statements
+        # We are validating a signature from each webhook coming from github app above
+        # in this function, (currently line 29), and we are validating signatures for every webhook
+        # coming from a repository within the hook handler.
+
+        # Only exception to the signature verification checks are repository level webhooks that do not generate
+        # a workflow (for example new member added to repo, or repository metadata changed). That will be covered
+        # in the following if block.
+        if (webhook_filter.repository_webhook? || (webhook_filter.member_webhook? && !webhook_filter.github_app_webhook?)) && !Semaphore::RepoHost::Hooks::Handler.webhook_signature_valid?(logger, project.organization.id, project.repository.id, repo_host_request.raw_post, signature)
+          logger.error("Webhook validation for repository changed and repository member events failed")
+          next
+        end
+
         if webhook_filter.member_webhook?
           Watchman.increment("repo_host_post_commit_hooks.controller.member_webhook")
           logger.info("Member Webhook")
@@ -103,9 +118,7 @@ def repo_host_post_commit_hook
         end
 
         workflow = Semaphore::RepoHost::Hooks::Recorder.record_hook(hook_params, project)
-
         logger.add(:post_commit_request_id => workflow.id)
-
         logger.info("Saved Request")
 
         if webhook_filter.unavailable_payload?
@@ -122,12 +135,6 @@ def repo_host_post_commit_hook
 
         Watchman.increment("IncommingHooks.processed", { external: true })
 
-        signature = repo_host_request.headers["X-Hub-Signature-256"]
-
-        unless signature
-          Watchman.increment("hooks.processing.missing_signature", tags: [project.id, project.organization.id])
-        end
-
         sidekiq_job_id = Semaphore::RepoHost::Hooks::Handler::Worker.perform_async(workflow.id, repo_host_request.raw_post, signature, 0)
 
         logger.info("Enqueud in Sidekiq", :sidekiq_job_id => sidekiq_job_id)

github_hooks/spec/controllers/projects_controller_spec.rb

@@ -306,8 +306,9 @@ def post_app_payload(payload)
         end
 
         it "publish event" do
+          repository = double("Repository", :id => "repo-123")
           project = double(Project, :id => "96b0a57c-d9ae-453f-b56f-3b154eb10cda", :organization => @organization,
-                                    :repo_owner_and_name => "foo/bar")
+                                    :repo_owner_and_name => "foo/bar", :repository => repository)
           expect(Project).to receive(:find_by).and_return(project)
 
           expect(Semaphore::Events::ProjectCollaboratorsChanged).to receive(:emit).and_call_original
@@ -373,10 +374,19 @@ def post_app_payload(payload)
         end
 
         it "publish event" do
+          repository = double(Repository, :id => "repo-123")
           project = double(Project, :id => "96b0a57c-d9ae-453f-b56f-3b154eb10cda", :organization => @organization,
-                                    :repo_owner => "foo")
+                                    :repo_owner => "foo", :repository => repository)
           expect(Project).to receive(:find_by).and_return(project)
 
+          expect(Semaphore::RepoHost::Hooks::Handler).to receive(:webhook_signature_valid?).with(
+            anything,
+            project.organization.id,
+            project.repository.id,
+            anything,
+            anything
+          ).and_return(true)
+
           expect(Semaphore::Events::RemoteRepositoryChanged).to receive(:emit).and_call_original
 
           post_payload(payload)
@@ -406,14 +416,35 @@ def post_app_payload(payload)
         end
 
         it "publish event" do
+          repository = double(Repository, :id => "repo-123")
           project = double(Project, :id => "96b0a57c-d9ae-453f-b56f-3b154eb10cda", :organization => @organization,
-                                    :repo_owner_and_name => "foo/bar")
+                                    :repo_owner_and_name => "foo/bar", :repository => repository)
           expect(Project).to receive(:find_by).and_return(project)
 
+          expect(Semaphore::RepoHost::Hooks::Handler).to receive(:webhook_signature_valid?).with(
+            anything,
+            project.organization.id,
+            project.repository.id,
+            anything,
+            anything
+          ).and_return(true)
+
           expect(Semaphore::Events::RemoteRepositoryChanged).to receive(:emit).and_call_original
 
           post_payload(payload)
         end
+
+        it "does not publish event when signature is invalid" do
+          repository = double("Repository", :id => "repo-123")
+          project = double(Project, :id => "96b0a57c-d9ae-453f-b56f-3b154eb10cda", :organization => @organization,
+                                    :repo_owner_and_name => "foo/bar", :repository => repository)
+          expect(Project).to receive(:find_by).and_return(project)
+
+          expect(Semaphore::RepoHost::Hooks::Handler).to receive(:webhook_signature_valid?).and_return(false)
+          expect(Semaphore::Events::RemoteRepositoryChanged).not_to receive(:emit)
+
+          post_payload(payload)
+        end
       end
 
       context "when github_app installation event occurs" do