Merge pull request #404 from zendesk/aishkan/secure-settings-scope

[APPS-7700] Add validation for scopes attribute for secure param

Author: aishkan

config/locales/en.yml

@@ -281,6 +281,11 @@ en:
             invalid_url: '%{field} must be a valid URL, got "%{value}".'
             password_parameter_type_deprecated: 'Password parameter type can no longer
               be used. Use Secure settings instead. Learn more: %{link}.'
+            field_requires_secure_parameter: "%{field} cannot be defined in a non-secure
+              parameter."
+            field_cannot_be_empty: "%{field} cannot be empty."
+            field_contains_invalid_values: "%{field} contains invalid values: %{values}."
+            field_contains_invalid_keys: "%{field} contains invalid keys: %{keys}."
         warning:
           app_build:
             deprecated_version: You are targeting a deprecated version of the framework.

config/locales/translations/zendesk_apps_support.yml

@@ -633,3 +633,19 @@ parts:
       key: "txt.apps.admin.error.app_build.password_parameter_type_deprecated"
       title: "App builder job: Password parameter type is deprecated"
       value: "Password parameter type can no longer be used. Use Secure settings instead. Learn more: %{link}."
+  - translation:
+      key: "txt.apps.admin.error.app_build.field_requires_secure_parameter"
+      title: "App builder job: Error when parameter field requires secure setting set to true. Placeholder %{field} shows parameter field along with parameter name like \"parameter[name='param'].scopes\" found in the supplied manifest file."
+      value: "%{field} cannot be defined in a non-secure parameter."
+  - translation:
+      key: "txt.apps.admin.error.app_build.field_cannot_be_empty"
+      title: "App builder job: field cannot be empty. %{field} will be replaced with empty fields such as \"name\", \"author\" \"parameter[name='param'].scopes\" etc in the supplied manifest file."
+      value: "%{field} cannot be empty."
+  - translation:
+      key: "txt.apps.admin.error.app_build.field_contains_invalid_values"
+      title: "App builder job: Error for invalid field values. Placeholder %{field} shows parameter fields like \"parameter[name='param'].scopes\" in the supplied manifest file, %{values} shows the invalid values in the user input corresponding to those fields."
+      value: "%{field} contains invalid values: %{values}."
+  - translation:
+      key: "txt.apps.admin.error.app_build.field_contains_invalid_keys"
+      title: "App builder job: Error for invalid field keys. Placeholder %{field} shows parameter fields like \"parameter[name='param'].scopes\" provided in the supplied manifest file, %{keys} shows the invalid keys found within the field."
+      value: "%{field} contains invalid keys: %{keys}."

lib/zendesk_apps_support/manifest/parameter.rb

@@ -4,7 +4,7 @@ module ZendeskAppsSupport
   class Manifest
     class Parameter
       TYPES = %w[text password checkbox url number multiline hidden oauth].freeze
-      ATTRIBUTES = %i[name type required secure default].freeze
+      ATTRIBUTES = %i[name type required secure default scopes].freeze
       attr_reader(*ATTRIBUTES)
       def default?
         @has_default
@@ -17,6 +17,7 @@ def initialize(p)
         @secure = p['secure'] || false
         @has_default = p.key? 'default'
         @default = p['default'] if @has_default
+        @scopes = p['scopes']
       end
     end
   end

lib/zendesk_apps_support/package.rb

@@ -36,9 +36,10 @@ def validate(options = {})
       skip_marketplace_translations = options.fetch(:skip_marketplace_translations, false)
       error_on_password_parameter = options.fetch(:error_on_password_parameter, false)
       validate_custom_objects_v2 = options.fetch(:validate_custom_objects_v2, false)
+      validate_scopes_for_secure_parameter = options.fetch(:validate_scopes_for_secure_parameter, false)
 
       errors = []
-      errors << Validations::Manifest.call(self, error_on_password_parameter: error_on_password_parameter)
+      errors << Validations::Manifest.call(self, error_on_password_parameter: error_on_password_parameter, validate_scopes_for_secure_parameter: validate_scopes_for_secure_parameter)
 
       if has_valid_manifest?(errors)
         errors << Validations::Marketplace.call(self) if marketplace

