From af83a9af01d1b18a771aef74967a21733ea253d2 Mon Sep 17 00:00:00 2001 From: Max Jacobson Date: Mon, 19 Dec 2016 12:35:54 -0500 Subject: [PATCH] Support authorizing asana requests via tokens Asana is deprecating their API Key access, as reported by a helpful user. https://asana.com/developers/documentation/getting-started/auth#api-key We confirmed with their support team that they're planning to drop support for API Key authorization in "early 2017". This PR introduces support for authorizing with personal access tokens, and attempts to nudge users in that direction gently. I think we'll need to make a plan about how to migrate existing repos over. --- lib/cc/services/asana.rb | 17 ++++- spec/cc/service/asana_spec.rb | 136 ++++++++++++++++++++++------------ 2 files changed, 104 insertions(+), 49 deletions(-) diff --git a/lib/cc/services/asana.rb b/lib/cc/services/asana.rb index 918bb57..2895d0e 100644 --- a/lib/cc/services/asana.rb +++ b/lib/cc/services/asana.rb @@ -1,6 +1,7 @@ class CC::Service::Asana < CC::Service class Config < CC::Service::Config - attribute :api_key, Axiom::Types::String, label: "API key" + attribute :personal_access_token, Axiom::Types::String, label: "Personal Access Token" + attribute :api_key, Axiom::Types::String, label: "API key (Deprecated)" attribute :workspace_id, Axiom::Types::String, label: "Workspace ID" @@ -10,8 +11,14 @@ class Config < CC::Service::Config attribute :assignee, Axiom::Types::String, label: "Assignee", description: "Assignee email address (optional)" - validates :api_key, presence: true + validate :authorization_provided validates :workspace_id, presence: true + + def authorization_provided + if api_key.blank? && personal_access_token.blank? + errors.add(:personal_access_token, "can't be blank") + end + end end ENDPOINT = "https://app.asana.com/api/1.0/tasks".freeze @@ -82,6 +89,10 @@ def generate_params(name, notes = nil) end def authenticate_http - http.basic_auth(config.api_key, "") + if config.personal_access_token.present? + http.headers["Authorization"] = "Bearer #{config.personal_access_token}" + else + http.basic_auth(config.api_key, "") + end end end diff --git a/spec/cc/service/asana_spec.rb b/spec/cc/service/asana_spec.rb index 9bee7d6..8471949 100644 --- a/spec/cc/service/asana_spec.rb +++ b/spec/cc/service/asana_spec.rb @@ -1,63 +1,96 @@ describe CC::Service::Asana, type: :service do - it "quality" do - assert_asana_receives( - event(:quality, to: "D", from: "C"), - "Refactor User from a D on Code Climate - https://codeclimate.com/repos/1/feed", - ) - end + it "requires an authorization key or token, and nudges users toward personal_access_token" do + config = CC::Service::Asana::Config.new(workspace_id: '1234') + expect(config).to_not be_valid + expect(config.errors[:personal_access_token]).to eq ["can't be blank"] - it "vulnerability" do - assert_asana_receives( - event(:vulnerability, vulnerabilities: [{ - "warning_type" => "critical", - "location" => "app/user.rb line 120", - }]), - "New critical issue found in app/user.rb line 120 - https://codeclimate.com/repos/1/feed", - ) - end + config.api_key = "foo" + expect(config).to be_valid - it "issue" do - payload = { - issue: { - "check_name" => "Style/LongLine", - "description" => "Line is too long [1000/80]", - }, - constant_name: "foo.rb", - details_url: "http://example.com/repos/id/foo.rb#issue_123", - } - - assert_asana_receives( - event(:issue, payload), - "Fix \"Style/LongLine\" issue in foo.rb", - "Line is too long [1000/80]\n\nhttp://example.com/repos/id/foo.rb#issue_123", - ) + config.api_key = nil + config.personal_access_token = "bar" + expect(config).to be_valid end - it "successful post" do - http_stubs.post "/api/1.0/tasks" do |_env| - [200, {}, '{"data":{"id":"2"}}'] + shared_examples "Asana integration" do |authorization| + it "creates a ticket for quality changes" do + assert_asana_receives( + event(:quality, to: "D", from: "C"), + "Refactor User from a D on Code Climate - https://codeclimate.com/repos/1/feed", + authorization, + ) end - response = receive_event + it "creates a ticket for vulnerability changes" do + assert_asana_receives( + event(:vulnerability, vulnerabilities: [{ + "warning_type" => "critical", + "location" => "app/user.rb line 120", + }]), + "New critical issue found in app/user.rb line 120 - https://codeclimate.com/repos/1/feed", + authorization, + ) + end - expect(response[:id]).to eq("2") - expect(response[:url]).to eq("https://app.asana.com/0/1/2") - end + it "creates a ticket for a new issue" do + payload = { + issue: { + "check_name" => "Style/LongLine", + "description" => "Line is too long [1000/80]", + }, + constant_name: "foo.rb", + details_url: "http://example.com/repos/id/foo.rb#issue_123", + } - it "receive test" do - http_stubs.post "/api/1.0/tasks" do |_env| - [200, {}, '{"data":{"id":"4"}}'] + assert_asana_receives( + event(:issue, payload), + "Fix \"Style/LongLine\" issue in foo.rb", + authorization, + "Line is too long [1000/80]\n\nhttp://example.com/repos/id/foo.rb#issue_123", + ) end - response = receive_event(name: "test") + it "can make a successful POST request" do + http_stubs.post "/api/1.0/tasks" do |_env| + [200, {}, '{"data":{"id":"2"}}'] + end + + response = receive_event(authorization) - expect(response[:message]).to eq("Ticket 4 created.") + expect(response[:id]).to eq("2") + expect(response[:url]).to eq("https://app.asana.com/0/1/2") + end + + it "can make a test request" do + http_stubs.post "/api/1.0/tasks" do |_env| + [200, {}, '{"data":{"id":"4"}}'] + end + + response = receive_event(authorization, name: "test") + + expect(response[:message]).to eq("Ticket 4 created.") + end end + it_behaves_like "Asana integration", :api_key + it_behaves_like "Asana integration", :personal_access_token + it_behaves_like "Asana integration", :both + private - def assert_asana_receives(event_data, name, notes = "") + def assert_asana_receives(event_data, name, authorization, notes = "") http_stubs.post "/api/1.0/tasks" do |env| + case authorization + when :api_key + expect(env[:request_headers]["Authorization"]).to include("Basic") + when :personal_access_token + expect(env[:request_headers]["Authorization"]).to eq("Bearer def456") + when :both + # prefer the personal access token + expect(env[:request_headers]["Authorization"]).to eq("Bearer def456") + else + raise ArgumentError + end body = JSON.parse(env[:body]) data = body["data"] @@ -70,13 +103,24 @@ def assert_asana_receives(event_data, name, notes = "") [200, {}, '{"data":{"id":4}}'] end - receive_event(event_data) + receive_event(authorization, event_data) end - def receive_event(event_data = nil) + def receive_event(authorization, event_data = nil) + service_configuration = { workspace_id: "1", project_id: "2", assignee: "jim@asana.com" } + case authorization + when :api_key + service_configuration[:api_key] = "abc123" + when :personal_access_token + service_configuration[:personal_access_token] = "def456" + when :both + service_configuration[:api_key] = "abc123" + service_configuration[:personal_access_token] = "def456" + else raise ArgumentError + end service_receive( CC::Service::Asana, - { api_key: "abc123", workspace_id: "1", project_id: "2", assignee: "jim@asana.com" }, + service_configuration, event_data || event(:quality, to: "D", from: "C"), ) end