From 48b588508a228303657b2b8742d297e8885bae49 Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Fri, 17 May 2019 16:18:46 -0300 Subject: [PATCH 01/19] Allowing grant to be extended --- lib/mumukit/auth/grant.rb | 39 +++++++++++++++++++++++++++++--------- spec/mumukit/grant_spec.rb | 29 ++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/lib/mumukit/auth/grant.rb b/lib/mumukit/auth/grant.rb index 3342933..adf7618 100644 --- a/lib/mumukit/auth/grant.rb +++ b/lib/mumukit/auth/grant.rb @@ -30,17 +30,26 @@ def inspect end def self.parse(pattern) - case pattern - when '*' then - AllGrant.new - when '*/*' then - AllGrant.new - when /(.*)\/\*/ - FirstPartGrant.new($1) - else - SingleGrant.new(Slug.parse pattern) + grant_types.each do |type| + type.try_parse(pattern).try { |it| return it } end end + + def self.grant_types + custom_grant_types + [AllGrant, FirstPartGrant, SingleGrant] + end + + def self.add_custom_grant_type!(grant_type) + custom_grant_types << grant_type + end + + def self.remove_custom_grant_type!(grant_type) + custom_grant_types.delete(grant_type) + end + + def self.custom_grant_types + @custom_grant_types ||= [] + end end class AllGrant < Grant @@ -55,6 +64,10 @@ def to_s def to_mumukit_slug Mumukit::Auth::Slug.new '*', '*' end + + def self.try_parse(pattern) + new if ['*', '*/*'].include? pattern + end end class FirstPartGrant < Grant @@ -73,6 +86,10 @@ def to_s def to_mumukit_slug Mumukit::Auth::Slug.new @first, '*' end + + def self.try_parse(pattern) + new($1) if pattern =~ /(.*)\/\*/ + end end class SingleGrant < Grant @@ -92,5 +109,9 @@ def to_s def to_mumukit_slug @slug end + + def self.try_parse(pattern) + new(Slug.parse pattern) + end end end diff --git a/spec/mumukit/grant_spec.rb b/spec/mumukit/grant_spec.rb index ff43f2c..5d5900f 100644 --- a/spec/mumukit/grant_spec.rb +++ b/spec/mumukit/grant_spec.rb @@ -89,4 +89,33 @@ it { expect(Mumukit::Auth::Grant.parse('FOO/Bar').allows? 'foo/BAR').to be true } end + describe 'custom grant' do + class IncludesGrant < Mumukit::Auth::Grant + def allows?(resource_slug) + resource_slug.to_mumukit_slug.first.include? 'foo' + end + + def to_s + '{includes:foo}' + end + + def to_mumukit_slug + Mumukit::Auth::Slug.new '*', '*' + end + + def self.try_parse(pattern) + new if pattern =~ /\{includes\:foo\}\/\*/ + end + end + before(:all) { Mumukit::Auth::Grant.add_custom_grant_type! IncludesGrant } + after(:all) { Mumukit::Auth::Grant.remove_custom_grant_type! IncludesGrant } + + let(:grant) { Mumukit::Auth::Grant.parse('{includes:foo}/*') } + + it { expect(Mumukit::Auth::Grant.custom_grant_types).to eq [IncludesGrant]} + it { expect(grant.allows? 'foo/baz').to be true } + it { expect(grant.allows? 'foobar/baz').to be true } + it { expect(grant.allows? 'bar/baz').to be false } + + end end From 52a35f74f363b3846ec9c6205fc2a1922c6e5565 Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Fri, 17 May 2019 16:27:47 -0300 Subject: [PATCH 02/19] Extracting Base class --- lib/mumukit/auth/grant.rb | 55 +++++++++++++++++++------------------- spec/mumukit/grant_spec.rb | 2 +- 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/lib/mumukit/auth/grant.rb b/lib/mumukit/auth/grant.rb index adf7618..35bbec1 100644 --- a/lib/mumukit/auth/grant.rb +++ b/lib/mumukit/auth/grant.rb @@ -4,9 +4,30 @@ def to_mumukit_grant end end +module Mumukit::Auth::Grant + def self.parse(pattern) + grant_types.each do |type| + type.try_parse(pattern).try { |it| return it } + end + end + + def self.grant_types + custom_grant_types + [AllGrant, FirstPartGrant, SingleGrant] + end + + def self.add_custom_grant_type!(grant_type) + custom_grant_types << grant_type + end + + def self.remove_custom_grant_type!(grant_type) + custom_grant_types.delete(grant_type) + end + + def self.custom_grant_types + @custom_grant_types ||= [] + end -module Mumukit::Auth - class Grant + class Base def as_json(options={}) to_s end @@ -28,31 +49,9 @@ def hash def inspect "" end - - def self.parse(pattern) - grant_types.each do |type| - type.try_parse(pattern).try { |it| return it } - end - end - - def self.grant_types - custom_grant_types + [AllGrant, FirstPartGrant, SingleGrant] - end - - def self.add_custom_grant_type!(grant_type) - custom_grant_types << grant_type - end - - def self.remove_custom_grant_type!(grant_type) - custom_grant_types.delete(grant_type) - end - - def self.custom_grant_types - @custom_grant_types ||= [] - end end - class AllGrant < Grant + class AllGrant < Base def allows?(_resource_slug) true end @@ -70,7 +69,7 @@ def self.try_parse(pattern) end end - class FirstPartGrant < Grant + class FirstPartGrant < Base def initialize(first) @first = first.downcase end @@ -92,7 +91,7 @@ def self.try_parse(pattern) end end - class SingleGrant < Grant + class SingleGrant < Base def initialize(slug) @slug = slug.normalize end @@ -111,7 +110,7 @@ def to_mumukit_slug end def self.try_parse(pattern) - new(Slug.parse pattern) + new(Mumukit::Auth::Slug.parse pattern) end end end diff --git a/spec/mumukit/grant_spec.rb b/spec/mumukit/grant_spec.rb index 5d5900f..5449fc7 100644 --- a/spec/mumukit/grant_spec.rb +++ b/spec/mumukit/grant_spec.rb @@ -90,7 +90,7 @@ end describe 'custom grant' do - class IncludesGrant < Mumukit::Auth::Grant + class IncludesGrant < Mumukit::Auth::Grant::Base def allows?(resource_slug) resource_slug.to_mumukit_slug.first.include? 'foo' end From 41090c8e1a721f2fe366ce52a786405c31b6f9d1 Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Fri, 17 May 2019 16:33:17 -0300 Subject: [PATCH 03/19] Adding some docs --- README.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/README.md b/README.md index becc342..7d43ef7 100644 --- a/README.md +++ b/README.md @@ -103,6 +103,17 @@ a_grant.to_s "baz/bar".to_mumukit_grant.allows? Mumukit::Auth::Slug.join('foo') # false ``` +#### Defining custom Grants + +Grants can be extend, by inheriting from `Mumukit::Auth::Grant::Base`, and defining the following method: + +* `#allows?(resource_slug)` +* `#to_s` +* `#to_mumukit_slug` +* `.try_parse(pattern)` + +New grant types must be registered using: `Mumukit::Auth::Grant.add_custom_grant_type!` + ### Roles ```ruby From 2af1e506808a7359bab49b7533befdb7a6ac8b7a Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Mon, 20 May 2019 20:02:55 -0300 Subject: [PATCH 04/19] Introducing includes? --- lib/mumukit/auth/grant.rb | 20 ++++++++++++++++++++ lib/mumukit/auth/scope.rb | 4 ++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/mumukit/auth/grant.rb b/lib/mumukit/auth/grant.rb index 35bbec1..97e05c4 100644 --- a/lib/mumukit/auth/grant.rb +++ b/lib/mumukit/auth/grant.rb @@ -56,6 +56,10 @@ def allows?(_resource_slug) true end + def includes?(_) + true + end + def to_s '*' end @@ -70,6 +74,8 @@ def self.try_parse(pattern) end class FirstPartGrant < Base + attr_accessor :first + def initialize(first) @first = first.downcase end @@ -86,12 +92,22 @@ def to_mumukit_slug Mumukit::Auth::Slug.new @first, '*' end + def includes?(other) + case other + when FirstPartGrant then other.first == first + when SingleGrant then other.slug.first == first + else false + end + end + def self.try_parse(pattern) new($1) if pattern =~ /(.*)\/\*/ end end class SingleGrant < Base + attr_accessor :slug + def initialize(slug) @slug = slug.normalize end @@ -101,6 +117,10 @@ def allows?(resource_slug) resource_slug.match_first(@slug.first) && resource_slug.match_second(@slug.second) end + def includes?(other) + other.is_a?(SingleGrant) && other.slug == slug + end + def to_s @slug.to_s end diff --git a/lib/mumukit/auth/scope.rb b/lib/mumukit/auth/scope.rb index dd920a1..065ded0 100644 --- a/lib/mumukit/auth/scope.rb +++ b/lib/mumukit/auth/scope.rb @@ -68,11 +68,11 @@ def push_and_compact!(grant) end def remove_narrower_grants!(grant) - grants.reject! { |it| grant.allows? it } + grants.reject! { |it| grant.includes? it } end def has_broader_grant?(grant) - grants.any? { |it| it.allows? grant } + grants.any? { |it| it.includes? grant } end end end From 100c45bc9b92203b7e651b1bb7b743fb156897a6 Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Tue, 21 May 2019 11:22:54 -0300 Subject: [PATCH 05/19] Introducing granted_organizations --- lib/mumukit/auth/grant.rb | 12 ++++++++++++ lib/mumukit/auth/permissions.rb | 2 +- spec/mumukit/permissions_spec.rb | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/mumukit/auth/grant.rb b/lib/mumukit/auth/grant.rb index 97e05c4..2e6b04d 100644 --- a/lib/mumukit/auth/grant.rb +++ b/lib/mumukit/auth/grant.rb @@ -36,6 +36,10 @@ def to_mumukit_grant self end + def granted_organizations + [] + end + def ==(other) other.class == self.class && to_s == other.to_s end @@ -80,6 +84,10 @@ def initialize(first) @first = first.downcase end + def granted_organizations + [first] + end + def allows?(resource_slug) resource_slug.to_mumukit_slug.normalize!.match_first @first end @@ -112,6 +120,10 @@ def initialize(slug) @slug = slug.normalize end + def granted_organizations + [slug.first] + end + def allows?(resource_slug) resource_slug = resource_slug.to_mumukit_slug.normalize! resource_slug.match_first(@slug.first) && resource_slug.match_second(@slug.second) diff --git a/lib/mumukit/auth/permissions.rb b/lib/mumukit/auth/permissions.rb index 3dbe006..5f80fd5 100644 --- a/lib/mumukit/auth/permissions.rb +++ b/lib/mumukit/auth/permissions.rb @@ -42,7 +42,7 @@ def student_granted_organizations end def granted_organizations_for(role) - scope_for(role)&.grants&.map { |grant| grant.to_mumukit_slug.organization }.to_set + scope_for(role)&.grants&.flat_map { |grant| grant.granted_organizations }.to_set end def add_permission!(role, *grants) diff --git a/spec/mumukit/permissions_spec.rb b/spec/mumukit/permissions_spec.rb index d851bf5..dca3193 100644 --- a/spec/mumukit/permissions_spec.rb +++ b/spec/mumukit/permissions_spec.rb @@ -208,7 +208,7 @@ def parse_permissions(hash) end context 'when all grant present organizations' do let(:permissions) { parse_permissions student: 'pdep/*:*' } - it { expect(permissions.student_granted_organizations.size).to eq 1 } + it { expect(permissions.student_granted_organizations.size).to eq 0 } end context 'when one organization appears twice' do let(:permissions) { parse_permissions student: 'pdep/*:pdep/*' } From 7990f822618c012a6d67754f678e826950910c15 Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Tue, 21 May 2019 11:29:36 -0300 Subject: [PATCH 06/19] Double dispatching allow --- lib/mumukit/auth/grant.rb | 4 ++++ lib/mumukit/auth/scope.rb | 2 +- lib/mumukit/auth/slug.rb | 8 ++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/mumukit/auth/grant.rb b/lib/mumukit/auth/grant.rb index 2e6b04d..8323802 100644 --- a/lib/mumukit/auth/grant.rb +++ b/lib/mumukit/auth/grant.rb @@ -53,6 +53,10 @@ def hash def inspect "" end + + def allowed_by?(grant) + grant.includes? self + end end class AllGrant < Base diff --git a/lib/mumukit/auth/scope.rb b/lib/mumukit/auth/scope.rb index 065ded0..f2f11ca 100644 --- a/lib/mumukit/auth/scope.rb +++ b/lib/mumukit/auth/scope.rb @@ -8,7 +8,7 @@ def initialize(grants=[]) end def allows?(resource_slug) - any_grant? { |grant| grant.allows? resource_slug } + any_grant? { |grant| resource_slug.allowed_by? grant } end def add_grant!(*grants) diff --git a/lib/mumukit/auth/slug.rb b/lib/mumukit/auth/slug.rb index 5caa5ad..a192afe 100644 --- a/lib/mumukit/auth/slug.rb +++ b/lib/mumukit/auth/slug.rb @@ -2,6 +2,10 @@ class String def to_mumukit_slug Mumukit::Auth::Slug.parse self end + + def allowed_by?(grant) + to_mumukit_slug.allowed_by?(grant) + end end module Mumukit::Auth @@ -68,6 +72,10 @@ def to_mumukit_slug self end + def allowed_by?(grant) + grant.allows? self + end + def self.from_options(hash) first = hash[:first] || hash[:organization] second = hash[:second] || hash[:repository] || hash[:course] || hash[:content] From a7d32f85c09b80069988a1c46fb792bc8c80b30f Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Tue, 21 May 2019 11:33:20 -0300 Subject: [PATCH 07/19] Removing to_mumukit_slug --- lib/mumukit/auth/grant.rb | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/lib/mumukit/auth/grant.rb b/lib/mumukit/auth/grant.rb index 8323802..6296588 100644 --- a/lib/mumukit/auth/grant.rb +++ b/lib/mumukit/auth/grant.rb @@ -72,10 +72,6 @@ def to_s '*' end - def to_mumukit_slug - Mumukit::Auth::Slug.new '*', '*' - end - def self.try_parse(pattern) new if ['*', '*/*'].include? pattern end @@ -100,10 +96,6 @@ def to_s "#{@first}/*" end - def to_mumukit_slug - Mumukit::Auth::Slug.new @first, '*' - end - def includes?(other) case other when FirstPartGrant then other.first == first @@ -141,10 +133,6 @@ def to_s @slug.to_s end - def to_mumukit_slug - @slug - end - def self.try_parse(pattern) new(Mumukit::Auth::Slug.parse pattern) end From 3dc0ea54fcbb97b4a427d3b72791ef23f6bddaca Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Tue, 21 May 2019 11:50:51 -0300 Subject: [PATCH 08/19] Adding docs and renaming authorized_by? --- lib/mumukit/auth/grant.rb | 20 +++++++++++++++++++- lib/mumukit/auth/permissions.rb | 8 ++++---- lib/mumukit/auth/role.rb | 17 +++++++++++------ lib/mumukit/auth/scope.rb | 4 ++-- lib/mumukit/auth/slug.rb | 16 +++++++++++++--- spec/mumukit/grant_spec.rb | 4 ++-- 6 files changed, 51 insertions(+), 18 deletions(-) diff --git a/lib/mumukit/auth/grant.rb b/lib/mumukit/auth/grant.rb index 6296588..4bb6dc4 100644 --- a/lib/mumukit/auth/grant.rb +++ b/lib/mumukit/auth/grant.rb @@ -54,9 +54,27 @@ def inspect "" end - def allowed_by?(grant) + # Tells wether the given grant + # is authorized by this grant + # + # This method exist in order to implement double dispatching + # for both grant and slugs authorization + # + # See: + # * `Mumukit::Auth::Slug#authorized_by?` + # * `Mumukit::Auth::Grant::Base#allows? + # * `Mumukit::Auth::Grant::Base#includes?` + def authorized_by?(grant) grant.includes? self end + + # tells whether the given slug is allowed by + # this grant + required :allows? + + # tells whether the given grant is included in - that is, is not broader than - + # this grant + required :includes? end class AllGrant < Base diff --git a/lib/mumukit/auth/permissions.rb b/lib/mumukit/auth/permissions.rb index 5f80fd5..9503b40 100644 --- a/lib/mumukit/auth/permissions.rb +++ b/lib/mumukit/auth/permissions.rb @@ -12,12 +12,12 @@ def initialize(scopes={}) @scopes = scopes.with_indifferent_access end - def has_permission?(role, resource_slug) - Mumukit::Auth::Role.parse(role).allows?(resource_slug, self) + def has_permission?(role, authorizable) + Mumukit::Auth::Role.parse(role).allows?(authorizable, self) end - def role_allows?(role, resource_slug) - scope_for(role).allows?(resource_slug) + def role_allows?(role, authorizable) + scope_for(role).allows?(authorizable) end def has_role?(role) diff --git a/lib/mumukit/auth/role.rb b/lib/mumukit/auth/role.rb index f13380d..bfbb352 100644 --- a/lib/mumukit/auth/role.rb +++ b/lib/mumukit/auth/role.rb @@ -4,13 +4,18 @@ def initialize(symbol) @symbol=symbol end - def allows?(resource_slug, permissions) - permissions.role_allows?(to_sym, resource_slug) || - parent_allows?(resource_slug, permissions) + # Tells wether the given authorizable object + # can be authorized using the given permissions + # by this role or its parent role + # + # This definition is recursive, thus traversing the whole ancenstry chain + def allows?(authorizable, permissions) + permissions.role_allows?(to_sym, authorizable) || + parent_allows?(authorizable, permissions) end - def parent_allows?(resource_slug, permissions) - parent.allows?(resource_slug, permissions) + def parent_allows?(authorizable, permissions) + parent.allows?(authorizable, permissions) end def to_sym @@ -57,4 +62,4 @@ def parent_allows?(*) end end end -end \ No newline at end of file +end diff --git a/lib/mumukit/auth/scope.rb b/lib/mumukit/auth/scope.rb index f2f11ca..a042231 100644 --- a/lib/mumukit/auth/scope.rb +++ b/lib/mumukit/auth/scope.rb @@ -7,8 +7,8 @@ def initialize(grants=[]) add_grant! *grants end - def allows?(resource_slug) - any_grant? { |grant| resource_slug.allowed_by? grant } + def allows?(authorizable) + any_grant? { |grant| authorizable.authorized_by? grant } end def add_grant!(*grants) diff --git a/lib/mumukit/auth/slug.rb b/lib/mumukit/auth/slug.rb index a192afe..d18735f 100644 --- a/lib/mumukit/auth/slug.rb +++ b/lib/mumukit/auth/slug.rb @@ -3,8 +3,8 @@ def to_mumukit_slug Mumukit::Auth::Slug.parse self end - def allowed_by?(grant) - to_mumukit_slug.allowed_by?(grant) + def authorized_by?(grant) + to_mumukit_slug.authorized_by?(grant) end end @@ -72,7 +72,17 @@ def to_mumukit_slug self end - def allowed_by?(grant) + # Tells wether the given grant + # is authorized by this slug + # + # This method exist in order to implement double dispatching + # for both grant and slugs authorization + # + # See: + # * `Mumukit::Auth::Grant::Base#authorized_by?` + # * `Mumukit::Auth::Grant::Base#allows? + # * `Mumukit::Auth::Grant::Base#includes?` + def authorized_by?(grant) grant.allows? self end diff --git a/spec/mumukit/grant_spec.rb b/spec/mumukit/grant_spec.rb index 5449fc7..82ddf01 100644 --- a/spec/mumukit/grant_spec.rb +++ b/spec/mumukit/grant_spec.rb @@ -91,8 +91,8 @@ describe 'custom grant' do class IncludesGrant < Mumukit::Auth::Grant::Base - def allows?(resource_slug) - resource_slug.to_mumukit_slug.first.include? 'foo' + def allows?(authorizable) + authorizable.to_mumukit_slug.first.include? 'foo' end def to_s From 9565e4c40dc94f26e95f1878edf9a5b246ac0fc5 Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Tue, 21 May 2019 12:04:26 -0300 Subject: [PATCH 09/19] Making includes? work with grant-likes to --- lib/mumukit/auth/grant.rb | 32 +++++++++++++++++--------------- spec/mumukit/grant_spec.rb | 10 ++++++++++ 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/lib/mumukit/auth/grant.rb b/lib/mumukit/auth/grant.rb index 4bb6dc4..c5ccc20 100644 --- a/lib/mumukit/auth/grant.rb +++ b/lib/mumukit/auth/grant.rb @@ -68,17 +68,17 @@ def authorized_by?(grant) grant.includes? self end - # tells whether the given slug is allowed by + # tells whether the given slug-like object is allowed by # this grant required :allows? - # tells whether the given grant is included in - that is, is not broader than - - # this grant + # tells whether the given grant-like object is included + # in - that is, is not broader than - this grant required :includes? end class AllGrant < Base - def allows?(_resource_slug) + def allows?(_slug_like) true end @@ -106,18 +106,19 @@ def granted_organizations [first] end - def allows?(resource_slug) - resource_slug.to_mumukit_slug.normalize!.match_first @first + def allows?(slug_like) + slug_like.to_mumukit_slug.normalize!.match_first @first end def to_s "#{@first}/*" end - def includes?(other) - case other - when FirstPartGrant then other.first == first - when SingleGrant then other.slug.first == first + def includes?(grant_like) + grant = grant_like.to_mumukit_grant + case grant + when FirstPartGrant then grant.first == first + when SingleGrant then grant.slug.first == first else false end end @@ -138,13 +139,14 @@ def granted_organizations [slug.first] end - def allows?(resource_slug) - resource_slug = resource_slug.to_mumukit_slug.normalize! - resource_slug.match_first(@slug.first) && resource_slug.match_second(@slug.second) + def allows?(slug_like) + slug = slug_like.to_mumukit_slug.normalize! + slug.match_first(@slug.first) && slug.match_second(@slug.second) end - def includes?(other) - other.is_a?(SingleGrant) && other.slug == slug + def includes?(grant_like) + grant = grant_like.to_mumukit_grant + grant.is_a?(SingleGrant) && grant.slug == slug end def to_s diff --git a/spec/mumukit/grant_spec.rb b/spec/mumukit/grant_spec.rb index 82ddf01..19d49c7 100644 --- a/spec/mumukit/grant_spec.rb +++ b/spec/mumukit/grant_spec.rb @@ -89,6 +89,16 @@ it { expect(Mumukit::Auth::Grant.parse('FOO/Bar').allows? 'foo/BAR').to be true } end + describe 'includes?' do + it { expect('foo/bar'.to_mumukit_grant.includes? 'foo/bar').to be true } + it { expect('foo/*'.to_mumukit_grant.includes? 'foo/*').to be true } + it { expect('foo/*'.to_mumukit_grant.includes? 'foo/bar').to be true } + it { expect('foo/bar'.to_mumukit_grant.includes? 'foo/*').to be false } + it { expect('*'.to_mumukit_grant.includes? 'foo/bar').to be true } + it { expect('foo/bar'.to_mumukit_grant.includes? '*').to be false } + xit { expect('foo/bar'.to_mumukit_grant.includes? '_/_').to raise_error('invalid grant') } + end + describe 'custom grant' do class IncludesGrant < Mumukit::Auth::Grant::Base def allows?(authorizable) From d3154ce46577b01b57d6bd2fab9c9d34a660f091 Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Tue, 21 May 2019 12:19:22 -0300 Subject: [PATCH 10/19] Implementing includes? by default --- lib/mumukit/auth/grant.rb | 11 +++++------ spec/mumukit/grant_spec.rb | 22 +++++++++++++--------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/lib/mumukit/auth/grant.rb b/lib/mumukit/auth/grant.rb index c5ccc20..22dcdc2 100644 --- a/lib/mumukit/auth/grant.rb +++ b/lib/mumukit/auth/grant.rb @@ -74,7 +74,11 @@ def authorized_by?(grant) # tells whether the given grant-like object is included # in - that is, is not broader than - this grant - required :includes? + # + # :warning: Custom grants **should not** override this method + def includes?(grant_like) + self == grant_like.to_mumukit_grant + end end class AllGrant < Base @@ -144,11 +148,6 @@ def allows?(slug_like) slug.match_first(@slug.first) && slug.match_second(@slug.second) end - def includes?(grant_like) - grant = grant_like.to_mumukit_grant - grant.is_a?(SingleGrant) && grant.slug == slug - end - def to_s @slug.to_s end diff --git a/spec/mumukit/grant_spec.rb b/spec/mumukit/grant_spec.rb index 19d49c7..4561dbe 100644 --- a/spec/mumukit/grant_spec.rb +++ b/spec/mumukit/grant_spec.rb @@ -87,6 +87,9 @@ it { expect(grant.allows? 'FOO/BAR').to be true } it { expect(Mumukit::Auth::Grant.parse('FOO/Bar').allows? 'foo/BAR').to be true } + + xit { expect('foo/bar'.to_mumukit_grant.includes? '*/*').to raise_error('invalid slug') } + xit { expect('foo/bar'.to_mumukit_grant.includes? '*').to raise_error('invalid slug') } end describe 'includes?' do @@ -101,31 +104,32 @@ describe 'custom grant' do class IncludesGrant < Mumukit::Auth::Grant::Base - def allows?(authorizable) - authorizable.to_mumukit_slug.first.include? 'foo' + def allows?(slug_like) + slug_like.to_mumukit_slug.first.include? 'foo' end def to_s - '{includes:foo}' - end - - def to_mumukit_slug - Mumukit::Auth::Slug.new '*', '*' + '{includesFoo}' end def self.try_parse(pattern) - new if pattern =~ /\{includes\:foo\}\/\*/ + new if pattern =~ /\{includesFoo\}/ end end before(:all) { Mumukit::Auth::Grant.add_custom_grant_type! IncludesGrant } after(:all) { Mumukit::Auth::Grant.remove_custom_grant_type! IncludesGrant } - let(:grant) { Mumukit::Auth::Grant.parse('{includes:foo}/*') } + let(:grant) { Mumukit::Auth::Grant.parse('{includesFoo}') } it { expect(Mumukit::Auth::Grant.custom_grant_types).to eq [IncludesGrant]} it { expect(grant.allows? 'foo/baz').to be true } it { expect(grant.allows? 'foobar/baz').to be true } it { expect(grant.allows? 'bar/baz').to be false } + it { expect(grant.includes? 'foo/baz').to be false } + it { expect(grant.includes? 'foobar/baz').to be false } + it { expect(grant.includes? 'bar/baz').to be false } + + it { expect(grant.includes? '{includesFoo}').to be true } end end From 6d1d576785dc0511a535bc5ab36b344b4be85667 Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Tue, 21 May 2019 12:56:29 -0300 Subject: [PATCH 11/19] Renaming `authorizes?` --- README.md | 8 ++++---- lib/mumukit/auth/grant.rb | 8 ++++++++ lib/mumukit/auth/permissions.rb | 6 +++--- lib/mumukit/auth/role.rb | 12 ++++++------ lib/mumukit/auth/scope.rb | 4 ++-- spec/mumukit/scope_spec.rb | 20 ++++++++++---------- 6 files changed, 33 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 7d43ef7..1ac9ca6 100644 --- a/README.md +++ b/README.md @@ -107,10 +107,10 @@ a_grant.to_s Grants can be extend, by inheriting from `Mumukit::Auth::Grant::Base`, and defining the following method: -* `#allows?(resource_slug)` -* `#to_s` -* `#to_mumukit_slug` -* `.try_parse(pattern)` +* `#allows?(resource_slug)`: mandatory +* `#to_s`: mandatory +* `#granted_organizations`: optional +* `.try_parse(pattern)`: mandatory New grant types must be registered using: `Mumukit::Auth::Grant.add_custom_grant_type!` diff --git a/lib/mumukit/auth/grant.rb b/lib/mumukit/auth/grant.rb index 22dcdc2..70c56a8 100644 --- a/lib/mumukit/auth/grant.rb +++ b/lib/mumukit/auth/grant.rb @@ -36,6 +36,10 @@ def to_mumukit_grant self end + # returns the organizations that are explicitly + # granted by this grant + # + # Custom grants may override this method def granted_organizations [] end @@ -79,6 +83,10 @@ def authorized_by?(grant) def includes?(grant_like) self == grant_like.to_mumukit_grant end + + # Returns a canonical string representation of this grant + # Equivalent grant **must** have equivalent string representations + required :to_s end class AllGrant < Base diff --git a/lib/mumukit/auth/permissions.rb b/lib/mumukit/auth/permissions.rb index 9503b40..2fab420 100644 --- a/lib/mumukit/auth/permissions.rb +++ b/lib/mumukit/auth/permissions.rb @@ -13,11 +13,11 @@ def initialize(scopes={}) end def has_permission?(role, authorizable) - Mumukit::Auth::Role.parse(role).allows?(authorizable, self) + Mumukit::Auth::Role.parse(role).authorizes?(authorizable, self) end - def role_allows?(role, authorizable) - scope_for(role).allows?(authorizable) + def role_authorizes?(role, authorizable) + scope_for(role).authorizes?(authorizable) end def has_role?(role) diff --git a/lib/mumukit/auth/role.rb b/lib/mumukit/auth/role.rb index bfbb352..97e655a 100644 --- a/lib/mumukit/auth/role.rb +++ b/lib/mumukit/auth/role.rb @@ -9,13 +9,13 @@ def initialize(symbol) # by this role or its parent role # # This definition is recursive, thus traversing the whole ancenstry chain - def allows?(authorizable, permissions) - permissions.role_allows?(to_sym, authorizable) || - parent_allows?(authorizable, permissions) + def authorizes?(authorizable, permissions) + permissions.role_authorizes?(to_sym, authorizable) || + parent_authorizes?(authorizable, permissions) end - def parent_allows?(authorizable, permissions) - parent.allows?(authorizable, permissions) + def parent_authorizes?(authorizable, permissions) + parent.authorizes?(authorizable, permissions) end def to_sym @@ -57,7 +57,7 @@ class Moderator < Role class Owner < Role parent nil - def parent_allows?(*) + def parent_authorizes?(*) false end end diff --git a/lib/mumukit/auth/scope.rb b/lib/mumukit/auth/scope.rb index a042231..37d5d52 100644 --- a/lib/mumukit/auth/scope.rb +++ b/lib/mumukit/auth/scope.rb @@ -7,8 +7,8 @@ def initialize(grants=[]) add_grant! *grants end - def allows?(authorizable) - any_grant? { |grant| authorizable.authorized_by? grant } + def authorizes?(authorizable) + any_grant? { |grant| authorizable.authorized_by? grant } end def add_grant!(*grants) diff --git a/spec/mumukit/scope_spec.rb b/spec/mumukit/scope_spec.rb index 125ac19..add9bf9 100644 --- a/spec/mumukit/scope_spec.rb +++ b/spec/mumukit/scope_spec.rb @@ -4,25 +4,25 @@ describe 'one grant' do let(:scope) { Mumukit::Auth::Scope.parse('*') } - it { expect(scope.allows? 'foo/bar').to be true } + it { expect(scope.authorizes? 'foo/bar').to be true } end describe 'two scope' do let(:scope) { Mumukit::Auth::Scope.parse('foo/*:mumuki/*') } - it { expect(scope.allows? 'foo/bag').to be true } - it { expect(scope.allows? 'foo/baz').to be true } - it { expect(scope.allows? 'fooz/baz').to be false } - it { expect(scope.allows? 'xfoo/baz').to be false } - it { expect(scope.allows? 'mumuki/funcional').to be true } - it { expect(scope.allows? 'mumuki/logico').to be true } - it { expect(scope.allows? 'Mumuki/Logico').to be true } + it { expect(scope.authorizes? 'foo/bag').to be true } + it { expect(scope.authorizes? 'foo/baz').to be true } + it { expect(scope.authorizes? 'fooz/baz').to be false } + it { expect(scope.authorizes? 'xfoo/baz').to be false } + it { expect(scope.authorizes? 'mumuki/funcional').to be true } + it { expect(scope.authorizes? 'mumuki/logico').to be true } + it { expect(scope.authorizes? 'Mumuki/Logico').to be true } end describe 'grant none' do let(:scope) { Mumukit::Auth::Scope.parse('') } - it { expect(scope.allows? 'foo/bag').to be false } - it { expect(scope.allows? 'fooz/baz').to be false } + it { expect(scope.authorizes? 'foo/bag').to be false } + it { expect(scope.authorizes? 'fooz/baz').to be false } end end From 26ea92d68d6f0e9d3575d94f0610f83ad4d55eb3 Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Tue, 21 May 2019 13:20:20 -0300 Subject: [PATCH 12/19] Renaming methods to be consistent with type Ensuring string is not an authorizable object anymore --- README.md | 2 +- lib/mumukit/auth/permissions.rb | 34 +++++++++++++++++++++++++++----- lib/mumukit/auth/protection.rb | 6 +++--- lib/mumukit/auth/roles.rb | 2 +- lib/mumukit/auth/scope.rb | 5 +++++ lib/mumukit/auth/slug.rb | 4 ---- spec/mumukit/permissions_spec.rb | 10 +++++----- spec/mumukit/scope_spec.rb | 22 +++++++++++---------- 8 files changed, 56 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index 1ac9ca6..5674e98 100644 --- a/README.md +++ b/README.md @@ -147,7 +147,7 @@ some_permissions.remove_permission! :student, 'foo/bar' some_permissions.update_permission! :student, 'foo/*', 'foo/bar' # Checking permissions -some_permissions.has_permission? :student, 'foo/_' +some_permissions.authorizes? :student, 'foo/_' some_permissions.student? 'foo/_' # equivalent to previous line some_permissions.protect! :student, 'foo/_' # similar to previous samples, # but raises and exception instead diff --git a/lib/mumukit/auth/permissions.rb b/lib/mumukit/auth/permissions.rb index 2fab420..2b73147 100644 --- a/lib/mumukit/auth/permissions.rb +++ b/lib/mumukit/auth/permissions.rb @@ -12,10 +12,34 @@ def initialize(scopes={}) @scopes = scopes.with_indifferent_access end - def has_permission?(role, authorizable) + # Deprecated: use `allows` or `authorizes?` instead + def has_permission?(role, thing) + warn "Don't use has_permission?\n" + + "Use allows? if want to validate a slug-like object\n" + + "Use authorizes? if you want to validate an authorizable - grant-like or slug-like - object" + if thing.is_a?(Mumukit::Auth::Grant::Base) + warn "Using authorizes?" + authorizes?(role, thing) + else + warn "Using allows?" + allows?(role, thing) + end + end + + # tells wether this permissions + # authorize the given authorizable object for the given role, + # or any of its parent roles. + def authorizes?(role, authorizable) Mumukit::Auth::Role.parse(role).authorizes?(authorizable, self) end + # Similar to `authorizes?`, but specialized for slug-like objects + def allows?(role, slug_like) + authorizes? role, slug_like.to_mumukit_slug + end + + # tells wether this permissions + # authorize the given authorizable object for the specific given role def role_authorizes?(role, authorizable) scope_for(role).authorizes?(authorizable) end @@ -63,7 +87,7 @@ def update_permission!(role, old_grant, new_grant) end def delegate_to?(other) - other.scopes.all? { |role, scope| has_all_permissions?(role, scope) } + other.scopes.all? { |role, scope| authorizes_all?(role, scope) } end def grant_strings_for(role) @@ -99,7 +123,7 @@ def self.dump(permission) def assign_to?(other, previous) diff = previous.as_set ^ other.as_set - diff.all? { |role, grant| has_permission?(role, grant) } + diff.all? { |role, grant| authorizes?(role, grant) } end def protect_permissions_assignment!(other, previous) @@ -134,8 +158,8 @@ def to_h private - def has_all_permissions?(role, scope) - scope.grants.all? { |grant| has_permission? role, grant } + def authorizes_all?(role, scope) + scope.grants.all? { |grant| authorizes? role, grant } end end diff --git a/lib/mumukit/auth/protection.rb b/lib/mumukit/auth/protection.rb index 7e7fa27..9286e53 100644 --- a/lib/mumukit/auth/protection.rb +++ b/lib/mumukit/auth/protection.rb @@ -1,7 +1,7 @@ module Mumukit::Auth::Protection - def protect!(role, slug) + def protect!(role, slug_like) raise Mumukit::Auth::UnauthorizedAccessError, - "Unauthorized access to #{slug} as #{role}. Scope is `#{scope_for role}`" unless has_permission?(role, slug) + "Unauthorized access to #{slug_like} as #{role}. Scope is `#{scope_for role}`" unless allows?(role, slug_like) end def protect_delegation!(other) @@ -9,4 +9,4 @@ def protect_delegation!(other) raise Mumukit::Auth::UnauthorizedAccessError, "Unauthorized delegation to #{other.to_h}" unless delegate_to?(Mumukit::Auth::Permissions.parse(other.to_h)) end -end \ No newline at end of file +end diff --git a/lib/mumukit/auth/roles.rb b/lib/mumukit/auth/roles.rb index 3475333..9cfbba0 100644 --- a/lib/mumukit/auth/roles.rb +++ b/lib/mumukit/auth/roles.rb @@ -4,7 +4,7 @@ module Roles ROLES.each do |role| define_method "#{role}?" do |scope = Mumukit::Auth::Slug.any| - has_permission? role.to_sym, scope + allows? role.to_sym, scope end end end diff --git a/lib/mumukit/auth/scope.rb b/lib/mumukit/auth/scope.rb index 37d5d52..7bc5eaa 100644 --- a/lib/mumukit/auth/scope.rb +++ b/lib/mumukit/auth/scope.rb @@ -11,6 +11,11 @@ def authorizes?(authorizable) any_grant? { |grant| authorizable.authorized_by? grant } end + # Similar to `authorizes?`, but specialized for slug-like objects + def allows?(slug_like) + authorizes? slug_like.to_mumukit_slug + end + def add_grant!(*grants) grants.each { |grant| push_and_compact! grant } end diff --git a/lib/mumukit/auth/slug.rb b/lib/mumukit/auth/slug.rb index d18735f..2f9630b 100644 --- a/lib/mumukit/auth/slug.rb +++ b/lib/mumukit/auth/slug.rb @@ -2,10 +2,6 @@ class String def to_mumukit_slug Mumukit::Auth::Slug.parse self end - - def authorized_by?(grant) - to_mumukit_slug.authorized_by?(grant) - end end module Mumukit::Auth diff --git a/spec/mumukit/permissions_spec.rb b/spec/mumukit/permissions_spec.rb index dca3193..19d1b62 100644 --- a/spec/mumukit/permissions_spec.rb +++ b/spec/mumukit/permissions_spec.rb @@ -152,9 +152,9 @@ def parse_permissions(hash) it { expect(permissions.teacher? 'test/bar').to eq true } - it { expect(permissions.has_permission? :teacher, 'test/bar').to be true } - it { expect(permissions.has_permission? :teacher, 'Test/Bar').to be true } - it { expect(permissions.has_permission? :teacher, 'test/baz').to be false } + it { expect(permissions.allows? :teacher, 'test/bar').to be true } + it { expect(permissions.allows? :teacher, 'Test/Bar').to be true } + it { expect(permissions.allows? :teacher, 'test/baz').to be false } it { expect(permissions.as_json).to json_like(teacher: 'test/bar') } @@ -162,14 +162,14 @@ def parse_permissions(hash) before { permissions.add_permission! :teacher, 'test/*' } it { expect(permissions).to json_like teacher: 'test/*' } - it { expect(permissions.has_permission? :teacher, 'test/baz').to be true } + it { expect(permissions.allows? :teacher, 'test/baz').to be true } end context 'when added broader grant with upcase' do before { permissions.add_permission! :teacher, 'Test/*' } it { expect(permissions).to json_like teacher: 'test/*' } - it { expect(permissions.has_permission? :teacher, 'test/baz').to be true } + it { expect(permissions.allows? :teacher, 'test/baz').to be true } end end diff --git a/spec/mumukit/scope_spec.rb b/spec/mumukit/scope_spec.rb index add9bf9..8b59e75 100644 --- a/spec/mumukit/scope_spec.rb +++ b/spec/mumukit/scope_spec.rb @@ -4,25 +4,27 @@ describe 'one grant' do let(:scope) { Mumukit::Auth::Scope.parse('*') } - it { expect(scope.authorizes? 'foo/bar').to be true } + it { expect(scope.allows? 'foo/bar').to be true } end describe 'two scope' do let(:scope) { Mumukit::Auth::Scope.parse('foo/*:mumuki/*') } - it { expect(scope.authorizes? 'foo/bag').to be true } - it { expect(scope.authorizes? 'foo/baz').to be true } - it { expect(scope.authorizes? 'fooz/baz').to be false } - it { expect(scope.authorizes? 'xfoo/baz').to be false } - it { expect(scope.authorizes? 'mumuki/funcional').to be true } - it { expect(scope.authorizes? 'mumuki/logico').to be true } - it { expect(scope.authorizes? 'Mumuki/Logico').to be true } + it { expect(scope.allows? 'foo/bag').to be true } + it { expect(scope.allows? 'foo/baz').to be true } + it { expect(scope.allows? 'fooz/baz').to be false } + it { expect(scope.allows? 'xfoo/baz').to be false } + it { expect(scope.allows? 'mumuki/funcional').to be true } + it { expect(scope.allows? 'mumuki/logico').to be true } + it { expect(scope.allows? 'Mumuki/Logico').to be true } + + xit { expect(scope.allows? 'Mumuki/*').to raise_error('invalid slug') } end describe 'grant none' do let(:scope) { Mumukit::Auth::Scope.parse('') } - it { expect(scope.authorizes? 'foo/bag').to be false } - it { expect(scope.authorizes? 'fooz/baz').to be false } + it { expect(scope.allows? 'foo/bag').to be false } + it { expect(scope.allows? 'fooz/baz').to be false } end end From 5baa1e61e5dcacf1458f14e9b2b3a9d191bab310 Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Tue, 21 May 2019 13:49:11 -0300 Subject: [PATCH 13/19] Validating slug format --- lib/mumukit/auth/slug.rb | 7 +++++-- spec/mumukit/permissions_spec.rb | 17 ++++++++++------- spec/mumukit/scope_spec.rb | 2 +- spec/mumukit/slug_spec.rb | 14 ++++++++------ 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/lib/mumukit/auth/slug.rb b/lib/mumukit/auth/slug.rb index 2f9630b..18aa0cd 100644 --- a/lib/mumukit/auth/slug.rb +++ b/lib/mumukit/auth/slug.rb @@ -15,8 +15,11 @@ class Slug alias_method :content, :second def initialize(first, second) - raise 'slug first part must be non-nil' unless first - raise 'slug second part must be non-nil' unless second + raise 'Slug first part must be non-nil' unless first + raise 'Slug second part must be non-nil' unless second + + raise "Invalid first part format #{first}" unless first.match? /^[\w\-\.]+$/ + raise "Invalid second part format #{second}" unless second.match? /^[\w\-\.]+$/ @first = first @second = second diff --git a/spec/mumukit/permissions_spec.rb b/spec/mumukit/permissions_spec.rb index 19d1b62..2e06fea 100644 --- a/spec/mumukit/permissions_spec.rb +++ b/spec/mumukit/permissions_spec.rb @@ -109,8 +109,8 @@ def parse_permissions(hash) it { expect(permissions.owner?).to be true } it { expect(permissions.owner? 'test/student').to be true } it { expect(permissions.owner? 'test/_').to be true } - it { expect(permissions.owner? 'test/*').to be true } - it { expect(permissions.owner? '*/*').to be false } + it { expect { permissions.owner? 'test/*' }.to raise_error 'Invalid second part format *' } + it { expect { permissions.owner? '*/*' }.to raise_error 'Invalid first part format *' } it { expect(permissions.writer? 'test/student').to be true } it { expect(permissions.writer? 'test/_').to be true } @@ -177,7 +177,7 @@ def parse_permissions(hash) before { permissions.add_permission!(:student, 'test/*') } it { expect(permissions.has_role? :student).to eq true } - it { expect(permissions.student? 'test/*').to eq true } + it { expect(permissions.student? 'test/_').to eq true } context 'when added twice' do before { permissions.add_permission! :student, 'test/*' } @@ -219,23 +219,26 @@ def parse_permissions(hash) describe 'remove_permission!' do let(:permissions) { parse_permissions(student: 'foo/bar:test/*:foo/baz') } + it { expect { permissions.student? 'test/*' }.to raise_error 'Invalid second part format *' } + it { expect { permissions.student? '*' }.to raise_error 'Invalid slug: *. It must be in first/second format' } + context 'when permission is present' do before { permissions.remove_permission!(:student, 'test/*') } - it { expect(permissions.student? 'test/*').to eq false } + it { expect(permissions.student? 'test/_').to eq false } it { expect(permissions.student? 'foo/bar').to eq true } it { expect(permissions.student? 'foo/baz').to eq true } end context 'when scope is not present' do before { permissions.remove_permission!(:student, 'baz/*') } - it { expect(permissions.student? 'test/*').to eq true } + it { expect(permissions.student? 'test/_').to eq true } it { expect(permissions.student? 'foo/bar').to eq true } it { expect(permissions.student? 'foo/baz').to eq true } end context 'when role is not present' do before { permissions.remove_permission!(:teacher, 'baz/*') } - it { expect(permissions.student? 'test/*').to eq true } + it { expect(permissions.student? 'test/_').to eq true } it { expect(permissions.student? 'foo/bar').to eq true } it { expect(permissions.student? 'foo/baz').to eq true } end @@ -247,7 +250,7 @@ def parse_permissions(hash) headmaster: Mumukit::Auth::Scope.parse('foo/*'), owner: Mumukit::Auth::Scope.parse('test/*')) end - it { expect(permissions.student? 'test/*').to eq true } + it { expect(permissions.student? 'test/_').to eq true } it { expect(permissions.teacher? 'foo/bar').to eq true } it { expect(permissions.headmaster? 'foo/baz').to eq true } it { expect(permissions.headmaster? 'bar/baz').to eq false } diff --git a/spec/mumukit/scope_spec.rb b/spec/mumukit/scope_spec.rb index 8b59e75..48364f2 100644 --- a/spec/mumukit/scope_spec.rb +++ b/spec/mumukit/scope_spec.rb @@ -18,7 +18,7 @@ it { expect(scope.allows? 'mumuki/logico').to be true } it { expect(scope.allows? 'Mumuki/Logico').to be true } - xit { expect(scope.allows? 'Mumuki/*').to raise_error('invalid slug') } + it { expect { scope.allows? 'Mumuki/*' }.to raise_error('Invalid second part format *') } end describe 'grant none' do diff --git a/spec/mumukit/slug_spec.rb b/spec/mumukit/slug_spec.rb index 7c64e06..041657a 100644 --- a/spec/mumukit/slug_spec.rb +++ b/spec/mumukit/slug_spec.rb @@ -5,6 +5,8 @@ it { expect('foo/bar--baz'.to_mumukit_slug.normalize).to eql 'foo/bar-baz'.to_mumukit_slug } it { expect('ein Bär/in München'.to_mumukit_slug.normalize).to eql 'ein-bar/in-munchen'.to_mumukit_slug } + it { expect { 'foo/*'.to_mumukit_slug }.to raise_error('Invalid second part format *') } + it { expect('foo/baz'.to_mumukit_slug).to eq 'foo/baz'.to_mumukit_slug } it { expect('FOO/BAZ'.to_mumukit_slug).to eq 'foo/baz'.to_mumukit_slug } it { expect('Foo/Baz'.to_mumukit_slug).to eq 'FOO/BAZ'.to_mumukit_slug } @@ -15,12 +17,12 @@ it { expect('foo/baz'.to_mumukit_slug.rebase('bar')).to eq 'bar/baz'.to_mumukit_slug } - it { expect('foo/*'.to_mumukit_slug == 'foo/*'.to_mumukit_slug).to be true } - it { expect('foo/*'.to_mumukit_slug.eql? 'foo/*'.to_mumukit_slug).to be true } - it { expect('foo/*'.to_mumukit_slug.hash == 'foo/*'.to_mumukit_slug.hash).to be true } + it { expect('foo/_'.to_mumukit_slug == 'foo/_'.to_mumukit_slug).to be true } + it { expect('foo/_'.to_mumukit_slug.eql? 'foo/_'.to_mumukit_slug).to be true } + it { expect('foo/_'.to_mumukit_slug.hash == 'foo/_'.to_mumukit_slug.hash).to be true } - it { expect { Mumukit::Auth::Slug.new(nil, 'bar') }.to raise_error 'slug first part must be non-nil' } - it { expect { Mumukit::Auth::Slug.new('foo', nil) }.to raise_error 'slug second part must be non-nil' } + it { expect { Mumukit::Auth::Slug.new(nil, 'bar') }.to raise_error 'Slug first part must be non-nil' } + it { expect { Mumukit::Auth::Slug.new('foo', nil) }.to raise_error 'Slug second part must be non-nil' } it { expect(Mumukit::Auth::Slug.new('foo', 'bar').to_s).to eq 'foo/bar' } it { expect(Mumukit::Auth::Slug.parse('foo/bar').to_s).to eq 'foo/bar' } @@ -48,7 +50,7 @@ it { expect { Mumukit::Auth::Slug.validate_slug!('foo-bar/baz') }.to_not raise_error } it { expect { Mumukit::Auth::Slug.validate_slug!('FOO.baR/bAz') }.to_not raise_error } it { expect { Mumukit::Auth::Slug.validate_slug!('123/!@#') }.to_not raise_error } - + it { expect { Mumukit::Auth::Slug.validate_slug!('/') }.to raise_error } it { expect { Mumukit::Auth::Slug.validate_slug!('foo/') }.to raise_error } it { expect { Mumukit::Auth::Slug.validate_slug!('/bar') }.to raise_error } From bf387daebbbb628ed90e45ef8e33c2095d02eb7d Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Wed, 22 May 2019 11:58:00 -0300 Subject: [PATCH 14/19] Correcting slug regexp --- lib/mumukit/auth/slug.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mumukit/auth/slug.rb b/lib/mumukit/auth/slug.rb index 18aa0cd..449ffa4 100644 --- a/lib/mumukit/auth/slug.rb +++ b/lib/mumukit/auth/slug.rb @@ -18,8 +18,8 @@ def initialize(first, second) raise 'Slug first part must be non-nil' unless first raise 'Slug second part must be non-nil' unless second - raise "Invalid first part format #{first}" unless first.match? /^[\w\-\.]+$/ - raise "Invalid second part format #{second}" unless second.match? /^[\w\-\.]+$/ + raise "Invalid first part format #{first}" unless first.match? /^[[[:alnum:]]\_\ \-\.]+$/ + raise "Invalid second part format #{second}" unless second.match? /^[[[:alnum:]]\_\ \-\.]+$/ @first = first @second = second From 38279988838595e4c832555eaa318fa0e7a46c87 Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Wed, 22 May 2019 12:10:15 -0300 Subject: [PATCH 15/19] Extracting constant --- lib/mumukit/auth/slug.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/mumukit/auth/slug.rb b/lib/mumukit/auth/slug.rb index 449ffa4..0163ed6 100644 --- a/lib/mumukit/auth/slug.rb +++ b/lib/mumukit/auth/slug.rb @@ -6,6 +6,8 @@ def to_mumukit_slug module Mumukit::Auth class Slug + SLUG_REGEXP = /^[[[:alnum:]]\_\ \-\.]+$/ + attr_accessor :first, :second alias_method :organization, :first @@ -18,8 +20,8 @@ def initialize(first, second) raise 'Slug first part must be non-nil' unless first raise 'Slug second part must be non-nil' unless second - raise "Invalid first part format #{first}" unless first.match? /^[[[:alnum:]]\_\ \-\.]+$/ - raise "Invalid second part format #{second}" unless second.match? /^[[[:alnum:]]\_\ \-\.]+$/ + raise "Invalid first part format #{first}" unless first.match? SLUG_REGEXP + raise "Invalid second part format #{second}" unless second.match? SLUG_REGEXP @first = first @second = second From 32d8d3d4c78d645cf3deccc7ada8eb111fc2eb55 Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Wed, 22 May 2019 13:05:15 -0300 Subject: [PATCH 16/19] Improving validations --- lib/mumukit/auth/grant.rb | 9 +++++++++ lib/mumukit/auth/slug.rb | 10 +++++----- spec/mumukit/grant_spec.rb | 25 +++++++++++++++++-------- spec/mumukit/permissions_spec.rb | 2 +- spec/mumukit/slug_spec.rb | 2 +- 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/lib/mumukit/auth/grant.rb b/lib/mumukit/auth/grant.rb index 70c56a8..65394f1 100644 --- a/lib/mumukit/auth/grant.rb +++ b/lib/mumukit/auth/grant.rb @@ -144,6 +144,8 @@ class SingleGrant < Base attr_accessor :slug def initialize(slug) + raise Mumukit::Auth::InvalidGrantFormatError, "Invalid slug grant. First part must not be _" if slug.first == '_' + raise Mumukit::Auth::InvalidGrantFormatError, "Invalid slug grant. Second part must not be _" if slug.second == '_' @slug = slug.normalize end @@ -162,6 +164,13 @@ def to_s def self.try_parse(pattern) new(Mumukit::Auth::Slug.parse pattern) + rescue Mumukit::Auth::InvalidSlugFormatError => e + raise Mumukit::Auth::InvalidGrantFormatError, "Invalid slug grant. Cause: #{e}" end end end + +module Mumukit::Auth + class InvalidGrantFormatError < StandardError + end +end diff --git a/lib/mumukit/auth/slug.rb b/lib/mumukit/auth/slug.rb index 0163ed6..3535850 100644 --- a/lib/mumukit/auth/slug.rb +++ b/lib/mumukit/auth/slug.rb @@ -17,11 +17,11 @@ class Slug alias_method :content, :second def initialize(first, second) - raise 'Slug first part must be non-nil' unless first - raise 'Slug second part must be non-nil' unless second + raise Mumukit::Auth::InvalidSlugFormatError, 'Slug first part must be non-nil' unless first + raise Mumukit::Auth::InvalidSlugFormatError, 'Slug second part must be non-nil' unless second - raise "Invalid first part format #{first}" unless first.match? SLUG_REGEXP - raise "Invalid second part format #{second}" unless second.match? SLUG_REGEXP + raise Mumukit::Auth::InvalidSlugFormatError, "Invalid first part format #{first}" unless first.match? SLUG_REGEXP + raise Mumukit::Auth::InvalidSlugFormatError, "Invalid second part format #{second}" unless second.match? SLUG_REGEXP @first = first @second = second @@ -133,7 +133,7 @@ def match(pattern, part) def self.validate_slug!(slug) unless slug =~ /\A[^\/\n]+\/[^\/\n]+\z/ - raise Mumukit::Auth::InvalidSlugFormatError, "Invalid slug: #{slug}. It must be in first/second format" + raise Mumukit::Auth::InvalidSlugFormatError, "Invalid slug #{slug}. It must be in first/second format" end end end diff --git a/spec/mumukit/grant_spec.rb b/spec/mumukit/grant_spec.rb index 4561dbe..eb5ee85 100644 --- a/spec/mumukit/grant_spec.rb +++ b/spec/mumukit/grant_spec.rb @@ -7,6 +7,16 @@ it { expect('Foo/Bar'.to_mumukit_grant.to_s).to eq 'foo/bar' } end + describe '.to_mumukit_grant' do + it { expect { '_'.to_mumukit_grant }.to raise_error('Invalid slug grant. Cause: Invalid slug _. It must be in first/second format') } + it { expect { '_/_'.to_mumukit_grant }.to raise_error('Invalid slug grant. First part must not be _') } + it { expect { 'foo/_'.to_mumukit_grant }.to raise_error('Invalid slug grant. Second part must not be _') } + it { expect { 'doo!/foo'.to_mumukit_grant }.to raise_error('Invalid slug grant. Cause: Invalid first part format doo!') } + it { expect { '[foo]'.to_mumukit_grant }.to raise_error('Invalid slug grant. Cause: Invalid slug [foo]. It must be in first/second format') } + it { expect { '{foo}'.to_mumukit_grant }.to raise_error('Invalid slug grant. Cause: Invalid slug {foo}. It must be in first/second format') } + it { expect { '*/foo'.to_mumukit_grant }.to raise_error('Invalid slug grant. Cause: Invalid first part format *') } + end + describe 'compare' do it { expect('foo/baz'.to_mumukit_grant).to eq 'foo/baz'.to_mumukit_grant } it { expect('FOO/BAZ'.to_mumukit_grant).to eq 'foo/baz'.to_mumukit_grant } @@ -87,9 +97,6 @@ it { expect(grant.allows? 'FOO/BAR').to be true } it { expect(Mumukit::Auth::Grant.parse('FOO/Bar').allows? 'foo/BAR').to be true } - - xit { expect('foo/bar'.to_mumukit_grant.includes? '*/*').to raise_error('invalid slug') } - xit { expect('foo/bar'.to_mumukit_grant.includes? '*').to raise_error('invalid slug') } end describe 'includes?' do @@ -99,7 +106,9 @@ it { expect('foo/bar'.to_mumukit_grant.includes? 'foo/*').to be false } it { expect('*'.to_mumukit_grant.includes? 'foo/bar').to be true } it { expect('foo/bar'.to_mumukit_grant.includes? '*').to be false } - xit { expect('foo/bar'.to_mumukit_grant.includes? '_/_').to raise_error('invalid grant') } + + it { expect { 'foo/bar'.to_mumukit_grant.includes? '_/_' }.to raise_error('Invalid slug grant. First part must not be _') } + it { expect { 'foo/bar'.to_mumukit_grant.includes? 'foo/_' }.to raise_error('Invalid slug grant. Second part must not be _') } end describe 'custom grant' do @@ -109,17 +118,17 @@ def allows?(slug_like) end def to_s - '{includesFoo}' + '[includesFoo]' end def self.try_parse(pattern) - new if pattern =~ /\{includesFoo\}/ + new if pattern =~ /\[includesFoo\]/ end end before(:all) { Mumukit::Auth::Grant.add_custom_grant_type! IncludesGrant } after(:all) { Mumukit::Auth::Grant.remove_custom_grant_type! IncludesGrant } - let(:grant) { Mumukit::Auth::Grant.parse('{includesFoo}') } + let(:grant) { Mumukit::Auth::Grant.parse('[includesFoo]') } it { expect(Mumukit::Auth::Grant.custom_grant_types).to eq [IncludesGrant]} it { expect(grant.allows? 'foo/baz').to be true } @@ -130,6 +139,6 @@ def self.try_parse(pattern) it { expect(grant.includes? 'foobar/baz').to be false } it { expect(grant.includes? 'bar/baz').to be false } - it { expect(grant.includes? '{includesFoo}').to be true } + it { expect(grant.includes? '[includesFoo]').to be true } end end diff --git a/spec/mumukit/permissions_spec.rb b/spec/mumukit/permissions_spec.rb index 2e06fea..b2dec9f 100644 --- a/spec/mumukit/permissions_spec.rb +++ b/spec/mumukit/permissions_spec.rb @@ -220,7 +220,7 @@ def parse_permissions(hash) let(:permissions) { parse_permissions(student: 'foo/bar:test/*:foo/baz') } it { expect { permissions.student? 'test/*' }.to raise_error 'Invalid second part format *' } - it { expect { permissions.student? '*' }.to raise_error 'Invalid slug: *. It must be in first/second format' } + it { expect { permissions.student? '*' }.to raise_error 'Invalid slug *. It must be in first/second format' } context 'when permission is present' do before { permissions.remove_permission!(:student, 'test/*') } diff --git a/spec/mumukit/slug_spec.rb b/spec/mumukit/slug_spec.rb index 041657a..dddead4 100644 --- a/spec/mumukit/slug_spec.rb +++ b/spec/mumukit/slug_spec.rb @@ -39,7 +39,7 @@ it { expect(Mumukit::Auth::Slug.join_s(organization: 'foo', course: 'bar')).to eq 'foo/bar' } it { expect { Mumukit::Auth::Slug.join('foo', 'bar', 'baz') }.to raise_error 'Slugs must have up to two parts' } - it { expect { Mumukit::Auth::Slug.parse('baz') }.to raise_error 'Invalid slug: baz. It must be in first/second format' } + it { expect { Mumukit::Auth::Slug.parse('baz') }.to raise_error 'Invalid slug baz. It must be in first/second format' } it { expect(Mumukit::Auth::Slug.normalize('foo', 'bar').to_s).to eq 'foo/bar' } it { expect(Mumukit::Auth::Slug.normalize('Foo', 'Bar').to_s).to eq 'foo/bar' } From f3d318c093184af1b36f077127ac69caf57ea75d Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Wed, 22 May 2019 13:27:42 -0300 Subject: [PATCH 17/19] Adding misisng validation --- lib/mumukit/auth/grant.rb | 3 ++- spec/mumukit/grant_spec.rb | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/mumukit/auth/grant.rb b/lib/mumukit/auth/grant.rb index 65394f1..f0ede80 100644 --- a/lib/mumukit/auth/grant.rb +++ b/lib/mumukit/auth/grant.rb @@ -111,6 +111,7 @@ class FirstPartGrant < Base attr_accessor :first def initialize(first) + raise Mumukit::Auth::InvalidGrantFormatError, "Invalid first grant. First part must not be _" if first == '_' @first = first.downcase end @@ -136,7 +137,7 @@ def includes?(grant_like) end def self.try_parse(pattern) - new($1) if pattern =~ /(.*)\/\*/ + new($1) if pattern =~ /(.+)\/\*/ end end diff --git a/spec/mumukit/grant_spec.rb b/spec/mumukit/grant_spec.rb index eb5ee85..92ea051 100644 --- a/spec/mumukit/grant_spec.rb +++ b/spec/mumukit/grant_spec.rb @@ -15,6 +15,7 @@ it { expect { '[foo]'.to_mumukit_grant }.to raise_error('Invalid slug grant. Cause: Invalid slug [foo]. It must be in first/second format') } it { expect { '{foo}'.to_mumukit_grant }.to raise_error('Invalid slug grant. Cause: Invalid slug {foo}. It must be in first/second format') } it { expect { '*/foo'.to_mumukit_grant }.to raise_error('Invalid slug grant. Cause: Invalid first part format *') } + it { expect { '_/*'.to_mumukit_grant }.to raise_error('Invalid first grant. First part must not be _') } end describe 'compare' do From b8bfc1f557571f96ff51d86f4a53370fd7381948 Mon Sep 17 00:00:00 2001 From: Franco Bulgarelli Date: Wed, 22 May 2019 13:27:55 -0300 Subject: [PATCH 18/19] Altering grants priority as @luchotc suggested --- lib/mumukit/auth/grant.rb | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/mumukit/auth/grant.rb b/lib/mumukit/auth/grant.rb index f0ede80..f5ba1f8 100644 --- a/lib/mumukit/auth/grant.rb +++ b/lib/mumukit/auth/grant.rb @@ -5,6 +5,19 @@ def to_mumukit_grant end module Mumukit::Auth::Grant + + # Parses a given string that describes a grant + # by trying with each grant type, in the following order: + # + # 1. All grant + # 2. Custom grants + # 3. First Grant + # 4. Slug grant + # + # Raises a `Mumukit::Auth::InvalidGrantFormatError` if the grant is not valid, which happens when + # + # * structure is invalid + # * `something/_`, `_/something`, `_/_` format is used def self.parse(pattern) grant_types.each do |type| type.try_parse(pattern).try { |it| return it } @@ -12,7 +25,7 @@ def self.parse(pattern) end def self.grant_types - custom_grant_types + [AllGrant, FirstPartGrant, SingleGrant] + [AllGrant] + custom_grant_types + [FirstPartGrant, SingleGrant] end def self.add_custom_grant_type!(grant_type) From 13fe905a34515a91c5bd6af308f40e41cd12b202 Mon Sep 17 00:00:00 2001 From: Lucho Cannavo Date: Tue, 28 May 2019 18:06:06 -0300 Subject: [PATCH 19/19] fixing minor typos --- README.md | 2 +- lib/mumukit/auth/grant.rb | 2 +- lib/mumukit/auth/permissions.rb | 4 ++-- lib/mumukit/auth/role.rb | 2 +- lib/mumukit/auth/slug.rb | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 5674e98..c5d63bb 100644 --- a/README.md +++ b/README.md @@ -105,7 +105,7 @@ a_grant.to_s #### Defining custom Grants -Grants can be extend, by inheriting from `Mumukit::Auth::Grant::Base`, and defining the following method: +Grants can be extended, by inheriting from `Mumukit::Auth::Grant::Base`, and defining the following method: * `#allows?(resource_slug)`: mandatory * `#to_s`: mandatory diff --git a/lib/mumukit/auth/grant.rb b/lib/mumukit/auth/grant.rb index f5ba1f8..dcf34d8 100644 --- a/lib/mumukit/auth/grant.rb +++ b/lib/mumukit/auth/grant.rb @@ -71,7 +71,7 @@ def inspect "" end - # Tells wether the given grant + # Tells whether the given grant # is authorized by this grant # # This method exist in order to implement double dispatching diff --git a/lib/mumukit/auth/permissions.rb b/lib/mumukit/auth/permissions.rb index 2b73147..2accfb2 100644 --- a/lib/mumukit/auth/permissions.rb +++ b/lib/mumukit/auth/permissions.rb @@ -26,7 +26,7 @@ def has_permission?(role, thing) end end - # tells wether this permissions + # tells whether this permissions # authorize the given authorizable object for the given role, # or any of its parent roles. def authorizes?(role, authorizable) @@ -38,7 +38,7 @@ def allows?(role, slug_like) authorizes? role, slug_like.to_mumukit_slug end - # tells wether this permissions + # tells whether this permissions # authorize the given authorizable object for the specific given role def role_authorizes?(role, authorizable) scope_for(role).authorizes?(authorizable) diff --git a/lib/mumukit/auth/role.rb b/lib/mumukit/auth/role.rb index 97e655a..9edfc84 100644 --- a/lib/mumukit/auth/role.rb +++ b/lib/mumukit/auth/role.rb @@ -4,7 +4,7 @@ def initialize(symbol) @symbol=symbol end - # Tells wether the given authorizable object + # Tells whether the given authorizable object # can be authorized using the given permissions # by this role or its parent role # diff --git a/lib/mumukit/auth/slug.rb b/lib/mumukit/auth/slug.rb index 3535850..d3a0153 100644 --- a/lib/mumukit/auth/slug.rb +++ b/lib/mumukit/auth/slug.rb @@ -73,7 +73,7 @@ def to_mumukit_slug self end - # Tells wether the given grant + # Tells whether the given grant # is authorized by this slug # # This method exist in order to implement double dispatching