Merge pull request #12139 from jasonwbarnett/feature/add-support-for-multiple-user-assigned-managed-identities

Add secrets support for multiple User Assigned Managed Identities in Azure

4 files changed, 171 insertions, 29 deletions

diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index 249a90e34b..ffdbdcbaba 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -308,6 +308,16 @@ class Chef
           super("No secret service provided. Supported services are: :#{fetcher_service_names.join(" :")}")
         end
       end
+
+      class Azure
+        class IdentityNotFound < RuntimeError
+          def initialize
+            super("The managed identity could not be found. This could mean one of the following things:\n\n" \
+                  "  1. The VM has no system or user assigned identities.\n" \
+                  "  2. The managed identity object_id or client_id that was specified is not assigned to the VM.\n")
+          end
+        end
+      end
     end
 
     # Exception class for collecting multiple failures. Used when running
diff --git a/lib/chef/secret_fetcher.rb b/lib/chef/secret_fetcher.rb
index af3e1d5cbb..96bbfc92a4 100644
--- a/lib/chef/secret_fetcher.rb
+++ b/lib/chef/secret_fetcher.rb
@@ -58,4 +58,3 @@ class Chef
     end
   end
 end
-
diff --git a/lib/chef/secret_fetcher/azure_key_vault.rb b/lib/chef/secret_fetcher/azure_key_vault.rb
index a617f3bb93..fdcf479eb3 100644
--- a/lib/chef/secret_fetcher/azure_key_vault.rb
+++ b/lib/chef/secret_fetcher/azure_key_vault.rb
@@ -1,4 +1,6 @@
 require_relative "base"
+require_relative "../exceptions"
+require "uri" unless defined?(URI)
 
 class Chef
   class SecretFetcher
@@ -14,13 +16,19 @@ class Chef
     #
     # @example
     #
-    # fetcher = SecretFetcher.for_service(:azure_key_vault, { vault: "my_vault" }, run_context )
+    # fetcher = SecretFetcher.for_service(:azure_key_vault, { vault: "my_vault" }, run_context)
     # fetcher.fetch("secretkey1", "v1")
     #
     # @example
     #
-    # fetcher = SecretFetcher.for_service(:azure_key_vault, {}, run_context )
+    # fetcher = SecretFetcher.for_service(:azure_key_vault, {}, run_context)
     # fetcher.fetch("my_vault/secretkey1", "v1")
+    #
+    # @example
+    #
+    # fetcher = SecretFetcher.for_service(:azure_key_vault, { client_id: "540d76b6-7f76-456c-b68b-ccae4dc9d99d" }, run_context)
+    # fetcher.fetch("my_vault/secretkey1", "v1")
+    #
     class AzureKeyVault < Base
 
       def do_fetch(name, version)
@@ -48,6 +56,12 @@ class Chef
         end
       end
 
+      def validate!
+        raise Chef::Exceptions::Secret::ConfigurationInvalid, "You may only specify one (these are mutually exclusive): :object_id, :client_id, or :mi_res_id" if [object_id, client_id, mi_res_id].select { |x| !x.nil? }.length > 1
+      end
+
+      private
+
       # Determine the vault name and secret name from the provided name.
       # If it is not in the provided name in the form "vault_name/secret_name"
       # it will determine the vault name from `config[:vault]`.
@@ -63,16 +77,56 @@ class Chef
         end
       end
 
+      def api_version
+        "2018-02-01"
+      end
+
+      def resource
+        "https://vault.azure.net"
+      end
+
+      def object_id
+        config[:object_id]
+      end
+
+      def client_id
+        config[:client_id]
+      end
+
+      def mi_res_id
+        config[:mi_res_id]
+      end
+
+      def token_query
+        @token_query ||= begin
+          p = {}
+          p["api-version"] = api_version
+          p["resource"] = resource
+          p["object_id"] = object_id if object_id
+          p["client_id"] = client_id if client_id
+          p["mi_res_id"] = mi_res_id if mi_res_id
+          URI.encode_www_form(p)
+        end
+      end
+
       def fetch_token
