Merge pull request #368 from 3scale/openapi-support-multiple-flows

THREESCALE-9768 OpenAPI support for multiple flows in oauth

Author: eguzki

docs/openapi.md

@@ -72,7 +72,6 @@ $ tool_to_read_openapi_from_source | 3scale import openapi -d <destination> -
   * Only first `servers[0].url` element in `servers` list parsed as *private base url*. As OpenAPI specification`basePath` property, `servers[0].url` URL's base path component will be used.
   * Toolbox will *not* parse servers in path item or operation objects.
   * Supported security schemes: apiKey, oauth2 (any flow type).
-  * Multiple flows in security scheme object not supported.
 
 ### Importing 3scale Backend
 

lib/3scale_toolbox/commands/import_command/openapi/update_service_oidc_conf_step.rb

@@ -29,13 +29,7 @@ def add_flow_settings(settings)
             # only applies to oauth2 sec type
             return if api_spec.security.nil? || api_spec.security[:type] != 'oauth2'
 
-            oidc_configuration = {
-              standard_flow_enabled: false,
-              implicit_flow_enabled: false,
-              service_accounts_enabled: false,
-              direct_access_grants_enabled: false
-            }.merge(api_spec.security[:flow] => true)
-            settings.merge!(oidc_configuration)
+            settings.merge!(api_spec.security[:flows] || {})
           end
         end
       end

lib/3scale_toolbox/openapi/oas3.rb

@@ -21,7 +21,11 @@ module OpenAPI
     #     * :type -> string
     #     * :name -> string
     #     * :in_f -> string
-    #     * :flow -> symbol (:implicit_flow_enabled, :direct_access_grants_enabled, :service_accounts_enabled, :standard_flow_enabled)
+    #     * :flows -> hash
+    #       * :implicit_flow_enabled -> bool
+    #       * :direct_access_grants_enabled -> bool
+    #       * :service_accounts_enabled -> bool
+    #       * :standard_flow_enabled -> bool
     #     * :scopes -> array of string
     # * OAS3.service_backend_version -> string ('1','2','oidc')
     # * OAS3.set_server_url -> def(spec, url)
@@ -103,9 +107,10 @@ def set_oauth2_urls(spec, sec_scheme_id, authorization_url, token_url)
           raise ThreeScaleToolbox::Error, "Expected security scheme {#{sec_scheme_id}} not found or not oauth2"
         end
 
-        flow_key, flow_obj = sec_scheme_obj['flows'].first
-        flow_obj['authorizationUrl'] = authorization_url if %w[implicit authorizationCode].include?(flow_key)
-        flow_obj['tokenUrl'] = token_url if %w[password clientCredentials authorizationCode].include?(flow_key)
+        sec_scheme_obj['flows'].each do |flow_key, flow_obj|
+          flow_obj['authorizationUrl'] = authorization_url if %w[implicit authorizationCode].include?(flow_key)
+          flow_obj['tokenUrl'] = token_url if %w[password clientCredentials authorizationCode].include?(flow_key)
+        end
       end
 
       def set_server_url(spec, url)
@@ -184,7 +189,7 @@ def parse_global_security_reqs
               type: sec_def['type'],
               name: sec_def['name'],
               in_f: sec_def['in'],
-              flow: parse_flows(sec_def['flows']),
+              flows: parse_flows(sec_def['flows']),
               scopes: sec_item
             }
           end
@@ -208,9 +213,18 @@ def security_schemes
       def parse_flows(flows_object)
         return nil if flows_object.nil?
 
-        raise ThreeScaleToolbox::Error, 'Invalid OAS: multiple flows' if flows_object.size > 1
+        flows_object.keys.reduce(basic_flows_object) do |obj, key|
+          obj.merge!(convert_flow(key) => true)
+        end
+      end
 
-        convert_flow(flows_object.keys.first)
+      def basic_flows_object
+        {
+          standard_flow_enabled: false,
+          implicit_flow_enabled: false,
+          service_accounts_enabled: false,
+          direct_access_grants_enabled: false
+        }
       end
 
       def convert_flow(flow_name)

lib/3scale_toolbox/openapi/swagger.rb

@@ -21,7 +21,11 @@ module OpenAPI
     #     * :type -> string
     #     * :name -> string
     #     * :in_f -> string
-    #     * :flow -> symbol (:implicit_flow_enabled, :direct_access_grants_enabled, :service_accounts_enabled, :standard_flow_enabled)
+    #     * :flows -> hash
+    #       * :implicit_flow_enabled -> bool
+    #       * :direct_access_grants_enabled -> bool
+    #       * :service_accounts_enabled -> bool
+    #       * :standard_flow_enabled -> bool
     #     * :scopes -> array of string
     # * Swagger.service_backend_version -> string ('1','2','oidc')
     # * Swagger.set_server_url -> def(spec, url)
