Add validation for scopes attribute for secure param

Author: aishkan

config/locales/en.yml

@@ -238,10 +238,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}.'
-            scope_requires_secure_parameter: Scopes can only be defined on secure
-              parameters.
-            scopes_cannot_be_empty: Scopes cannot be empty when defined.
-            invalid_secure_parameter_scopes: 'Invalid secure parameter scopes: %{invalid_scope}.'
+            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

@@ -628,14 +628,18 @@ parts:
       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.scope_requires_secure_parameter"
+      key: "txt.apps.admin.error.app_build.field_requires_secure_parameter"
       title: "App builder job: scopes can be defined only on secure parameters"
-      value: "Scopes can only be defined on secure parameters."
+      value: "%{field} cannot be defined in a non-secure parameter"
   - translation:
-      key: "txt.apps.admin.error.app_build.scopes_cannot_be_empty"
-      title: "App builder job: scopes cannot be empty when defined"
-      value: "Scopes cannot be empty when defined."
+      key: "txt.apps.admin.error.app_build.field_cannot_be_empty"
+      title: "App builder job: field cannot be empty"
+      value: "%{field} cannot be empty."
   - translation:
-      key: "txt.apps.admin.error.app_build.invalid_secure_parameter_scopes"
-      title: "App builder job: invalid secure parameter scopes"
-      value: "Invalid secure parameter scopes: %{invalid_scope}."
+      key: "txt.apps.admin.error.app_build.field_contains_invalid_values"
+      title: "App builder job: invalid field values"
+      value: "%{field} contains invalid values: %{values}."
+  - translation:
+      key: "txt.apps.admin.error.app_build.field_contains_invalid_keys"
+      title: "App builder job: invalid field keys"
+      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/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,49 @@ 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}]")
+            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}]", values: invalid_scopes)
+            end
+          end
+          
+          errors
+        end
+
         def marketing_only_errors(manifest)
           [
             ban_parameters(manifest),
@@ -261,12 +306,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