-        token_uri = URI.parse("http://169.254.169.254/metadata/identity/oauth2/token?api-version=2018-02-01&resource=https%3A%2F%2Fvault.azure.net")
+        token_uri = URI.parse("http://169.254.169.254/metadata/identity/oauth2/token")
+        token_uri.query = token_query
         http = Net::HTTP.new(token_uri.host, token_uri.port)
         response = http.get(token_uri, { "Metadata" => "true" })
-        body = JSON.parse(response.body)
-        body["access_token"]
+
+        case response
+        when Net::HTTPSuccess
+          body = JSON.parse(response.body)
+          body["access_token"]
+        when Net::HTTPBadRequest
+          body = JSON.parse(response.body)
+          raise Chef::Exceptions::Secret::Azure::IdentityNotFound if body["error_description"] =~ /identity not found/i
+        else
+          body = JSON.parse(response.body)
+          body["access_token"]
+        end
       end
     end
   end
 end
-
-
-
diff --git a/spec/unit/secret_fetcher/azure_key_vault_spec.rb b/spec/unit/secret_fetcher/azure_key_vault_spec.rb
index d0fc2727bc..748bad791f 100644
--- a/spec/unit/secret_fetcher/azure_key_vault_spec.rb
+++ b/spec/unit/secret_fetcher/azure_key_vault_spec.rb
@@ -20,51 +20,130 @@
 require_relative "../../spec_helper"
 require "chef/secret_fetcher"
 require "chef/secret_fetcher/azure_key_vault"
+require "net/http/responses"
 
 describe Chef::SecretFetcher::AzureKeyVault do
-  let(:config) { { vault: "my_vault" } }
+  let(:config) { { vault: "my-vault" } }
   let(:fetcher) { Chef::SecretFetcher::AzureKeyVault.new(config, nil) }