@@ -150,7 +154,7 @@ def parse_global_security_reqs
               type: sec_def['type'],
               name: sec_def['name'],
               in_f: sec_def['in'],
-              flow: convert_flow(sec_def['flow']),
+              flows: parse_flows(sec_def['flow']),
               scopes: sec_item
             }
           end
@@ -171,6 +175,21 @@ def security_definitions
         raw['securityDefinitions'] || {}
       end
 
+      def parse_flows(flow_name)
+        return nil if flow_name.nil?
+
+        basic_flows_object.merge!({ convert_flow(flow_name) => true })
+      end
+
+      def basic_flows_object
+        {
+          standard_flow_enabled: false,
+          implicit_flow_enabled: false,
+          service_accounts_enabled: false,
+          direct_access_grants_enabled: false
+        }
+      end
+
       def convert_flow(flow_name)
         return nil if flow_name.nil?
 

spec/shared_oas3_contexts.rb

@@ -345,6 +345,16 @@
           petstore_oauth:
             type: oauth2
             flows:
+              clientCredentials:
+                tokenUrl: http://example.org/api/oauth/dialog
+                scopes:
+                  write:pets: modify pets in your account
+                  read:pets: read your pets
+              authorizationCode:
+                authorizationUrl: https://example.com/api/oauth/dialog
+                tokenUrl: http://example.org/api/oauth/token
+                scopes:
+                  write:pets: modify pets in your account
               implicit:
                 authorizationUrl: http://example.org/api/oauth/dialog
                 scopes:

spec/unit/commands/import_command/openapi/update_service_oidc_conf_spec.rb

@@ -20,7 +20,7 @@
     {
       target: service,
       api_spec: api_spec,
-      logger: logger,
+      logger: logger
     }
   end
 
@@ -56,35 +56,68 @@
       let(:expected_implicit_flow) { false }
       let(:expected_service_accounts) { false }
       let(:expected_direct_access_grants) { false }
-      let(:security) { { id: 'oidc', type: 'oauth2', flow: flow } }
+      let(:basic_empty_flow) do
+        {
+          standard_flow_enabled: false, implicit_flow_enabled: false,
+          service_accounts_enabled: false, direct_access_grants_enabled: false
+        }
+      end
+
+      let(:security) { { id: 'oidc', type: 'oauth2', flows: flows } }
+
+      context 'no flows' do
+        let(:flows) { nil }
+
+        it 'service is not updated' do
+          # if service.update_oidc is called, this test should fail
+          subject
+        end
+      end
 
       context 'flow implicit' do
-        let(:flow) { :implicit_flow_enabled }
+        let(:flows) { basic_empty_flow.merge(implicit_flow_enabled: true) }
         let(:expected_implicit_flow) { true }
 
         it_behaves_like 'oidc is updated with required flow'
       end
 
       context 'flow password' do
-        let(:flow) { :direct_access_grants_enabled }
+        let(:flows) { basic_empty_flow.merge(direct_access_grants_enabled: true) }
         let(:expected_direct_access_grants) { true }
 
         it_behaves_like 'oidc is updated with required flow'
       end
 
       context 'flow application' do
-        let(:flow) { :service_accounts_enabled }
+        let(:flows) { basic_empty_flow.merge(service_accounts_enabled: true) }
         let(:expected_service_accounts) { true }
 
         it_behaves_like 'oidc is updated with required flow'
       end
 
       context 'flow accessCode' do
-        let(:flow) { :standard_flow_enabled }
+        let(:flows) { basic_empty_flow.merge(standard_flow_enabled: true) }
         let(:expected_standard_flow) { true }
 
         it_behaves_like 'oidc is updated with required flow'
       end
+
+      context 'all flows' do
+        let(:flows) do
+          {
+            standard_flow_enabled: true,
+            implicit_flow_enabled: true,
+            service_accounts_enabled: true,
+            direct_access_grants_enabled: true
+          }
+        end
+        let(:expected_standard_flow) { true }
+        let(:expected_service_accounts) { true }
+        let(:expected_direct_access_grants) { true }
+        let(:expected_implicit_flow) { true }
+
+        it_behaves_like 'oidc is updated with required flow'
+      end
     end
   end
 end

spec/unit/openapi/oas3_spec.rb

@@ -6,6 +6,12 @@
   let(:path) { '/path/to/petstore.yaml' }
   subject { described_class.build(path, raw_specification, validate: validate) }
   let(:content) { basic_oas3_content }
+  let(:basic_empty_flow) do
+    {
+      standard_flow_enabled: false, implicit_flow_enabled: false,
+      service_accounts_enabled: false, direct_access_grants_enabled: false
+    }
+  end
 
   context 'missing info' do
     let(:content) do
@@ -262,7 +268,7 @@
       end
 
       it 'flow matches' do