lib/zendesk_apps_support/validations/manifest.rb

@@ -11,9 +11,10 @@ module Manifest
       OAUTH_REQUIRED_FIELDS = %w[client_id client_secret authorize_uri access_token_uri].freeze
       PARAMETER_TYPES = ZendeskAppsSupport::Manifest::Parameter::TYPES
       OAUTH_MANIFEST_LINK = 'https://developer.zendesk.com/apps/docs/developer-guide/manifest#oauth'
+      SECURE_PARAM_SCOPES = %w[header body url_host url_path jwt_secret_key jwt_claim basic_auth_username basic_auth_password].freeze
 
       class << self
-        def call(package, error_on_password_parameter: false)
+        def call(package, error_on_password_parameter: false, validate_scopes_for_secure_parameter: false)
           unless package.has_file?('manifest.json')
             nested_manifest = package.files.find { |file| file =~ %r{\A[^/]+?/manifest\.json\Z} }
             if nested_manifest
@@ -24,7 +25,7 @@ def call(package, error_on_password_parameter: false)
 
           package.warnings << password_parameter_warning if !error_on_password_parameter && password_param_present?(package.manifest)
 
-          collate_manifest_errors(package, error_on_password_parameter)
+          collate_manifest_errors(package, error_on_password_parameter, validate_scopes_for_secure_parameter)
         rescue JSON::ParserError => e
           return [ValidationError.new(:manifest_not_json, errors: e)]
         rescue ZendeskAppsSupport::Manifest::OverrideError => e
@@ -37,7 +38,7 @@ def password_param_present?(manifest)
           manifest.parameters.any? { |p| p.type == 'password' }
         end
 
-        def collate_manifest_errors(package, error_on_password_parameter)
+        def collate_manifest_errors(package, error_on_password_parameter, validate_scopes_for_secure_parameter)
           manifest = package.manifest
 
           errors = [
@@ -46,7 +47,7 @@ def collate_manifest_errors(package, error_on_password_parameter)
             oauth_error(manifest),
             default_locale_error(manifest, package),
             validate_urls(manifest),
-            validate_parameters(manifest),
+            validate_parameters(manifest, validate_scopes_for_secure_parameter),
             if manifest.requirements_only? || manifest.marketing_only?
               [ ban_location(manifest),
                 ban_framework_version(manifest) ]
@@ -87,7 +88,7 @@ def validate_urls(manifest)
           errors
         end
 
-        def validate_parameters(manifest)
+        def validate_parameters(manifest, validate_scopes_for_secure_parameter)
           if manifest.marketing_only?
             marketing_only_errors(manifest)
           else
@@ -97,7 +98,8 @@ def validate_parameters(manifest)
               invalid_type_error(manifest),
               too_many_oauth_parameters(manifest),
               oauth_cannot_be_secure(manifest),
-              name_as_parameter_name_error(manifest)
+              name_as_parameter_name_error(manifest),
+              *(validate_scopes_for_secure_parameter ? invalid_secure_param_scopes_errors(manifest) : invalid_scopes_key_error(manifest)),
             ]
           end
         end
@@ -110,6 +112,52 @@ def oauth_cannot_be_secure(manifest)
           end
         end
 
