From f04e3df7e0b03cd96c3b8d8c6534ca22dbfb064c Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Tue, 30 Jan 2024 13:43:24 -0500 Subject: [PATCH 01/19] some questions --- test/exemptions/server_spec.rb | 3 +++ test/exemptions/site_spec.rb | 3 +++ test/projection/public_in_restricted_proxy_spec.rb | 2 ++ test/projection/public_in_restricted_spec.rb | 11 +++++++++++ test/projection/restricted_in_other_spec.rb | 13 +++++++++++++ test/projection/restricted_in_public_proxy_spec.rb | 14 ++++++++++++++ test/projection/restricted_in_public_spec.rb | 2 ++ 7 files changed, 48 insertions(+) diff --git a/test/exemptions/server_spec.rb b/test/exemptions/server_spec.rb index 9395cb6f..4c9aed0d 100644 --- a/test/exemptions/server_spec.rb +++ b/test/exemptions/server_spec.rb @@ -1,4 +1,7 @@ RSpec.describe "Exemptions configured at the server/module level" do # These are paths/URLs marked as exempt in the server or module config. # They should bypass enforcement and delegation. + # + # So these should only require apache config? + # Do we have examples of what this looks like? end diff --git a/test/exemptions/site_spec.rb b/test/exemptions/site_spec.rb index d9e50e30..1a1f20ff 100644 --- a/test/exemptions/site_spec.rb +++ b/test/exemptions/site_spec.rb @@ -1,4 +1,7 @@ RSpec.describe "Exemptions configured at the server/module level" do # These are paths/URLs marked as exempt in the site or virtual host config. # They should bypass enforcement and delegation. + # + # So these should only require apache config? + # Do we have examples of what this looks like? end diff --git a/test/projection/public_in_restricted_proxy_spec.rb b/test/projection/public_in_restricted_proxy_spec.rb index b25f7e31..1efa8fbd 100644 --- a/test/projection/public_in_restricted_proxy_spec.rb +++ b/test/projection/public_in_restricted_proxy_spec.rb @@ -3,4 +3,6 @@ # rewrite rules to be authorized as another collection. Specifically, the # request URL would normally be in restricted space and proxied to an # external app, but adopts the rules for a public collection. + # + # See public_in_restricted_spec.rb but without the proxy setup end diff --git a/test/projection/public_in_restricted_spec.rb b/test/projection/public_in_restricted_spec.rb index 840cde21..542d186d 100644 --- a/test/projection/public_in_restricted_spec.rb +++ b/test/projection/public_in_restricted_spec.rb @@ -3,4 +3,15 @@ # rewrite rules to be authorized as another collection. Specifically, the # request URL would normally be in restricted space, but adopts the rules for # a public collection. + + # create a collection pubc, with location, public + # create a collection priv, with location + # add rewrite rule in apache conf /priv/ -> /pub/ + # create a user priv_user + # add grant priv_user -> priv (technically we shouldn't need this, but....) + # + # as nobody, request pubc -> is pubc, allowed + # as nobody, request priv -> rewrites to pubc, allowed + # as priv_user, request pubc -> is pubc, allowed + # as priv_user, request priv -> rewrites to pubc, allowed end diff --git a/test/projection/restricted_in_other_spec.rb b/test/projection/restricted_in_other_spec.rb index d5970fc4..803c29c7 100644 --- a/test/projection/restricted_in_other_spec.rb +++ b/test/projection/restricted_in_other_spec.rb @@ -3,4 +3,17 @@ # rewrite rules to be authorized as another collection. Specifically, the # request URL would normally be in restricted space, but adopts the rules for # another restricted collection. + + # create a collection foo, with a location + # create a collection bar, with a location + # add rewrite rule to apache config that rewrites locations /foo/ to /bar/ + # create a user foo_user + # create a grant for foo_user->foo + # create a user bar_user + # crete a grant for bar_user->bar + # + # as foo_user, request foo -> rewrites to bar, denied + # as foo_user, request bar -> is bar, denied + # as bar_user, request foo -> rewrites to bar, allowed + # as bar_user, request foo -> is bar, allowed end diff --git a/test/projection/restricted_in_public_proxy_spec.rb b/test/projection/restricted_in_public_proxy_spec.rb index 6be6263d..b0770131 100644 --- a/test/projection/restricted_in_public_proxy_spec.rb +++ b/test/projection/restricted_in_public_proxy_spec.rb @@ -3,4 +3,18 @@ # rewrite rules to be authorized as another collection. Specifically, the # request URL would normally be in public, proxied space, but adopts the # rules for a restricted collection. + + # create a collection pub_coll, with a location, public + # create a collection foo, with a location + # setup a proxy for pub_coll + # add a rewrite rule to apache config that rewrites locations /pub_coll/ to /foo/ + # create a user foo_user + # create a grant foo_user -> foo + # + # Unclear if we actually need the proxied app, but we have one so could use it. + # + # as nobody, request pub_coll -> rewrites to foo, denied + # as nobody, request foo -> is foo, denied (maybe skip this case) + # as foo_user, request pub_coll -> rewrites to foo, allowed + # as foo_user, request foo -> is foo, allowed end diff --git a/test/projection/restricted_in_public_spec.rb b/test/projection/restricted_in_public_spec.rb index 60c457eb..4dbd8523 100644 --- a/test/projection/restricted_in_public_spec.rb +++ b/test/projection/restricted_in_public_spec.rb @@ -3,4 +3,6 @@ # rewrite rules to be authorized as another collection. Specifically, the # request URL would normally be in public space, but adopts the rules for # a restricted collection. + + # See restricted_in_public_proxy_spec.rb but without the proxy setup end From 2325c37e7bcf881b478645f3685aa538217eebfd Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Thu, 15 Feb 2024 15:27:11 -0500 Subject: [PATCH 02/19] public_in_restricted added, rough, green --- apache/conf/test-site.conf | 3 + db/projection.sql | 63 +++++++++++++++++++ db/setup.sh | 1 + .../private/but-not-really/index.html | 1 + test-site/web/projection/public/index.html | 1 + test/projection/public_in_restricted_spec.rb | 52 ++++++++++++--- 6 files changed, 111 insertions(+), 10 deletions(-) create mode 100644 db/projection.sql create mode 100644 test-site/web/projection/private/but-not-really/index.html create mode 100644 test-site/web/projection/public/index.html diff --git a/apache/conf/test-site.conf b/apache/conf/test-site.conf index f6641460..87a64e7b 100644 --- a/apache/conf/test-site.conf +++ b/apache/conf/test-site.conf @@ -6,6 +6,9 @@ # LogLevel debug + RewriteEngine on + RewriteRule "^/projection/private/but-not-really/" "/projection/public/index.html" [L] + ScriptAlias /lauth/test-site/cgi/printenv AuthType RemoteUser diff --git a/db/projection.sql b/db/projection.sql new file mode 100644 index 00000000..78ef48cb --- /dev/null +++ b/db/projection.sql @@ -0,0 +1,63 @@ +INSERT INTO aa_coll VALUES( + 'projection-public', -- uniqueIdentifier + 'projection-public', -- commonName + 'auth system testing: projection', + 'unused', -- dlpsClass + 'none', -- dlpsSource (unused) + 'pw', -- dlpsAuthenMethod + 'd', -- dlpsAuthzType + 't', -- dlpsPartlyPublic + 0, -- manager + CURRENT_TIMESTAMP, 'root', -- modified info + 'f' -- deleted +); + +INSERT INTO aa_coll_obj VALUES( + 'www.lauth.local', -- server hostname, not vhost + '/lauth/test-site/web/projection/public', -- dlpsPath + 'projection-public', -- coll.uniqueIdentifier + CURRENT_TIMESTAMP, 'root', -- modified info + 'f' -- deleted +); + +INSERT INTO aa_coll_obj VALUES( + 'www.lauth.local', -- server hostname, not vhost + '/lauth/test-site/web/projection/public%', -- dlpsPath + 'projection-public', -- coll.uniqueIdentifier + CURRENT_TIMESTAMP, 'root', -- modified info + 'f' -- deleted +); + +INSERT INTO aa_coll VALUES( + 'projection-private', -- uniqueIdentifier + 'projection-private', -- commonName + 'auth system testing: projection', + 'unused', -- dlpsClass + 'none', -- dlpsSource (unused) + 'pw', -- dlpsAuthenMethod + 'd', -- dlpsAuthzType + 'f', -- dlpsPartlyPublic + 0, -- manager + CURRENT_TIMESTAMP, 'root', -- modified info + 'f' -- deleted +); + +INSERT INTO aa_coll_obj VALUES( + 'www.lauth.local', -- server hostname, not vhost + '/lauth/test-site/web/projection/private%', -- dlpsPath + 'projection-private', -- coll.uniqueIdentifier + CURRENT_TIMESTAMP, 'root', -- modified info + 'f' -- deleted +); + +INSERT INTO aa_may_access VALUES( + NULL, -- uniqueIdentifier + 'lauth-allowed', -- userid + NULL, -- user_grp + NULL, -- inst + 'projection-private', -- coll + CURRENT_TIMESTAMP, + 'root', + NULL, + 'f' +); diff --git a/db/setup.sh b/db/setup.sh index 4369f58f..83631ca0 100755 --- a/db/setup.sh +++ b/db/setup.sh @@ -44,4 +44,5 @@ if [[ $all == "true" ]]; then mariadb --user=$user --host=$host --port=$port --password=$password $database < "$directory/test-fixture.sql" mariadb --user=$user --host=$host --port=$port --password=$password $database < "$directory/network.sql" mariadb --user=$user --host=$host --port=$port --password=$password $database < "$directory/delegation.sql" + mariadb --user=$user --host=$host --port=$port --password=$password $database < "$directory/projection.sql" fi diff --git a/test-site/web/projection/private/but-not-really/index.html b/test-site/web/projection/private/but-not-really/index.html new file mode 100644 index 00000000..69c7d561 --- /dev/null +++ b/test-site/web/projection/private/but-not-really/index.html @@ -0,0 +1 @@ +if this page was reached, something went wrong diff --git a/test-site/web/projection/public/index.html b/test-site/web/projection/public/index.html new file mode 100644 index 00000000..f5ee5593 --- /dev/null +++ b/test-site/web/projection/public/index.html @@ -0,0 +1 @@ +public in projection scenarios diff --git a/test/projection/public_in_restricted_spec.rb b/test/projection/public_in_restricted_spec.rb index 542d186d..a3538409 100644 --- a/test/projection/public_in_restricted_spec.rb +++ b/test/projection/public_in_restricted_spec.rb @@ -4,14 +4,46 @@ # request URL would normally be in restricted space, but adopts the rules for # a public collection. - # create a collection pubc, with location, public - # create a collection priv, with location - # add rewrite rule in apache conf /priv/ -> /pub/ - # create a user priv_user - # add grant priv_user -> priv (technically we shouldn't need this, but....) - # - # as nobody, request pubc -> is pubc, allowed - # as nobody, request priv -> rewrites to pubc, allowed - # as priv_user, request pubc -> is pubc, allowed - # as priv_user, request priv -> rewrites to pubc, allowed + include BasicAuth + let(:content) { "public in projection scenarios" } + + context "when logged in as an authorized user" do + let(:website) do + Faraday.new( + url: TestSite::URL, + headers: { "X-Forwarded-User" => good_user } + ) + end + + it "allows access to the public collection" do + response = website.get("/projection/public/") + expect(response.status).to eq HttpCodes::OK + expect(response.body).to include content + end + it "allows access to the private collection" do + response = website.get("/projection/private/but-not-really/") + expect(response.status).to eq HttpCodes::OK + expect(response.body).to include content + end + end + context "when not logged in" do + let(:website) { Faraday.new(TestSite::URL) } + + it "allows access to the public collection" do + response = website.get("/projection/public/") + expect(response.status).to eq HttpCodes::OK + expect(response.body).to include content + end + it "allows access to the private collection" do + response = website.get("/projection/private/but-not-really/") + expect(response.status).to eq HttpCodes::OK + expect(response.body).to include content + end + end + + private + + def website + @website ||= Faraday.new(TestSite::URL) + end end From 2c11c5c4fc92231af2a59737c7f92d314bab15e6 Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Tue, 20 Feb 2024 14:48:47 -0500 Subject: [PATCH 03/19] Fixup apache conf for projection scenarios --- apache/conf/test-site.conf | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/apache/conf/test-site.conf b/apache/conf/test-site.conf index 87a64e7b..bc5f45f6 100644 --- a/apache/conf/test-site.conf +++ b/apache/conf/test-site.conf @@ -7,7 +7,9 @@ # LogLevel debug RewriteEngine on - RewriteRule "^/projection/private/but-not-really/" "/projection/public/index.html" [L] + RewriteRule "^/projection/private/but-not-really/" "/projection/public/index.html" [PT] + RewriteRule "^/projection/public/but-not-really/" "/projection/private/index.html" [PT] + RewriteRule "^/projection/private-also/but-not-really/" "/projection/private/index.html" [PT] ScriptAlias /lauth/test-site/cgi/printenv @@ -29,6 +31,22 @@ Require all granted + + AuthType RemoteUser + + Require valid-user + Require lauth + + + + + AuthType RemoteUser + + Require valid-user + Require lauth + + + AuthType Basic AuthName "Restricted Resource" From b9fdccd61e58715053275821d0bb7866e7f634cc Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Tue, 20 Feb 2024 14:49:07 -0500 Subject: [PATCH 04/19] Fixup db/projection.sql --- db/projection.sql | 59 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 10 deletions(-) diff --git a/db/projection.sql b/db/projection.sql index 78ef48cb..9212c155 100644 --- a/db/projection.sql +++ b/db/projection.sql @@ -5,21 +5,13 @@ INSERT INTO aa_coll VALUES( 'unused', -- dlpsClass 'none', -- dlpsSource (unused) 'pw', -- dlpsAuthenMethod - 'd', -- dlpsAuthzType + 'n', -- dlpsAuthzType 't', -- dlpsPartlyPublic 0, -- manager CURRENT_TIMESTAMP, 'root', -- modified info 'f' -- deleted ); -INSERT INTO aa_coll_obj VALUES( - 'www.lauth.local', -- server hostname, not vhost - '/lauth/test-site/web/projection/public', -- dlpsPath - 'projection-public', -- coll.uniqueIdentifier - CURRENT_TIMESTAMP, 'root', -- modified info - 'f' -- deleted -); - INSERT INTO aa_coll_obj VALUES( 'www.lauth.local', -- server hostname, not vhost '/lauth/test-site/web/projection/public%', -- dlpsPath @@ -35,7 +27,7 @@ INSERT INTO aa_coll VALUES( 'unused', -- dlpsClass 'none', -- dlpsSource (unused) 'pw', -- dlpsAuthenMethod - 'd', -- dlpsAuthzType + 'n', -- dlpsAuthzType 'f', -- dlpsPartlyPublic 0, -- manager CURRENT_TIMESTAMP, 'root', -- modified info @@ -61,3 +53,50 @@ INSERT INTO aa_may_access VALUES( NULL, 'f' ); + +INSERT INTO aa_coll VALUES( + 'projection-private-also', -- uniqueIdentifier + 'projection-private-also', -- commonName + 'auth system testing: projection', + 'unused', -- dlpsClass + 'none', -- dlpsSource (unused) + 'pw', -- dlpsAuthenMethod + 'n', -- dlpsAuthzType + 'f', -- dlpsPartlyPublic + 0, -- manager + CURRENT_TIMESTAMP, 'root', -- modified info + 'f' -- deleted +); + +INSERT INTO aa_coll_obj VALUES( + 'www.lauth.local', -- server hostname, not vhost + '/lauth/test-site/web/projection/private-also%', -- dlpsPath + 'projection-private-also', -- coll.uniqueIdentifier + CURRENT_TIMESTAMP, 'root', -- modified info + 'f' -- deleted +); + +INSERT INTO aa_user VALUES( + 'lauth-allowed-also',NULL,'Lauth',NULL,'Tester-Allowed','lauth-allowed-also@umich.edu', + NULL, -- org unit + 'Library auth system test user - this user is granted access', + 'Ann Arbor','MI','48109-119',NULL,NULL,'Staff',NULL, + '!none', -- umich id, !none + '@umich.edu', -- password, @umich.edu MAY signify SSO + 0,NULL, + CURRENT_TIMESTAMP,'root', -- modified + NULL, -- expiry + 'f' +); + +INSERT INTO aa_may_access VALUES( + NULL, -- uniqueIdentifier + 'lauth-allowed-also', -- userid + NULL, -- user_grp + NULL, -- inst + 'projection-private-also', -- coll + CURRENT_TIMESTAMP, + 'root', + NULL, + 'f' +) From 5abdc5f428ecb2fb78f1d3ac02e72284269d746d Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Tue, 20 Feb 2024 14:49:33 -0500 Subject: [PATCH 05/19] Projection scenario test-site setup --- test-site/web/projection/private-also/but-not-really/index.html | 0 test-site/web/projection/private-also/index.html | 0 test-site/web/projection/private/but-not-really/index.html | 2 +- test-site/web/projection/private/index.html | 1 + test-site/web/projection/public/but-not-really/index.html | 1 + 5 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 test-site/web/projection/private-also/but-not-really/index.html create mode 100644 test-site/web/projection/private-also/index.html create mode 100644 test-site/web/projection/private/index.html create mode 100644 test-site/web/projection/public/but-not-really/index.html diff --git a/test-site/web/projection/private-also/but-not-really/index.html b/test-site/web/projection/private-also/but-not-really/index.html new file mode 100644 index 00000000..e69de29b diff --git a/test-site/web/projection/private-also/index.html b/test-site/web/projection/private-also/index.html new file mode 100644 index 00000000..e69de29b diff --git a/test-site/web/projection/private/but-not-really/index.html b/test-site/web/projection/private/but-not-really/index.html index 69c7d561..2056e811 100644 --- a/test-site/web/projection/private/but-not-really/index.html +++ b/test-site/web/projection/private/but-not-really/index.html @@ -1 +1 @@ -if this page was reached, something went wrong +private but not really; should never be reached diff --git a/test-site/web/projection/private/index.html b/test-site/web/projection/private/index.html new file mode 100644 index 00000000..389eb91b --- /dev/null +++ b/test-site/web/projection/private/index.html @@ -0,0 +1 @@ +private in projection scenarios diff --git a/test-site/web/projection/public/but-not-really/index.html b/test-site/web/projection/public/but-not-really/index.html new file mode 100644 index 00000000..1d0fd519 --- /dev/null +++ b/test-site/web/projection/public/but-not-really/index.html @@ -0,0 +1 @@ +public but not really; should never be reached From b72fc88cc539765f3c1d8eea428d88f8e6e590ce Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Tue, 20 Feb 2024 14:50:05 -0500 Subject: [PATCH 06/19] Add another_good_user --- test/support/auth_users.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/support/auth_users.rb b/test/support/auth_users.rb index d401a01b..168455bb 100644 --- a/test/support/auth_users.rb +++ b/test/support/auth_users.rb @@ -7,6 +7,10 @@ def good_user "lauth-allowed" end + def another_good_user + "lauth-allowed-also" + end + def inst_user "lauth-inst-member" end From c24742f49a3cb213536900c4e31ea5b2cfdd603a Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Tue, 20 Feb 2024 14:50:35 -0500 Subject: [PATCH 07/19] Fix false-positive test in login-or-network system tests Because of a bug where the collection repo would match the first matching location, this was finding the grant for lauth-allowed on the username test fixture and still passing. The test was always intended to use the inst user. This change addresses that. --- test/restrictions/login_or_network_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/restrictions/login_or_network_spec.rb b/test/restrictions/login_or_network_spec.rb index 2798fd88..123375e8 100644 --- a/test/restrictions/login_or_network_spec.rb +++ b/test/restrictions/login_or_network_spec.rb @@ -8,7 +8,7 @@ context "when the client is within an allowed network" do let(:ip) { "10.1.16.22" } it "allows an authorized user" do - response = request(from: ip, as: good_user) + response = request(from: ip, as: inst_user) expect(response.status).to eq HttpCodes::OK end it "allows an unauthorized user" do @@ -24,7 +24,7 @@ context "when the client is within a denied network" do let(:ip) { "10.1.17.2" } it "allows an authorized user" do - response = request(from: ip, as: good_user) + response = request(from: ip, as: inst_user) expect(response.status).to eq HttpCodes::OK end it "denies an unauthorized user" do @@ -42,7 +42,7 @@ context "when the client is outside known networks" do let(:ip) { "10.1.8.1" } it "allows an authorized user" do - response = request(from: ip, as: good_user) + response = request(from: ip, as: inst_user) expect(response.status).to eq HttpCodes::OK end it "denies an unauthorized user" do From ed36215ca5ea1990b2288dcdbf5a69cc885ae7f6 Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Tue, 20 Feb 2024 14:53:23 -0500 Subject: [PATCH 08/19] Remove system test scenarios for projection onto proxies These would be identical to the other tests, and not provide additional coverage or value. --- .../public_in_restricted_proxy_spec.rb | 8 -------- .../restricted_in_public_proxy_spec.rb | 20 ------------------- 2 files changed, 28 deletions(-) delete mode 100644 test/projection/public_in_restricted_proxy_spec.rb delete mode 100644 test/projection/restricted_in_public_proxy_spec.rb diff --git a/test/projection/public_in_restricted_proxy_spec.rb b/test/projection/public_in_restricted_proxy_spec.rb deleted file mode 100644 index 1efa8fbd..00000000 --- a/test/projection/public_in_restricted_proxy_spec.rb +++ /dev/null @@ -1,8 +0,0 @@ -RSpec.describe "Projecting public access in restricted, proxied space" do - # These are resources that are rewritten internally and sent back through the - # rewrite rules to be authorized as another collection. Specifically, the - # request URL would normally be in restricted space and proxied to an - # external app, but adopts the rules for a public collection. - # - # See public_in_restricted_spec.rb but without the proxy setup -end diff --git a/test/projection/restricted_in_public_proxy_spec.rb b/test/projection/restricted_in_public_proxy_spec.rb deleted file mode 100644 index b0770131..00000000 --- a/test/projection/restricted_in_public_proxy_spec.rb +++ /dev/null @@ -1,20 +0,0 @@ -RSpec.describe "Projecting restricted access in public, proxied space" do - # These are resources that are rewritten internally and sent back through the - # rewrite rules to be authorized as another collection. Specifically, the - # request URL would normally be in public, proxied space, but adopts the - # rules for a restricted collection. - - # create a collection pub_coll, with a location, public - # create a collection foo, with a location - # setup a proxy for pub_coll - # add a rewrite rule to apache config that rewrites locations /pub_coll/ to /foo/ - # create a user foo_user - # create a grant foo_user -> foo - # - # Unclear if we actually need the proxied app, but we have one so could use it. - # - # as nobody, request pub_coll -> rewrites to foo, denied - # as nobody, request foo -> is foo, denied (maybe skip this case) - # as foo_user, request pub_coll -> rewrites to foo, allowed - # as foo_user, request foo -> is foo, allowed -end From cb7c093c1a2b312b0a098b2c0fa19e895641b8b1 Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Tue, 20 Feb 2024 14:54:29 -0500 Subject: [PATCH 09/19] fixup projection/public_in_restricted_spec.rb --- test/projection/public_in_restricted_spec.rb | 21 +++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/test/projection/public_in_restricted_spec.rb b/test/projection/public_in_restricted_spec.rb index a3538409..ca730f73 100644 --- a/test/projection/public_in_restricted_spec.rb +++ b/test/projection/public_in_restricted_spec.rb @@ -5,7 +5,6 @@ # a public collection. include BasicAuth - let(:content) { "public in projection scenarios" } context "when logged in as an authorized user" do let(:website) do @@ -15,29 +14,33 @@ ) end + it "allows access to the projected-public area in the private collection" do + response = website.get("/projection/private/but-not-really/") + expect(response.status).to eq HttpCodes::OK + end it "allows access to the public collection" do response = website.get("/projection/public/") expect(response.status).to eq HttpCodes::OK - expect(response.body).to include content end it "allows access to the private collection" do - response = website.get("/projection/private/but-not-really/") + response = website.get("/projection/private/") expect(response.status).to eq HttpCodes::OK - expect(response.body).to include content end end context "when not logged in" do let(:website) { Faraday.new(TestSite::URL) } + it "allows access to the projected-public area in the private collection" do + response = website.get("/projection/private/but-not-really/") + expect(response.status).to eq HttpCodes::OK + end it "allows access to the public collection" do response = website.get("/projection/public/") expect(response.status).to eq HttpCodes::OK - expect(response.body).to include content end - it "allows access to the private collection" do - response = website.get("/projection/private/but-not-really/") - expect(response.status).to eq HttpCodes::OK - expect(response.body).to include content + it "denies access to the private collection" do + response = website.get("/projection/private/") + expect(response.status).to eq HttpCodes::FORBIDDEN end end From e167da109afa6c3e79449f99dd1ecfaab11c7c40 Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Tue, 20 Feb 2024 14:54:58 -0500 Subject: [PATCH 10/19] Add tests for projection/restricted_in_public --- test/projection/restricted_in_public_spec.rb | 47 +++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/test/projection/restricted_in_public_spec.rb b/test/projection/restricted_in_public_spec.rb index 4dbd8523..d9cc18ac 100644 --- a/test/projection/restricted_in_public_spec.rb +++ b/test/projection/restricted_in_public_spec.rb @@ -4,5 +4,50 @@ # request URL would normally be in public space, but adopts the rules for # a restricted collection. - # See restricted_in_public_proxy_spec.rb but without the proxy setup + include BasicAuth + + context "when logged in as an authorized user" do + let(:website) do + Faraday.new( + url: TestSite::URL, + headers: { "X-Forwarded-User" => good_user } + ) + end + + it "allows access to the projected-private area in the public collection" do + response = website.get("/projection/public/but-not-really/") + expect(response.status).to eq HttpCodes::OK + end + it "allows access to the public collection" do + response = website.get("/projection/public/") + expect(response.status).to eq HttpCodes::OK + end + it "allows access to the private collection" do + response = website.get("/projection/private/") + expect(response.status).to eq HttpCodes::OK + end + end + + context "when not logged in" do + let(:website) { Faraday.new(TestSite::URL) } + + it "denies access to the projected-private area in the public collection" do + response = website.get("/projection/public/but-not-really/") + expect(response.status).to eq HttpCodes::FORBIDDEN + end + it "allows access to the public collection" do + response = website.get("/projection/public/") + expect(response.status).to eq HttpCodes::OK + end + it "denies access to the private collection" do + response = website.get("/projection/private/") + expect(response.status).to eq HttpCodes::FORBIDDEN + end + end + + private + + def website + @website ||= Faraday.new(TestSite::URL) + end end From be4f789882cb479b366a49e06a0ea15941e21abb Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Tue, 20 Feb 2024 14:55:15 -0500 Subject: [PATCH 11/19] Add tests for projection onto other --- test/projection/restricted_in_other_spec.rb | 67 +++++++++++++++++---- 1 file changed, 55 insertions(+), 12 deletions(-) diff --git a/test/projection/restricted_in_other_spec.rb b/test/projection/restricted_in_other_spec.rb index 803c29c7..d3405180 100644 --- a/test/projection/restricted_in_other_spec.rb +++ b/test/projection/restricted_in_other_spec.rb @@ -3,17 +3,60 @@ # rewrite rules to be authorized as another collection. Specifically, the # request URL would normally be in restricted space, but adopts the rules for # another restricted collection. - - # create a collection foo, with a location - # create a collection bar, with a location - # add rewrite rule to apache config that rewrites locations /foo/ to /bar/ - # create a user foo_user - # create a grant for foo_user->foo - # create a user bar_user - # crete a grant for bar_user->bar # - # as foo_user, request foo -> rewrites to bar, denied - # as foo_user, request bar -> is bar, denied - # as bar_user, request foo -> rewrites to bar, allowed - # as bar_user, request foo -> is bar, allowed + # good_user has been granted access to /projection/private/ + # another_good_user has been granted access to /projection/private-also/ + # /projection/private-also/but-not-really/ is rewritten to /projection/private/ + + include AuthUsers + + context "when logged in as an authorized user of the base collection (good_user)" do + let(:website) do + Faraday.new( + url: TestSite::URL, + headers: { "X-Forwarded-User" => good_user } + ) + end + + it "allows access to the area projected into good_user's collection" do + response = website.get("/projection/private-also/but-not-really/") + expect(response.status).to eq HttpCodes::OK + end + it "denies access to another_good_user's private collection" do + response = website.get("/projection/private-also/") + expect(response.status).to eq HttpCodes::FORBIDDEN + end + it "allows access to good_user's private collection" do + response = website.get("/projection/private/") + expect(response.status).to eq HttpCodes::OK + end + end + + context "when logged in as an authorized user of the projected collection (another_good_user)" do + let(:website) do + Faraday.new( + url: TestSite::URL, + headers: { "X-Forwarded-User" => another_good_user } + ) + end + + it "denies access to the area projected into good_user's collection" do + response = website.get("/projection/private-also/but-not-really/") + expect(response.status).to eq HttpCodes::FORBIDDEN + end + it "allows access to another_good_user's private collection" do + response = website.get("/projection/private-also/") + expect(response.status).to eq HttpCodes::OK + end + it "denies access to good_user's private collection" do + response = website.get("/projection/private/") + expect(response.status).to eq HttpCodes::FORBIDDEN + end + end + + private + + def website + @website ||= Faraday.new(TestSite::URL) + end end From 4c214eefd1ff5fce325f23c44d9b80b94bd66a82 Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Tue, 20 Feb 2024 15:00:03 -0500 Subject: [PATCH 12/19] CollectionRepo#find_by_uri finds most specific collection --- lauth/app/repositories/collection_repo.rb | 18 ++++++++++++++++++ .../spec/repositories/collection_repo_spec.rb | 11 +++++++---- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/lauth/app/repositories/collection_repo.rb b/lauth/app/repositories/collection_repo.rb index 1949224e..223fc881 100644 --- a/lauth/app/repositories/collection_repo.rb +++ b/lauth/app/repositories/collection_repo.rb @@ -4,12 +4,30 @@ class CollectionRepo < ROM::Repository[:collections] include Deps[container: "persistence.rom"] struct_namespace Lauth + # Find a collection via its uri location, specifically the dlpsPath. + # This prefers the "most specific" location. We define that here as + # the path with the deepest nesting, and in case of a tie we then prefer + # whichever path is longest. I.e. the path /foo/bar/baz% is more specific + # than /foo/bar/b%. + # There is an assumption that all dlpsPath values end in the SQL wildcard %. + # @param uri [String] + # @return [Collection] def find_by_uri(uri) dataset = collections .dataset .where(collections[:dlpsDeleted].is("f")) .join(locations.name.dataset, coll: :uniqueIdentifier, dlpsDeleted: "f") .where(Sequel.ilike(uri, locations[:dlpsPath])) + .select_append(Sequel.as( # count the slashes + Sequel.expr { + char_length(:dlpsPath) - char_length(replace(:dlpsPath, '/', '')) + }, + :path_depth + )) + .order( + Sequel.desc(:path_depth), + Sequel.desc(Sequel.expr { length(:dlpsPath) } ) + ) collections.class.new(dataset).to_a.first end diff --git a/lauth/spec/repositories/collection_repo_spec.rb b/lauth/spec/repositories/collection_repo_spec.rb index de4cfea9..cd2a18c8 100644 --- a/lauth/spec/repositories/collection_repo_spec.rb +++ b/lauth/spec/repositories/collection_repo_spec.rb @@ -2,13 +2,16 @@ subject(:repo) { Lauth::Repositories::CollectionRepo.new } describe "#find_by_uri" do - it "finds the collection for the given location path" do - collection = Lauth::Fab::Collection.create - Factory[:location, dlpsPath: "/cool/path%", collection: collection] + it "finds the most specific collection for the given location path" do + expected_collection = Lauth::Fab::Collection.create(uniqueIdentifier: "expected-collection") + wrong_collection = Lauth::Fab::Collection.create + Factory[:location, dlpsPath: "/cool/p%", collection: wrong_collection] + Factory[:location, dlpsPath: "/uncool%", collection: wrong_collection] + Factory[:location, dlpsPath: "/cool/path%", collection: expected_collection] found = repo.find_by_uri("/cool/path") - expect(found.uniqueIdentifier).to eq collection.uniqueIdentifier + expect(found.uniqueIdentifier).to eq "expected-collection" end end From 18a297ccc76f292dc554d60d121d502e6f6c7258 Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Tue, 20 Feb 2024 15:04:29 -0500 Subject: [PATCH 13/19] GrantRepo takes collection, doesn't take uri --- lauth/app/ops/authorize.rb | 6 ++-- lauth/app/repositories/grant_repo.rb | 8 ++---- lauth/spec/ops/authorize_spec.rb | 4 +-- lauth/spec/repositories/grant_repo_spec.rb | 33 +++++++++------------- 4 files changed, 21 insertions(+), 30 deletions(-) diff --git a/lauth/app/ops/authorize.rb b/lauth/app/ops/authorize.rb index 9e3ead5f..f8d26cba 100644 --- a/lauth/app/ops/authorize.rb +++ b/lauth/app/ops/authorize.rb @@ -17,7 +17,7 @@ def call collection = collection_repo.find_by_uri(request.uri) case collection.dlpsAuthzType when "n" - normal_mode + normal_mode(collection: collection) when "d" delegated_mode(collection: collection) else @@ -52,10 +52,10 @@ def delegated_mode(collection:) ) end - def normal_mode + def normal_mode(collection:) relevant_grants = grant_repo.for( username: request.user, - uri: request.uri, + collection: collection, client_ip: request.client_ip ) determination = if relevant_grants.any? diff --git a/lauth/app/repositories/grant_repo.rb b/lauth/app/repositories/grant_repo.rb index 8b630af1..8208035c 100644 --- a/lauth/app/repositories/grant_repo.rb +++ b/lauth/app/repositories/grant_repo.rb @@ -49,14 +49,12 @@ def for_collection_class(username:, client_ip:, collection_class:) rel.to_a end - def for(username:, uri:, client_ip: nil) + def for(username:, collection:, client_ip: nil) smallest_network = smallest_network_for_ip(client_ip) ds = grants .dataset .where(grants[:dlpsDeleted].is("f")) - .join(collections.name.dataset, uniqueIdentifier: :coll, dlpsDeleted: "f") - .join(locations.name.dataset, coll: :uniqueIdentifier, dlpsDeleted: "f") .left_join(users.name.dataset, userid: grants[:userid], dlpsDeleted: "f") .left_join(institutions.name.dataset, uniqueIdentifier: grants[:inst], dlpsDeleted: "f") .left_join(institution_memberships.name.dataset, inst: :uniqueIdentifier, dlpsDeleted: "f") @@ -65,7 +63,7 @@ def for(username:, uri:, client_ip: nil) .left_join(group_memberships.name.dataset, user_grp: :uniqueIdentifier, dlpsDeleted: "f") .left_join(Sequel.as(users.name.dataset, :group_users), userid: :userid, dlpsDeleted: "f") .left_join(Sequel.as(smallest_network, :smallest), inst: institutions[:uniqueIdentifier]) - .where(Sequel.ilike(uri, locations[:dlpsPath])) + .where(grants[:coll] => collection.uniqueIdentifier) .where( Sequel.|( Sequel.&( @@ -90,7 +88,7 @@ def for(username:, uri:, client_ip: nil) ) rel = grants.class.new(ds) - rel.combine(:user, collections: :locations, institutions: {institution_memberships: :users}).to_a + rel.combine(:user, institutions: {institution_memberships: :users}).to_a end private diff --git a/lauth/spec/ops/authorize_spec.rb b/lauth/spec/ops/authorize_spec.rb index 5210ea37..bd0c51eb 100644 --- a/lauth/spec/ops/authorize_spec.rb +++ b/lauth/spec/ops/authorize_spec.rb @@ -26,7 +26,7 @@ it "allows a request with a grant" do allow(grant_repo).to receive(:for).with( username: "cool_dude", - uri: "/some/uri/", + collection: anything, client_ip: "10.11.22.33" ).and_return([:somegrant]) @@ -36,7 +36,7 @@ it "denies a request without any grants" do allow(grant_repo).to receive(:for).with( username: "cool_dude", - uri: "/some/uri/", + collection: anything, client_ip: "10.11.22.33" ).and_return([]) diff --git a/lauth/spec/repositories/grant_repo_spec.rb b/lauth/spec/repositories/grant_repo_spec.rb index ff56dbe6..8860d8f1 100644 --- a/lauth/spec/repositories/grant_repo_spec.rb +++ b/lauth/spec/repositories/grant_repo_spec.rb @@ -26,7 +26,7 @@ let!(:network) { create_network("allow", "10.1.6.0/24") } let!(:enclave) { create_network("deny", "10.1.6.2/31") } it "finds no grant for a client_ip within the denied enclave" do - grants = repo.for(username: "", uri: "/restricted-by-client-ip/", client_ip: "10.1.6.3") + grants = repo.for(username: "", collection: collection, client_ip: "10.1.6.3") expect(grants).to eq [] end @@ -35,7 +35,7 @@ let!(:network) { create_network("deny", "10.1.7.0/24") } let!(:enclave) { create_network("allow", "10.1.7.8/29") } it "finds the grant for a client_ip within the allowed enclave" do - grants = repo.for(username: "", uri: "/restricted-by-client-ip/", client_ip: "10.1.7.14") + grants = repo.for(username: "", collection: collection, client_ip: "10.1.7.14") expect(grants.first.uniqueIdentifier).to eq grant.uniqueIdentifier end @@ -49,32 +49,25 @@ let!(:grant) { Factory[:grant, :for_user, user: user, collection: collection] } it "finds the grant for authorized individual and location within the collection" do - grants = repo.for(username: "lauth-allowed", uri: "/restricted-by-username/") + grants = repo.for(username: "lauth-allowed", collection: collection) expect(grants.first.uniqueIdentifier).to eq grant.uniqueIdentifier end it "finds no grant for unauthorized individual and location within the collection" do - grants = repo.for(username: "lauth-denied", uri: "/restricted-by-username/") + grants = repo.for(username: "lauth-denied", collection: collection) expect(grants).to eq [] end # TODO: extract this describe "grant association loading" do - subject(:found_grant) { repo.for(username: "lauth-allowed", uri: "/restricted-by-username/").first } + subject(:found_grant) { repo.for(username: "lauth-allowed", collection: collection).first } it "loads user" do expect(found_grant.user.userid).to eq grant.user.userid end - it "loads collection" do - expect(found_grant.collection.uniqueIdentifier).to eq "lauth-by-username" - end - - it "loads location" do - expect(found_grant.collection.locations.first.dlpsPath).to eq "/restricted-by-username%" - end end end @@ -86,26 +79,26 @@ let!(:grant) { Factory[:grant, :for_institution, institution: institution, collection: collection] } it "finds that member's grant" do - grant_ids = repo.for(username: "lauth-inst-member", uri: "/restricted-by-username/") + grant_ids = repo.for(username: "lauth-inst-member", collection: collection) .map(&:uniqueIdentifier) expect(grant_ids).to contain_exactly(grant.uniqueIdentifier) end it "finds nothing for a nonmember" do - grants = repo.for(username: "lauth-denied", uri: "/restricted-by-username/") + grants = repo.for(username: "lauth-denied", collection: collection) expect(grants).to be_empty end it "finds nothing for an empty user" do - grants = repo.for(username: "", uri: "/restricted-by-username/") + grants = repo.for(username: "", collection: collection) expect(grants).to be_empty end it "finds nothing for a nil user" do - grants = repo.for(username: nil, uri: "/restricted-by-username/") + grants = repo.for(username: nil, collection: collection) expect(grants).to be_empty end @@ -122,26 +115,26 @@ let!(:grant) { Factory[:grant, :for_group, group: group, collection: collection] } it "finds that member's grant" do - grant_ids = repo.for(username: "lauth-group-member", uri: "/restricted-by-username/") + grant_ids = repo.for(username: "lauth-group-member", collection: collection) .map(&:uniqueIdentifier) expect(grant_ids).to contain_exactly(grant.uniqueIdentifier) end it "finds nothing for a nonmember" do - grants = repo.for(username: "lauth-denied", uri: "/restricted-by-username/") + grants = repo.for(username: "lauth-denied", collection: collection) expect(grants).to be_empty end it "finds nothing for an empty user" do - grants = repo.for(username: "", uri: "/restricted-by-username/") + grants = repo.for(username: "", collection: collection) expect(grants).to be_empty end it "finds nothing for a nil user" do - grants = repo.for(username: nil, uri: "/restricted-by-username/") + grants = repo.for(username: nil, collection: collection) expect(grants).to be_empty end From 8c23f0a622a8067368b871be9e3736e19fed64fe Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Tue, 20 Feb 2024 15:09:16 -0500 Subject: [PATCH 14/19] standardrb pass --- lauth/app/repositories/collection_repo.rb | 4 ++-- lauth/spec/repositories/grant_repo_spec.rb | 1 - test/projection/public_in_restricted_spec.rb | 2 +- test/projection/restricted_in_other_spec.rb | 4 ++-- test/projection/restricted_in_public_spec.rb | 2 +- 5 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lauth/app/repositories/collection_repo.rb b/lauth/app/repositories/collection_repo.rb index 223fc881..0f9730d6 100644 --- a/lauth/app/repositories/collection_repo.rb +++ b/lauth/app/repositories/collection_repo.rb @@ -20,13 +20,13 @@ def find_by_uri(uri) .where(Sequel.ilike(uri, locations[:dlpsPath])) .select_append(Sequel.as( # count the slashes Sequel.expr { - char_length(:dlpsPath) - char_length(replace(:dlpsPath, '/', '')) + char_length(:dlpsPath) - char_length(replace(:dlpsPath, "/", "")) }, :path_depth )) .order( Sequel.desc(:path_depth), - Sequel.desc(Sequel.expr { length(:dlpsPath) } ) + Sequel.desc(Sequel.expr { length(:dlpsPath) }) ) collections.class.new(dataset).to_a.first end diff --git a/lauth/spec/repositories/grant_repo_spec.rb b/lauth/spec/repositories/grant_repo_spec.rb index 8860d8f1..65eabb2d 100644 --- a/lauth/spec/repositories/grant_repo_spec.rb +++ b/lauth/spec/repositories/grant_repo_spec.rb @@ -67,7 +67,6 @@ it "loads user" do expect(found_grant.user.userid).to eq grant.user.userid end - end end diff --git a/test/projection/public_in_restricted_spec.rb b/test/projection/public_in_restricted_spec.rb index ca730f73..53e9c5f7 100644 --- a/test/projection/public_in_restricted_spec.rb +++ b/test/projection/public_in_restricted_spec.rb @@ -10,7 +10,7 @@ let(:website) do Faraday.new( url: TestSite::URL, - headers: { "X-Forwarded-User" => good_user } + headers: {"X-Forwarded-User" => good_user} ) end diff --git a/test/projection/restricted_in_other_spec.rb b/test/projection/restricted_in_other_spec.rb index d3405180..fba4c079 100644 --- a/test/projection/restricted_in_other_spec.rb +++ b/test/projection/restricted_in_other_spec.rb @@ -14,7 +14,7 @@ let(:website) do Faraday.new( url: TestSite::URL, - headers: { "X-Forwarded-User" => good_user } + headers: {"X-Forwarded-User" => good_user} ) end @@ -36,7 +36,7 @@ let(:website) do Faraday.new( url: TestSite::URL, - headers: { "X-Forwarded-User" => another_good_user } + headers: {"X-Forwarded-User" => another_good_user} ) end diff --git a/test/projection/restricted_in_public_spec.rb b/test/projection/restricted_in_public_spec.rb index d9cc18ac..703a996c 100644 --- a/test/projection/restricted_in_public_spec.rb +++ b/test/projection/restricted_in_public_spec.rb @@ -10,7 +10,7 @@ let(:website) do Faraday.new( url: TestSite::URL, - headers: { "X-Forwarded-User" => good_user } + headers: {"X-Forwarded-User" => good_user} ) end From bd2b2b7f02f2906060cd39a7d4a399bf4c39904c Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Tue, 20 Feb 2024 17:57:29 -0500 Subject: [PATCH 15/19] Update soft delete for change to passing collection --- lauth/app/repositories/grant_repo.rb | 3 ++- .../soft_delete/grants_for_client_ip_spec.rb | 18 +----------------- .../grants_for_group_member_spec.rb | 10 +--------- .../soft_delete/grants_for_individual_spec.rb | 10 +--------- .../grants_for_institution_member_spec.rb | 10 +--------- 5 files changed, 6 insertions(+), 45 deletions(-) diff --git a/lauth/app/repositories/grant_repo.rb b/lauth/app/repositories/grant_repo.rb index 8208035c..6b6d66df 100644 --- a/lauth/app/repositories/grant_repo.rb +++ b/lauth/app/repositories/grant_repo.rb @@ -50,8 +50,9 @@ def for_collection_class(username:, client_ip:, collection_class:) end def for(username:, collection:, client_ip: nil) - smallest_network = smallest_network_for_ip(client_ip) + return [] unless collection&.dlpsDeleted == "f" + smallest_network = smallest_network_for_ip(client_ip) ds = grants .dataset .where(grants[:dlpsDeleted].is("f")) diff --git a/lauth/spec/repositories/soft_delete/grants_for_client_ip_spec.rb b/lauth/spec/repositories/soft_delete/grants_for_client_ip_spec.rb index d743b222..35fb855e 100644 --- a/lauth/spec/repositories/soft_delete/grants_for_client_ip_spec.rb +++ b/lauth/spec/repositories/soft_delete/grants_for_client_ip_spec.rb @@ -1,5 +1,5 @@ RSpec.describe "Soft Delete Grants#for Client IP", type: :database do - subject(:grants) { repo.for(username: "", uri: "/restricted-by-client-ip/", client_ip: client_ip) } + subject(:grants) { repo.for(username: "", collection: collection, client_ip: client_ip) } let(:repo) { Lauth::Repositories::GrantRepo.new } let(:collection) { Factory[:collection, :restricted_by_client_ip] } @@ -41,14 +41,6 @@ end end - context "when collection location is soft deleted" do - let(:collection) { Factory[:collection, :restricted_by_client_ip_location_soft_deleted] } - - it "finds no grants" do - expect(grants).to be_empty - end - end - context "when institution is soft deleted" do let(:institution) { Factory[:institution, :soft_deleted] } @@ -96,14 +88,6 @@ end end - context "when collection location is soft deleted" do - let(:collection) { Factory[:collection, :restricted_by_client_ip_location_soft_deleted] } - - it "finds no grants" do - expect(grants).to be_empty - end - end - context "when institution is soft deleted" do let(:institution) { Factory[:institution, :soft_deleted] } diff --git a/lauth/spec/repositories/soft_delete/grants_for_group_member_spec.rb b/lauth/spec/repositories/soft_delete/grants_for_group_member_spec.rb index 6f728dd4..b3f15c8c 100644 --- a/lauth/spec/repositories/soft_delete/grants_for_group_member_spec.rb +++ b/lauth/spec/repositories/soft_delete/grants_for_group_member_spec.rb @@ -1,5 +1,5 @@ RSpec.describe "Soft Delete Grants#for Group Member", type: :database do - subject(:grants) { repo.for(username: "lauth-group-member", uri: "/restricted-by-username/") } + subject(:grants) { repo.for(username: "lauth-group-member", collection: collection) } let(:repo) { Lauth::Repositories::GrantRepo.new } let(:collection) { Factory[:collection, :restricted_by_username, dlpsClass: "someclass"] } @@ -37,14 +37,6 @@ end end - context "when collection location is soft deleted" do - let(:collection) { Factory[:collection, :restricted_by_username_location_soft_deleted] } - - it "finds no grants" do - expect(grants).to be_empty - end - end - context "when group is soft deleted" do let(:group) { Factory[:group, :soft_deleted] } diff --git a/lauth/spec/repositories/soft_delete/grants_for_individual_spec.rb b/lauth/spec/repositories/soft_delete/grants_for_individual_spec.rb index 167cdaa7..31f4f9fa 100644 --- a/lauth/spec/repositories/soft_delete/grants_for_individual_spec.rb +++ b/lauth/spec/repositories/soft_delete/grants_for_individual_spec.rb @@ -1,5 +1,5 @@ RSpec.describe "Soft Delete Grants#for Individual", type: :database do - subject(:grants) { repo.for(username: "lauth-allowed", uri: "/restricted-by-username/") } + subject(:grants) { repo.for(username: "lauth-allowed", collection: collection) } let(:repo) { Lauth::Repositories::GrantRepo.new } let(:collection) { Factory[:collection, :restricted_by_username] } @@ -25,14 +25,6 @@ end end - context "when collection location is soft deleted" do - let(:collection) { Factory[:collection, :restricted_by_username_location_soft_deleted] } - - it "finds no grants" do - expect(grants).to be_empty - end - end - context "when individual is soft deleted" do let(:individual) { Factory[:user, :soft_deleted, userid: "lauth-allowed"] } diff --git a/lauth/spec/repositories/soft_delete/grants_for_institution_member_spec.rb b/lauth/spec/repositories/soft_delete/grants_for_institution_member_spec.rb index 95e354e2..6399a620 100644 --- a/lauth/spec/repositories/soft_delete/grants_for_institution_member_spec.rb +++ b/lauth/spec/repositories/soft_delete/grants_for_institution_member_spec.rb @@ -1,5 +1,5 @@ RSpec.describe "Soft Delete Grants#for Institution Member", type: :database do - subject(:grants) { repo.for(username: "lauth-inst-member", uri: "/restricted-by-username/") } + subject(:grants) { repo.for(username: "lauth-inst-member", collection: collection) } let(:repo) { Lauth::Repositories::GrantRepo.new } let(:collection) { Factory[:collection, :restricted_by_username, dlpsClass: "someclass"] } @@ -37,14 +37,6 @@ end end - context "when collection location is soft deleted" do - let(:collection) { Factory[:collection, :restricted_by_username_location_soft_deleted] } - - it "finds no grants" do - expect(grants).to be_empty - end - end - context "when institution is soft deleted" do let(:institution) { Factory[:institution, :soft_deleted] } From 82b83c1510afd5b4c1a6dcc5f18b8999cb353fc9 Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Tue, 20 Feb 2024 18:06:36 -0500 Subject: [PATCH 16/19] remove defunct, paused location soft delete specs --- .../grants_for_collection_class_spec.rb | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/lauth/spec/repositories/soft_delete/grants_for_collection_class_spec.rb b/lauth/spec/repositories/soft_delete/grants_for_collection_class_spec.rb index 393a1dfd..77fea2e9 100644 --- a/lauth/spec/repositories/soft_delete/grants_for_collection_class_spec.rb +++ b/lauth/spec/repositories/soft_delete/grants_for_collection_class_spec.rb @@ -29,14 +29,6 @@ end end - xcontext "when collection_a location is soft deleted" do - let(:collection_a) { Factory[:collection, :restricted_by_username_location_soft_deleted, dlpsClass: "same-class"] } - - it "finds only grant_b" do - expect(grants.map(&:uniqueIdentifier)).to contain_exactly grant_b.uniqueIdentifier - end - end - context "when collection_b is soft deleted" do let(:collection_b) { Factory[:collection, :soft_deleted, dlpsClass: "same-class"] } @@ -45,14 +37,6 @@ end end - xcontext "when collection_b location is soft deleted" do - let(:collection_b) { Factory[:collection, :location_soft_deleted, dlpsClass: "same-class"] } - - it "finds only grant_a" do - expect(grants.map(&:uniqueIdentifier)).to contain_exactly grant_a.uniqueIdentifier - end - end - context "when individual is soft deleted" do let(:individual) { Factory[:user, :soft_deleted, userid: "lauth-allowed"] } From 4bd70e554a5f29a27a198ee95199f34e66afed62 Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Wed, 21 Feb 2024 18:30:52 -0500 Subject: [PATCH 17/19] Add missing for_collection_class soft delete specs --- .../grants_for_group_member_spec.rb | 24 +++++++++++++++++++ .../grants_for_institution_member_spec.rb | 24 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/lauth/spec/repositories/soft_delete/grants_for_group_member_spec.rb b/lauth/spec/repositories/soft_delete/grants_for_group_member_spec.rb index b3f15c8c..66e8b248 100644 --- a/lauth/spec/repositories/soft_delete/grants_for_group_member_spec.rb +++ b/lauth/spec/repositories/soft_delete/grants_for_group_member_spec.rb @@ -35,6 +35,14 @@ it "finds no grants" do expect(grants).to be_empty end + + it "#for_collection_class finds no grants" do + expect(repo.for_collection_class( + username: "lauth-group-member", + client_ip: "10.99.3.4", + collection_class: "someclass" + )).to be_empty + end end context "when group is soft deleted" do @@ -59,6 +67,14 @@ it "finds no grants" do expect(grants).to be_empty end + + it "#for_collection_class finds no grants" do + expect(repo.for_collection_class( + username: "lauth-group-member", + client_ip: "10.99.3.4", + collection_class: "someclass" + )).to be_empty + end end context "when membership is soft deleted" do @@ -83,5 +99,13 @@ it "finds no grants" do expect(grants).to be_empty end + + it "#for_collection_class finds no grants" do + expect(repo.for_collection_class( + username: "lauth-group-member", + client_ip: "10.99.3.4", + collection_class: "someclass" + )).to be_empty + end end end diff --git a/lauth/spec/repositories/soft_delete/grants_for_institution_member_spec.rb b/lauth/spec/repositories/soft_delete/grants_for_institution_member_spec.rb index 6399a620..07249fac 100644 --- a/lauth/spec/repositories/soft_delete/grants_for_institution_member_spec.rb +++ b/lauth/spec/repositories/soft_delete/grants_for_institution_member_spec.rb @@ -35,6 +35,14 @@ it "finds no grants" do expect(grants).to be_empty end + + it "#for_collection_class finds no grants" do + expect(repo.for_collection_class( + username: "lauth-inst-member", + client_ip: "10.99.3.4", + collection_class: "someclass" + )).to be_empty + end end context "when institution is soft deleted" do @@ -59,6 +67,14 @@ it "finds no grants" do expect(grants).to be_empty end + + it "#for_collection_class finds no grants" do + expect(repo.for_collection_class( + username: "lauth-inst-member", + client_ip: "10.2.3.4", + collection_class: "someclass" + )).to be_empty + end end context "when membership is soft deleted" do @@ -83,5 +99,13 @@ it "finds no grants" do expect(grants).to be_empty end + + it "#for_collection_class finds no grants" do + expect(repo.for_collection_class( + username: "lauth-inst-member", + client_ip: "10.99.3.4", + collection_class: "someclass" + )).to be_empty + end end end From 7f175372675df1b74028101c6dd4cfba224e5e99 Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Wed, 21 Feb 2024 18:31:51 -0500 Subject: [PATCH 18/19] DRY, fix last soft-delete in GrantRepo --- lauth/app/repositories/grant_repo.rb | 76 +++++++++------------------- 1 file changed, 25 insertions(+), 51 deletions(-) diff --git a/lauth/app/repositories/grant_repo.rb b/lauth/app/repositories/grant_repo.rb index 6b6d66df..f600d47c 100644 --- a/lauth/app/repositories/grant_repo.rb +++ b/lauth/app/repositories/grant_repo.rb @@ -10,40 +10,9 @@ def find(id) def for_collection_class(username:, client_ip:, collection_class:) smallest_network = smallest_network_for_ip(client_ip) - - ds = grants - .dataset - .where(grants[:dlpsDeleted].is("f")) - .join(collections.name.dataset, uniqueIdentifier: :coll, dlpsDeleted: "f") - .left_join(users.name.dataset, userid: grants[:userid], dlpsDeleted: "f") - .left_join(institutions.name.dataset, uniqueIdentifier: grants[:inst], dlpsDeleted: "f") - .left_join(institution_memberships.name.dataset, inst: grants[:inst], dlpsDeleted: "f") - .left_join(groups.name.dataset, uniqueIdentifier: grants[:user_grp], dlpsDeleted: "f") - .left_join(group_memberships.name.dataset, user_grp: grants[:user_grp], dlpsDeleted: "f") - .left_join(Sequel.as(smallest_network, :smallest), inst: grants[:inst]) + ds = base_grants_for(username: username, network: smallest_network) + .join(collections.name.dataset, uniqueIdentifier: grants[:coll], dlpsDeleted: "f") .where(collections[:dlpsClass] => collection_class) - .where( - Sequel.|( - Sequel.&( - Sequel.~(users[:userid] => nil), - {users[:userid] => username} - ), - Sequel.&( - Sequel.~(institutions[:uniqueIdentifier] => nil), - Sequel.~(institution_memberships[:userid] => nil), - {institution_memberships[:userid] => username} - ), - Sequel.&( - Sequel.~(groups[:uniqueIdentifier] => nil), - Sequel.~(group_memberships[:userid] => nil), - {group_memberships[:userid] => username} - ), - Sequel.&( - Sequel.~(Sequel[:smallest][:inst] => nil), - {Sequel[:smallest][:dlpsAccessSwitch] => "allow"} - ) - ) - ) rel = grants.class.new(ds) rel.to_a @@ -53,7 +22,28 @@ def for(username:, collection:, client_ip: nil) return [] unless collection&.dlpsDeleted == "f" smallest_network = smallest_network_for_ip(client_ip) - ds = grants + ds = base_grants_for(username: username, network: smallest_network) + .where(grants[:coll] => collection.uniqueIdentifier) + + rel = grants.class.new(ds) + rel.combine(:user, institutions: {institution_memberships: :users}).to_a + end + + private + + def smallest_network_for_ip(client_ip) + ip = client_ip ? IPAddr.new(client_ip).to_i : nil + networks + .dataset + .where(dlpsDeleted: "f") + .where { dlpsAddressStart <= ip } + .where { dlpsAddressEnd >= ip } + .select_append(Sequel.as(Sequel.expr { dlpsAddressEnd - dlpsAddressStart }, :block_size)) + .order(Sequel.asc(:block_size)).limit(1) + end + + def base_grants_for(username:, network:) + grants .dataset .where(grants[:dlpsDeleted].is("f")) .left_join(users.name.dataset, userid: grants[:userid], dlpsDeleted: "f") @@ -63,8 +53,7 @@ def for(username:, collection:, client_ip: nil) .left_join(groups.name.dataset, uniqueIdentifier: grants[:user_grp], dlpsDeleted: "f") .left_join(group_memberships.name.dataset, user_grp: :uniqueIdentifier, dlpsDeleted: "f") .left_join(Sequel.as(users.name.dataset, :group_users), userid: :userid, dlpsDeleted: "f") - .left_join(Sequel.as(smallest_network, :smallest), inst: institutions[:uniqueIdentifier]) - .where(grants[:coll] => collection.uniqueIdentifier) + .left_join(Sequel.as(network, :smallest), inst: institutions[:uniqueIdentifier]) .where( Sequel.|( Sequel.&( @@ -87,23 +76,8 @@ def for(username:, collection:, client_ip: nil) ) ) ) - - rel = grants.class.new(ds) - rel.combine(:user, institutions: {institution_memberships: :users}).to_a end - private - - def smallest_network_for_ip(client_ip) - ip = client_ip ? IPAddr.new(client_ip).to_i : nil - networks - .dataset - .where(dlpsDeleted: "f") - .where { dlpsAddressStart <= ip } - .where { dlpsAddressEnd >= ip } - .select_append(Sequel.as(Sequel.expr { dlpsAddressEnd - dlpsAddressStart }, :block_size)) - .order(Sequel.asc(:block_size)).limit(1) - end end end end From dfb074cac6b628000e962d661bed06ef17df55a5 Mon Sep 17 00:00:00 2001 From: Bryan Hockey Date: Thu, 22 Feb 2024 11:52:35 -0500 Subject: [PATCH 19/19] Remove empty line per standardrb --- lauth/app/repositories/grant_repo.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lauth/app/repositories/grant_repo.rb b/lauth/app/repositories/grant_repo.rb index f600d47c..782a951a 100644 --- a/lauth/app/repositories/grant_repo.rb +++ b/lauth/app/repositories/grant_repo.rb @@ -77,7 +77,6 @@ def base_grants_for(username:, network:) ) ) end - end end end