Skip to content

Commit 1fc8a24

Browse files
aishkanaishkan
committed
Add validation for scopes attribute for secure param
1 parent cd649b1 commit 1fc8a24

File tree

3 files changed

+161
-9
lines changed

3 files changed

+161
-9
lines changed

‎lib/zendesk_apps_support/manifest/parameter.rb‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ module ZendeskAppsSupport
44
class Manifest
55
class Parameter
66
TYPES = %w[text password checkbox url number multiline hidden oauth].freeze
7-
ATTRIBUTES = %i[name type required secure default].freeze
7+
ATTRIBUTES = %i[name type required secure default scopes].freeze
88
attr_reader(*ATTRIBUTES)
99
def default?
1010
@has_default
@@ -17,6 +17,7 @@ def initialize(p)
1717
@secure = p['secure'] || false
1818
@has_default = p.key? 'default'
1919
@default = p['default'] if @has_default
20+
@scopes = p['scopes']
2021
end
2122
end
2223
end

‎lib/zendesk_apps_support/validations/manifest.rb‎

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,10 @@ module Manifest
1111
OAUTH_REQUIRED_FIELDS = %w[client_id client_secret authorize_uri access_token_uri].freeze
1212
PARAMETER_TYPES = ZendeskAppsSupport::Manifest::Parameter::TYPES
1313
OAUTH_MANIFEST_LINK = 'https://developer.zendesk.com/apps/docs/developer-guide/manifest#oauth'
14+
SECURE_PARAM_SCOPES = %w[header body url_host url_path jwt_secret_key jwt_claim basic_auth_username basic_auth_password].freeze
1415

1516
class << self
16-
def call(package, error_on_password_parameter: false)
17+
def call(package, error_on_password_parameter: false, validate_scopes_for_secure_parameter: false)
1718
unless package.has_file?('manifest.json')
1819
nested_manifest = package.files.find { |file| file =~ %r{\A[^/]+?/manifest\.json\Z} }
1920
if nested_manifest
@@ -24,7 +25,7 @@ def call(package, error_on_password_parameter: false)
2425

2526
package.warnings << password_parameter_warning if !error_on_password_parameter && password_param_present?(package.manifest)
2627

27-
collate_manifest_errors(package, error_on_password_parameter)
28+
collate_manifest_errors(package, error_on_password_parameter, validate_scopes_for_secure_parameter)
2829
rescue JSON::ParserError => e
2930
return [ValidationError.new(:manifest_not_json, errors: e)]
3031
rescue ZendeskAppsSupport::Manifest::OverrideError => e
@@ -37,7 +38,7 @@ def password_param_present?(manifest)
3738
manifest.parameters.any? { |p| p.type == 'password' }
3839
end
3940

40-
def collate_manifest_errors(package, error_on_password_parameter)
41+
def collate_manifest_errors(package, error_on_password_parameter, validate_scopes_for_secure_parameter)
4142
manifest = package.manifest
4243

