Merge pull request #372 from zendesk/mtsolakis/APPS-7218-add-warning-for-password-parameter

[APPS-7218] Add warning for password parameter

Author: aishkan

config/locales/en.yml

@@ -123,6 +123,8 @@ en:
                 for %{product}: %{missing_key}'
               products_do_not_match_manifest_products: Products in manifest (%{manifest_products})
                 do not match products in translations (%{translation_products})
+              password_parameter_deprecated: 'Password parameter type is deprecated and will not
+                be accepted in the future. Use Basic Access Authentication instead. Learn more: %{link}.'
               insecure_token_parameter_in_manifest: 'Make sure to set secure to true
                 when using keys in Settings. Learn more: %{link}'
               default_secure_or_hidden_parameter_in_manifest: Default values for secure

lib/zendesk_apps_support/package.rb

@@ -31,9 +31,9 @@ def initialize(dir, is_cached = true)
       @warnings = []
     end
 
-    def validate(marketplace: true, skip_marketplace_translations: false, apply_password_parameter_check: false)
+    def validate(marketplace: true, skip_marketplace_translations: false, error_on_password_parameter: false)
       errors = []
-      errors << Validations::Manifest.call(self, apply_password_parameter_check: apply_password_parameter_check)
+      errors << Validations::Manifest.call(self, error_on_password_parameter: error_on_password_parameter)
 
       if has_valid_manifest?(errors)
         errors << Validations::Marketplace.call(self) if marketplace
@@ -61,8 +61,8 @@ def validate(marketplace: true, skip_marketplace_translations: false, apply_pass
       errors.flatten.compact
     end
 
-    def validate!(marketplace: true, skip_marketplace_translations: false, apply_password_parameter_check: false)
-      errors = validate(marketplace: marketplace, skip_marketplace_translations: skip_marketplace_translations, apply_password_parameter_check: apply_password_parameter_check)
+    def validate!(marketplace: true, skip_marketplace_translations: false, error_on_password_parameter: false)
+      errors = validate(marketplace: marketplace, skip_marketplace_translations: skip_marketplace_translations, error_on_password_parameter: error_on_password_parameter)
       raise errors.first if errors.any?
       true
     end

lib/zendesk_apps_support/validations/manifest.rb

@@ -13,7 +13,7 @@ module Manifest
       OAUTH_MANIFEST_LINK = 'https://developer.zendesk.com/apps/docs/developer-guide/manifest#oauth'
 
       class << self
-        def call(package, apply_password_parameter_check: false)
+        def call(package, error_on_password_parameter: false)
           unless package.has_file?('manifest.json')
             nested_manifest = package.files.find { |file| file =~ %r{\A[^/]+?/manifest\.json\Z} }
             if nested_manifest
@@ -22,7 +22,9 @@ def call(package, apply_password_parameter_check: false)
             return [ValidationError.new(:missing_manifest)]
           end
 
-          collate_manifest_errors(package, apply_password_parameter_check)
+          package.warnings << password_parameter_warning unless error_on_password_parameter
+
+          collate_manifest_errors(package, error_on_password_parameter)
         rescue JSON::ParserError => e
           return [ValidationError.new(:manifest_not_json, errors: e)]
         rescue ZendeskAppsSupport::Manifest::OverrideError => e
@@ -31,7 +33,7 @@ def call(package, apply_password_parameter_check: false)
 
         private
 
-        def collate_manifest_errors(package, apply_password_parameter_check)
+        def collate_manifest_errors(package, error_on_password_parameter)
           manifest = package.manifest
 
           errors = [
@@ -50,7 +52,7 @@ def collate_manifest_errors(package, apply_password_parameter_check)
                 invalid_version_error(manifest) ]
             end,
             ban_no_template(manifest),
-            if apply_password_parameter_check
+            if error_on_password_parameter
              [ deprecate_password_parameter_type(manifest) ]
             end
           ]
@@ -449,6 +451,13 @@ def valid_url?(value)
         rescue URI::InvalidURIError
           false
         end
+
+        def password_parameter_warning
+          I18n.t(
+            'txt.apps.admin.error.app_build.translation.password_parameter_deprecated',
+            link: 'https://developer.zendesk.com/documentation/apps/app-developer-guide/making-api-requests-from-a-zendesk-app/#using-basic-access-authentication'
+          )
+        end
       end
     end
   end

spec/package_spec.rb

@@ -455,15 +455,15 @@ def build_app_source_with_files(files)
       end
     end
 
-    context 'when apply_password_parameter_check is true' do
+    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, apply_password_parameter_check: true)
+        package.validate!(marketplace: true, error_on_password_parameter: true)
       end
-      it 'validate manifest and passes in the apply_password_parameter_check correctly' do
-        expect(ZendeskAppsSupport::Validations::Manifest).to have_received(:call).with(package, {:apply_password_parameter_check => true})
+      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
   end

spec/validations/manifest_spec.rb

@@ -814,35 +814,37 @@ def create_package(parameter_hash)
     end
   end
 
-  context 'when the apply_password_parameter_check option is set to false' do
+  context 'when the error_on_password_parameter option is set to false' do
 
-    it 'should be valid with a password param type' do
-      apply_password_parameter_check = false
+    it 'should be valid but add a warning about the password param type' do
+      error_on_password_parameter = false
       @manifest_hash = {
         'parameters' => [
           'name'     => 'a password param',
           'type'     => 'password',
         ]}
         package = create_package(@manifest_hash)
-        errors = ZendeskAppsSupport::Validations::Manifest.call(package, apply_password_parameter_check: apply_password_parameter_check)
+        errors = ZendeskAppsSupport::Validations::Manifest.call(package, error_on_password_parameter: error_on_password_parameter)
 
       expect(errors.map(&:to_s).join()).not_to include("Password parameter type can no longer be used")
+      expect(package.warnings.join()).to include("Password parameter type is deprecated and will not be accepted in the future")
     end
   end
 
-  context 'when the apply_password_parameter_check option is set to true' do
+  context 'when the error_on_password_parameter option is set to true' do
 
-    it 'should not be valid with a password param type' do
-      apply_password_parameter_check = true
+    it 'should not be valid with a password param type and add no warnings' do
+      error_on_password_parameter = true
       @manifest_hash = {
         'parameters' => [
           'name'     => 'a password param',
           'type'     => 'password',
         ]}
         package = create_package(@manifest_hash)
-        errors = ZendeskAppsSupport::Validations::Manifest.call(package, apply_password_parameter_check: apply_password_parameter_check)
-        
+        errors = ZendeskAppsSupport::Validations::Manifest.call(package, error_on_password_parameter: error_on_password_parameter)
+
       expect(errors.map(&:to_s).join()).to include("Password parameter type can no longer be used")
+      expect(package.warnings.join()).not_to include("Password parameter type is deprecated and will not be accepted in the future")
     end
   end
 end