Skip to content

Commit

Permalink
Add manifest validation to deprecate password type param
Browse files Browse the repository at this point in the history
  • Loading branch information
aishkan committed Aug 28, 2024
1 parent 73a7cfb commit 989c20c
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 9 deletions.
2 changes: 2 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ en:
oauth_parameter_cannot_be_secure: oauth parameter cannot be set to be
secure.
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}.'
warning:
app_build:
deprecated_version: You are targeting a deprecated version of the framework.
Expand Down
4 changes: 4 additions & 0 deletions config/locales/translations/zendesk_apps_support.yml
Original file line number Diff line number Diff line change
Expand Up @@ -415,3 +415,7 @@ parts:
key: "txt.apps.admin.error.app_build.invalid_url"
title: "App builder job: this value needs to be a valid URL, but something else was passed in. placeholder value is taken from user input. You can translate as: The value %{field} must be a valid URL... to avoid any gender issues."
value: "%{field} must be a valid URL, got \"%{value}\"."
- translation:
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}."
4 changes: 2 additions & 2 deletions lib/zendesk_apps_support/package.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ def initialize(dir, is_cached = true)
@warnings = []
end

def validate(marketplace: true, skip_marketplace_translations: false)
def validate(marketplace: true, skip_marketplace_translations: false, apply_password_parameter_check: false)
errors = []
errors << Validations::Manifest.call(self)
errors << Validations::Manifest.call(self, apply_password_parameter_check: apply_password_parameter_check)

if has_valid_manifest?(errors)
errors << Validations::Marketplace.call(self) if marketplace
Expand Down
18 changes: 14 additions & 4 deletions lib/zendesk_apps_support/validations/manifest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module Manifest
OAUTH_MANIFEST_LINK = 'https://developer.zendesk.com/apps/docs/developer-guide/manifest#oauth'

class << self
def call(package)
def call(package, opts={})
unless package.has_file?('manifest.json')
nested_manifest = package.files.find { |file| file =~ %r{\A[^/]+?/manifest\.json\Z} }
if nested_manifest
Expand All @@ -22,7 +22,7 @@ def call(package)
return [ValidationError.new(:missing_manifest)]
end

collate_manifest_errors(package)
collate_manifest_errors(package, opts.fetch(:apply_password_parameter_check) )
rescue JSON::ParserError => e
return [ValidationError.new(:manifest_not_json, errors: e)]
rescue ZendeskAppsSupport::Manifest::OverrideError => e
Expand All @@ -31,7 +31,7 @@ def call(package)

private

def collate_manifest_errors(package)
def collate_manifest_errors(package, apply_password_parameter_check)
manifest = package.manifest

errors = [
Expand All @@ -49,7 +49,10 @@ def collate_manifest_errors(package)
missing_framework_version(manifest),
invalid_version_error(manifest) ]
end,
ban_no_template(manifest)
ban_no_template(manifest),
if apply_password_parameter_check
[ deprecate_password_parameter_type(manifest) ]
end
]
errors.flatten.compact
end
Expand Down Expand Up @@ -207,6 +210,13 @@ def ban_framework_version(manifest)
ValidationError.new(:no_framework_version_required) unless manifest.framework_version.nil?
end

def deprecate_password_parameter_type(manifest)
secure_settings_link = 'https://developer.zendesk.com/documentation/apps/app-developer-guide/making-api-requests-from-a-zendesk-app/#using-secure-settings'
if manifest.parameters.any? { |p| p.type == 'password' }
ValidationError.new(:password_parameter_type_deprecated, link: secure_settings_link)
end
end

def private_marketing_app_error(manifest)
ValidationError.new(:marketing_only_app_cant_be_private) if manifest.private?
end
Expand Down
40 changes: 37 additions & 3 deletions spec/validations/manifest_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
require 'tmpdir'

describe ZendeskAppsSupport::Validations::Manifest do
let(:opts) {{ apply_password_parameter_check: false }}

def default_required_params(overrides = {})
{
'author' => 'author',
Expand All @@ -24,7 +26,7 @@ def create_package(parameter_hash)

RSpec::Matchers.define :have_error do |error|
match do |package|
errors = ZendeskAppsSupport::Validations::Manifest.call(package)
errors = ZendeskAppsSupport::Validations::Manifest.call(package, opts)
errors.map!(&:to_s) unless error.is_a? Symbol
@actual = errors.compact

Expand Down Expand Up @@ -141,7 +143,7 @@ def create_package(parameter_hash)
context 'with a marketing only app' do
it 'should not have any errors' do
@package = ZendeskAppsSupport::Package.new('spec/fixtures/marketing_only_app')
errors = ZendeskAppsSupport::Validations::Manifest.call(@package)
errors = ZendeskAppsSupport::Validations::Manifest.call(@package, opts)
expect(errors).to be_empty
end

Expand Down Expand Up @@ -556,7 +558,7 @@ def create_package(parameter_hash)
'access_token_uri' => '4'
}
}
expect(@package).to have_error(/Please upgrade to our new oauth format/)
expect(create_package(@manifest_hash)).to have_error(/Please upgrade to our new oauth format/)
end
end
end
Expand Down Expand Up @@ -813,4 +815,36 @@ def create_package(parameter_hash)
expect(@package).not_to have_error(/author url must be a valid URL/)
end
end

context 'when the apply_password_parameter_check option is set to false' do
let(:opts) {{ apply_password_parameter_check: false }}

it 'should be valid with a password param type' do
@manifest_hash = {
'parameters' => [
'name' => 'a password param',
'type' => 'password',
]}
package = create_package(@manifest_hash)
errors = ZendeskAppsSupport::Validations::Manifest.call(package, opts)

expect(errors.map(&:to_s).join()).not_to include("Password parameter type can no longer be used")
end
end

context 'when the apply_password_parameter_check option is set to true' do
let(:opts) {{ apply_password_parameter_check: true }}

it 'should be valid with a password param type' do
@manifest_hash = {
'parameters' => [
'name' => 'a password param',
'type' => 'password',
]}
package = create_package(@manifest_hash)
errors = ZendeskAppsSupport::Validations::Manifest.call(package, opts)

expect(errors.map(&:to_s).join()).to include("Password parameter type can no longer be used")
end
end
end

0 comments on commit 989c20c

Please sign in to comment.