+        def invalid_scopes_key_error(manifest)
+          errors = []
+          manifest.parameters.each do |parameter|
+            next if parameter.scopes.nil?
+
+            errors << ValidationError.new(:field_contains_invalid_keys,
+                                        field: "parameters[name=\"#{parameter.name}\"]",
+                                        keys: 'scopes')
+          end
+          errors
+        end
+
+        def invalid_secure_param_scopes_errors(manifest)
+          errors = []
+
+          manifest.parameters.each do |parameter|
+            next if parameter.scopes.nil?
+
+            unless parameter.secure
+              errors << ValidationError.new(:field_requires_secure_parameter, field: "parameters[name=\"#{parameter.name}\"].scopes")
+            end
+
+            unless parameter.scopes.is_a?(Array)
+              errors << ValidationError.new(:unacceptable_array,
+                                          field: "parameters[name=\"#{parameter.name}\"].scopes",
+                                          value: parameter.scopes.class.to_s)
+              next
+            end
+
+            if parameter.scopes.empty?
+              errors << ValidationError.new(:field_cannot_be_empty, field: "parameters[name=\"#{parameter.name}\"].scopes")
+            end
+
+            invalid_scopes = parameter.scopes - SECURE_PARAM_SCOPES
+            if invalid_scopes.any?
+                errors << ValidationError.new(
+                  :field_contains_invalid_values,
+                  field: "parameters[name=\"#{parameter.name}\"].scopes",
+                  values: invalid_scopes.join(I18n.t('txt.apps.admin.error.app_build.listing_comma'))
+                )
+            end
+          end
+
+          errors
+        end
+
         def marketing_only_errors(manifest)
           [
             ban_parameters(manifest),
@@ -261,12 +309,12 @@ def parameters_error(manifest)
           end
 
           invalid_required = manifest.parameters.map do |parameter|
-            validate_boolean(parameter.required, "parameters.#{parameter.name}.required")
+            validate_boolean(parameter.required, "parameters[name=\"#{parameter.name}\"].required")
           end.compact
           return invalid_required if invalid_required.any?
 
           invalid_secure = manifest.parameters.map do |parameter|
-            validate_boolean(parameter.secure, "parameters.#{parameter.name}.secure")
+            validate_boolean(parameter.secure, "parameters[name=\"#{parameter.name}\"].secure")
           end.compact
           return invalid_secure if invalid_secure.any?
         end

spec/package_spec.rb

@@ -455,18 +455,6 @@ def build_app_source_with_files(files)
       end
     end
 
-    context 'when error_on_password_parameter is true' do
-      let(:package) { ZendeskAppsSupport::Package.new('spec/fixtures/iframe_only_app') }
-
-      before do
-        allow(ZendeskAppsSupport::Validations::Manifest).to receive(:call)
-        package.validate!(marketplace: true, error_on_password_parameter: true)
-      end
-      it 'validate manifest and passes in the error_on_password_parameter correctly' do
-        expect(ZendeskAppsSupport::Validations::Manifest).to have_received(:call).with(package, {:error_on_password_parameter => true})
-      end
-    end
-
     context 'when validate_custom_objects_v2 is true' do
       let(:package) { ZendeskAppsSupport::Package.new('spec/fixtures/iframe_only_app') }
 
@@ -481,18 +469,25 @@ def build_app_source_with_files(files)
     end
 
     context 'when handling validation options' do
+      let(:package) { ZendeskAppsSupport::Package.new('spec/fixtures/iframe_only_app') }
+
       before do
         allow(ZendeskAppsSupport::Validations::Manifest).to receive(:call).and_return([])
         allow(ZendeskAppsSupport::Validations::Translations).to receive(:call)
         allow(ZendeskAppsSupport::Validations::Requirements).to receive(:call)
       end
 
+      it 'passes manifest validation options correctly' do
+        package.validate!(marketplace: true, error_on_password_parameter: true, validate_scopes_for_secure_parameter: true)
+        expect(ZendeskAppsSupport::Validations::Manifest).to have_received(:call).with(package, {:error_on_password_parameter => true, :validate_scopes_for_secure_parameter => true})
+      end
+
       it 'uses default values when called with empty options' do
         package.validate!
 
         expect(ZendeskAppsSupport::Validations::Manifest).to have_received(:call).with(
           package,
-          { error_on_password_parameter: false }
+          { error_on_password_parameter: false, validate_scopes_for_secure_parameter: false }
         )
         expect(ZendeskAppsSupport::Validations::Marketplace).to have_received(:call).with(package)
         expect(ZendeskAppsSupport::Validations::Translations).to have_received(:call).with(