-        expect(subject.security[:flow]).to be_nil
+        expect(subject.security[:flows]).to be_nil
       end
 
       it 'scopes matches' do
@@ -294,7 +300,7 @@
       end
 
       it 'flow matches' do
-        expect(subject.security[:flow]).to be(:implicit_flow_enabled)
+        expect(subject.security[:flows]).to eq(basic_empty_flow.merge(implicit_flow_enabled: true))
       end
 
       it 'scopes matches' do
@@ -326,7 +332,7 @@
       end
 
       it 'flow matches' do
-        expect(subject.security[:flow]).to be(:direct_access_grants_enabled)
+        expect(subject.security[:flows]).to eq(basic_empty_flow.merge(direct_access_grants_enabled: true))
       end
 
       it 'scopes matches' do
@@ -358,7 +364,7 @@
       end
 
       it 'flow matches' do
-        expect(subject.security[:flow]).to be(:service_accounts_enabled)
+        expect(subject.security[:flows]).to eq(basic_empty_flow.merge(service_accounts_enabled: true))
       end
 
       it 'scopes matches' do
@@ -390,7 +396,7 @@
       end
 
       it 'flow matches' do
-        expect(subject.security[:flow]).to be(:standard_flow_enabled)
+        expect(subject.security[:flows]).to eq(basic_empty_flow.merge(standard_flow_enabled: true))
       end
 
       it 'scopes matches' do
@@ -459,9 +465,41 @@
     context 'multiple flow security schema' do
       let(:content) { oauth2_multiple_flow_oas3_content }
 
-      it 'parsing raises error' do
-        expect { subject.security }.to raise_error(ThreeScaleToolbox::Error,
-                                                   /Invalid OAS: multiple flows/)
+      it 'matches oidc version' do
+        expect(subject.service_backend_version).to eq('oidc')
+      end
+
+      it 'available' do
+        expect(subject.security).not_to be_nil
+      end
+
+      it 'id matches' do
+        expect(subject.security[:id]).to eq('petstore_oauth')
+      end
+
+      it 'type matches' do
+        expect(subject.security[:type]).to eq('oauth2')
+      end
+
+      it 'name matches' do
+        expect(subject.security[:name]).to be_nil
+      end
+
+      it 'in_f matches' do
+        expect(subject.security[:in_f]).to be_nil
+      end
+
+      it 'flow matches' do
+        expect(subject.security[:flows]).to eq(basic_empty_flow.merge({
+                                                                       implicit_flow_enabled: true,
+                                                                       direct_access_grants_enabled: true,
+                                                                       service_accounts_enabled: true,
+                                                                       standard_flow_enabled: true
+                                                                     }))
+      end
+
+      it 'scopes matches' do
+        expect(subject.security[:scopes]).to match_array(['write:pets', 'read:pets'])
       end
     end
   end

spec/unit/openapi/swagger_spec.rb

@@ -5,6 +5,12 @@
   let(:validate) { true }
   subject { described_class.build(raw_specification, validate: validate) }
   let(:content) { basic_swagger_content }
+  let(:basic_empty_flow) do
+    {
+      standard_flow_enabled: false, implicit_flow_enabled: false,
+      service_accounts_enabled: false, direct_access_grants_enabled: false
+    }
+  end
 
   context 'missing info' do
     let(:content) do
@@ -217,7 +223,7 @@
       end
 
       it 'flow matches' do
-        expect(subject.security[:flow]).to be_nil
+        expect(subject.security[:flows]).to be_nil
       end
 
       it 'scopes matches' do
@@ -249,7 +255,7 @@
       end
 
       it 'flow matches' do
-        expect(subject.security[:flow]).to be(:implicit_flow_enabled)
+        expect(subject.security[:flows]).to eq(basic_empty_flow.merge(implicit_flow_enabled: true))
       end
 
       it 'scopes matches' do
@@ -281,7 +287,7 @@
       end
 
       it 'flow matches' do
-        expect(subject.security[:flow]).to be(:direct_access_grants_enabled)
+        expect(subject.security[:flows]).to eq(basic_empty_flow.merge(direct_access_grants_enabled: true))
       end
 
       it 'scopes matches' do
@@ -313,7 +319,7 @@
       end
 
       it 'flow matches' do
-        expect(subject.security[:flow]).to be(:service_accounts_enabled)
+        expect(subject.security[:flows]).to eq(basic_empty_flow.merge(service_accounts_enabled: true))
       end
 
       it 'scopes matches' do
@@ -345,7 +351,7 @@
       end
 
       it 'flow matches' do
-        expect(subject.security[:flow]).to be(:standard_flow_enabled)
+        expect(subject.security[:flows]).to eq(basic_empty_flow.merge(standard_flow_enabled: true))
       end
 
       it 'scopes matches' do