4344
errors = [
@@ -46,7 +47,7 @@ def collate_manifest_errors(package, error_on_password_parameter)
4647
oauth_error(manifest),
4748
default_locale_error(manifest, package),
4849
validate_urls(manifest),
49-
validate_parameters(manifest),
50+
validate_parameters(manifest, validate_scopes_for_secure_parameter),
5051
if manifest.requirements_only? || manifest.marketing_only?
5152
[ ban_location(manifest),
5253
ban_framework_version(manifest) ]
@@ -87,18 +88,23 @@ def validate_urls(manifest)
8788
errors
8889
end
8990

90-
def validate_parameters(manifest)
91+
def validate_parameters(manifest, validate_scopes_for_secure_parameter)
9192
if manifest.marketing_only?
9293
marketing_only_errors(manifest)
9394
else
94-
[
95+
errors = [
9596
parameters_error(manifest),
9697
invalid_hidden_parameter_error(manifest),
9798
invalid_type_error(manifest),
9899
too_many_oauth_parameters(manifest),
99100
oauth_cannot_be_secure(manifest),
100-
name_as_parameter_name_error(manifest)
101+
name_as_parameter_name_error(manifest),
102+
if validate_scopes_for_secure_parameter
103+
[ invalid_secure_param_scopes_error(manifest) ]
104+
end
101105
]
106+
107+
errors.flatten
102108
end
103109
end
104110

@@ -110,6 +116,33 @@ def oauth_cannot_be_secure(manifest)
110116
end
111117
end
112118

119+
def invalid_secure_param_scopes_error(manifest)
120+
errors = []
121+
122+
manifest.parameters.each do |parameter|
123+
next if parameter.scopes.nil?
124+
125+
unless parameter.secure
126+
errors << ValidationError.new(:scope_requires_secure_parameter)
127+
next
128+
end
129+
130+
unless parameter.scopes.any?
131+
errors << ValidationError.new(:scopes_cannot_be_empty)
132+
next
133+
end
134+
135+
parameter.scopes.each do |scope|
136+
unless SECURE_PARAM_SCOPES.include?(scope)
137+
errors << ValidationError.new(:invalid_secure_parameter_scopes,
138+
invalid_scope: scope)
139+
end
140+
end
141+
end
142+
143+
errors
144+
end
145+
113146
def marketing_only_errors(manifest)
114147
[
115148
ban_parameters(manifest),

‎spec/validations/manifest_spec.rb‎

Lines changed: 119 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ def create_package(parameter_hash)
2424

2525
RSpec::Matchers.define :have_error do |error|
2626
match do |package|
27-
errors = ZendeskAppsSupport::Validations::Manifest.call(package)
27+
errors = ZendeskAppsSupport::Validations::Manifest.call(package, validate_scopes_for_secure_parameter: true)
2828
errors.map!(&:to_s) unless error.is_a? Symbol
2929
@actual = errors.compact
3030

@@ -864,4 +864,122 @@ def create_package(parameter_hash)
864864
end
865865
end
866866
end
867+
868+
context 'scope parameter validations' do
869+
context 'when validate_scopes_for_secure_parameter is false' do
870+
it 'should not validate scopes even if parameter has invalid scopes' do
871+
@manifest_hash = {
872+
'parameters' => [
873+
{
874+
'name' => 'api_token',
875+
'type' => 'text',
876+
'secure' => false,
877+
'scopes' => ['header']
878+
}
879+
]
880+
}
881+
package = create_package(@manifest_hash)
882+
errors = ZendeskAppsSupport::Validations::Manifest.call(package, validate_scopes_for_secure_parameter: false)
883+
expect(errors.find { |e| e.key == :scope_requires_secure_parameter }).to be_nil
884+
end
885+
end
886+
887+
context 'when validate_scopes_for_secure_parameter is true' do
888+
context 'when a parameter has scopes but is not secure' do
889+
it 'should have an error requiring secure parameter for scopes' do
890+
@manifest_hash = {
891+
'parameters' => [
892+
{
893+
'name' => 'api_token',
894+
'type' => 'text',
895+
'secure' => false,
896+
'scopes' => ['header']
897+
}
898+
]
899+
}
900+
package = create_package(@manifest_hash)
901+
errors = ZendeskAppsSupport::Validations::Manifest.call(package, validate_scopes_for_secure_parameter: true)
902+
expect(errors.find { |e| e.key == :scope_requires_secure_parameter }).not_to be_nil
903+
end
904+
end
905+
end
906+
907+
context 'when a parameter has scopes but is not secure' do
908+
it 'should have an error requiring secure parameter for scopes' do
909+
@manifest_hash = {
910+
'parameters' => [
911+
{
912+
'name' => 'api_token',
913+
'type' => 'text',
914+
'secure' => false,
915+
'scopes' => ['header']
916+
}
917+
]
918+
}
919+
expect(create_package(@manifest_hash)).to have_error(:scope_requires_secure_parameter)
920+
end
921+
end
922+
923+
context 'when a parameter has scopes and is secure' do
924+
it 'should not have an error for valid scope values' do
925+
@manifest_hash = {
926+
'parameters' => [
927+
{
928+
'name' => 'api_token',
929+
'type' => 'text',
930+
'secure' => true,
931+
'scopes' => ['header', 'body']
932+
}
933+
]
934+
}
935+
expect(create_package(@manifest_hash)).not_to have_error(:scope_requires_secure_parameter)
936+
expect(create_package(@manifest_hash)).not_to have_error(:invalid_secure_parameter_scopes)
937+
end
938+
939+
it 'should have an error for invalid scope values' do
940+
@manifest_hash = {
941+
'parameters' => [
942+
{
943+
'name' => 'api_token',
944+
'type' => 'text',
945+
'secure' => true,
946+
'scopes' => ['invalid_scope']
947+
}
948+
]
949+
}
950+
expect(create_package(@manifest_hash)).to have_error(:invalid_secure_parameter_scopes)
951+
end
952+
953+
it 'should have an error when scopes are empty' do
954+
@manifest_hash = {
955+
'parameters' => [
956+
{
957+
'name' => 'api_token',
958+
'type' => 'text',
959+
'secure' => true,
960+
'scopes' => []
961+
}
962+
]
963+
}
964+
expect(create_package(@manifest_hash)).to have_error(:scopes_cannot_be_empty)
965+
end
966+
967+
end
968+
969+
context 'when a parameter has no scopes' do
970+
it 'should not have any scope-related errors' do
971+
@manifest_hash = {
972+
'parameters' => [
973+
{
974+
'name' => 'api_token',
975+
'type' => 'text',
976+
'secure' => true
977+
}
978+
]
979+
}
980+
expect(create_package(@manifest_hash)).not_to have_error(:scope_requires_secure_parameter)
981+
expect(create_package(@manifest_hash)).not_to have_error(:invalid_secure_parameter_scopes)
982+
end
983+
end
984+
end
867985
end

0 commit comments

Comments
 (0)