+  let(:secrets_response_body) { '{ "value" : "my secret value" }' }
+  let(:secrets_response_mock) do
+    rm = Net::HTTPSuccess.new("1.0", "400", "OK")
+    allow(rm).to receive(:body).and_return(secrets_response_body)
+    rm
+  end
+  let(:token_response_body) { %Q({"access_token":"#{access_token}","client_id":"#{client_id}","expires_in":"86294","expires_on":"1627761860","ext_expires_in":"86399","not_before":"1627675160","resource":"https://vault.azure.net","token_type":"Bearer"}) }
+  let(:access_token) { "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsIng1dCI6Im5PbzNaRHJPRFhFSzFqS1doWHNsSFJfS1hFZyIsImtpZCI6Im5PbzNaRHJPRFhFSzFqS1doWHNsSFJfS1hFZyJ9.eyJhdWQiOiJodHRwczovL3ZhdWx0LmF6dXJlLm5ldCIsImlzcyI6Imh0dHBzOi8vc3RzLndpbmRvd3MubmV0L2E5ZTY2ZDhkLTA1ZTAtNGMwMC1iOWRkLWM0Yjc3M2U5MWNhNi8iLCJpYXQiOjE2Mjc2NzUxNjAsIm5iZiI6MTYyNzY3NTE2MCwiZXhwIjoxNjI3NzYxODYwLCJhaW8iOiJFMlpnWUhCWGplaTdWS214eEh6bjdoSWpNZFlMQUE9PSIsImFwcGlkIjoiNjU2Mjc1MjEtMzYzYi00ZDk2LTkyMTctMjcIsIm9pZCI6IjNiZjI1NjVhLWY4NWQtNDBiNy1hZWJkLTNlZDA1ZDA0N2FmNiIsInJoIjoiMC5BUk1BalczbXFlQUZBRXk1M2NTM2Mta2NwaUYxWW1VN05wWk5raGNuRGpuZEwxb1RBQUEuIiwic3ViIjoiM2JmMjU2NWEtZjg1ZC00MGI3LWFlYmQtM2VkMDVkMDQ3YWY2IiwidGlkIjoiYTllNjZkOGQtMDVlMC00YzAwLWI5ZGQtYzRiNzczZTkxY2E2IiwidXRpIjoibXlzeHpSRTV3ay1ibTFlYkNqc09BQSIsInZlciI6IjEuMCIsInhtc19taXJpZCI6Ii9zdWJzY3JpcHRpb25zLzYzNDJkZDZkLTc1NTQtNDJjOS04NTM2LTdkZmU3MmY1MWZhZC9yZXNvdXJjZWdyb3Vwcy9pbWFnZS1waXBlbGluZS1ydW5uZXItcWEtZWFzdHVzMi1yZy9wcm92aWRlcnMvTWljcm9zb2Z0Lk1hbmFnZWRJZGVudGl0eS91c2VyQXNzaWduZWRJZGVudGl0aWVzL2ltYWdlLXBpcGVsaW5lLXJ1bm5lci1xYS1lYXN0dXMyLW1pIn0.BquzjN6d0g4zlvkbkdVwNEfRxIXSmxYwCHMk6UG3iza2fVioiOrcoP4Cp9P5--AB4G_CAhIXaP7YIZs3mq05QiDjSvkVAM0t67UPGhEr66sNXkV72iZBnKca_auh6EHsjPfxeVHkE1wdrsncrYdKhzgO4IAj8Jg4N5qjcE2q-OkliadmEuTwrhPhq" }
+  let(:token_response_mock) do
+    rm = Net::HTTPSuccess.new("1.0", "400", "OK")
+    allow(rm).to receive(:body).and_return(token_response_body)
+    rm
+  end
+  let(:client_id) { SecureRandom.uuid }
+  let(:http_mock) { instance_double("Net::HTTP", :use_ssl= => nil) }
+  let(:token_uri) { URI.parse("http://169.254.169.254/metadata/identity/oauth2/token") }
+  let(:vault_name) { "my-vault" }
+  let(:secret_name) { "my-secret" }
+  let(:vault_secret_uri) { URI.parse("https://#{vault_name}.vault.azure.net/secrets/#{secret_name}/?api-version=7.2") }
+
+  before do
+    # Cache these up front so we can pass into allow statements without hitting:
+    #   URI received :parse with unexpected arguments
+    token_uri
+    vault_secret_uri
+  end
+
+  before do
+    allow(Net::HTTP).to receive(:new).and_return(http_mock)
+    allow(URI).to receive(:parse).with("http://169.254.169.254/metadata/identity/oauth2/token").and_return(token_uri)
+    allow(URI).to receive(:parse).with("https://#{vault_name}.vault.azure.net/secrets/#{secret_name}/?api-version=7.2").and_return(vault_secret_uri)
+    allow(http_mock).to receive(:get).with(token_uri, { "Metadata" => "true" }).and_return(token_response_mock)
+    allow(http_mock).to receive(:get).with(vault_secret_uri, { "Authorization" => "Bearer #{access_token}", "Content-Type" => "application/json" }).and_return(secrets_response_mock)
+  end
+
+  describe "#validate!" do
+    it "raises error when more than one is provided: :object_id, :client_id, :mi_res_id" do
+      expect { Chef::SecretFetcher::AzureKeyVault.new({ object_id: "abc", client_id: "abc", mi_res_id: "abc" }, nil).validate! }.to raise_error(Chef::Exceptions::Secret::ConfigurationInvalid)
+      expect { Chef::SecretFetcher::AzureKeyVault.new({ object_id: "abc", client_id: "abc" }, nil).validate! }.to raise_error(Chef::Exceptions::Secret::ConfigurationInvalid)
+      expect { Chef::SecretFetcher::AzureKeyVault.new({ object_id: "abc", mi_res_id: "abc" }, nil).validate! }.to raise_error(Chef::Exceptions::Secret::ConfigurationInvalid)
+      expect { Chef::SecretFetcher::AzureKeyVault.new({ client_id: "abc", mi_res_id: "abc" }, nil).validate! }.to raise_error(Chef::Exceptions::Secret::ConfigurationInvalid)
+    end
+  end
 
-  context "when performing a fetch" do
-    let(:body) { '{ "value" : "my secret value" }' }
-    let(:response_mock) { double("response", body: body) }
-    let(:http_mock) { double("http", :get => response_mock, :use_ssl= => nil) }
+  describe "#fetch_token" do
+    context "when Net::HTTPBadRequest is returned and the error description contains \"Identity not found\"" do
+      let(:token_response_mock) { Net::HTTPBadRequest.new("1.0", "400", "Bad Request") }
+
+      before do
+        allow(fetcher).to receive(:fetch_token).and_call_original
+        allow(token_response_mock).to receive(:body).and_return('{"error":"invalid_request","error_description":"Identity not found"}')
+      end
 
-    before do
-      allow(fetcher).to receive(:fetch_token).and_return "a token"
-      allow(Net::HTTP).to receive(:new).and_return(http_mock)
+      it "raises Chef::Exceptions::Secret::Azure::IdentityNotFound" do
+        expect { fetcher.send(:fetch_token) }.to raise_error(Chef::Exceptions::Secret::Azure::IdentityNotFound)
+      end
     end
 
-    context "and vault name is only provided in the secret name" do
-      let(:body) { '{ "value" : "my secret value" }' }
+    context "when :object_id is provided" do
+      let(:object_id) { SecureRandom.uuid }
+      let(:config) { { vault: "my-vault", object_id: object_id } }
+
+      it "adds client_id to request params" do
+        fetcher.send(:fetch_token)
+        expect(token_uri.query).to match(/object_id=#{object_id}/)
+      end
+    end
+
+    context "when :client_id is provided" do
+      let(:config) { { vault: "my-vault", client_id: client_id } }
+
+      it "adds client_id to request params" do
+        fetcher.send(:fetch_token)
+        expect(token_uri.query).to match(/client_id=#{client_id}/)
+      end
+    end
+
+    context "when :mi_res_id is provided" do
+      let(:mi_res_id) { SecureRandom.uuid }
+      let(:config) { { vault: "my-vault", mi_res_id: mi_res_id } }
+
+      it "adds client_id to request params" do
+        fetcher.send(:fetch_token)
+        expect(token_uri.query).to match(/mi_res_id=#{mi_res_id}/)
+      end
+    end
+  end
+
+  describe "#fetch" do
+    context "when vault name is only provided in the secret name" do
+      let(:secrets_response_body) { '{ "value" : "my secret value" }' }
       let(:config) { {} }
       it "fetches the value" do
-        expect(fetcher.fetch("my_vault/value")).to eq "my secret value"
+        expect(fetcher.fetch("my-vault/my-secret")).to eq "my secret value"
       end
     end
 
-    context "and vault name is not provided in the secret name" do
+    context "when vault name is not provided in the secret name" do
       context "and vault name is not provided in config" do
         let(:config) { {} }
         it "raises a ConfigurationInvalid exception" do
-          expect { fetcher.fetch("value") }.to raise_error(Chef::Exceptions::Secret::ConfigurationInvalid)
+          expect { fetcher.fetch("my-secret") }.to raise_error(Chef::Exceptions::Secret::ConfigurationInvalid)
         end
       end
 
       context "and vault name is provided in config" do
-        let(:config) { { vault: "my_vault" } }
+        let(:config) { { vault: "my-vault" } }
         it "fetches the value" do
-          expect(fetcher.fetch("value")).to eq "my secret value"
+          expect(fetcher.fetch("my-secret")).to eq "my secret value"
         end
       end
     end
-    context "and an error response is received in the body" do
-      let(:config) { { vault: "my_vault" } }
-      let(:body) { '{ "error" : { "code" : 404, "message" : "secret not found" } }' }
+
+    context "when an error response is received in the response body" do
+      let(:config) { { vault: "my-vault" } }
+      let(:secrets_response_body) { '{ "error" : { "code" : 404, "message" : "secret not found" } }' }
       it "raises FetchFailed" do
-        expect { fetcher.fetch("value") }.to raise_error(Chef::Exceptions::Secret::FetchFailed)
+        expect { fetcher.fetch("my-secret") }.to raise_error(Chef::Exceptions::Secret::FetchFailed)
       end
     end
   end
 end
-