Skip to content

Commit

Permalink
Fix bad match of nil/empty username
Browse files Browse the repository at this point in the history
When passing nil or empty username, Sequel was matching grants we don't
want because it has rather intelligent handling of nil literals. Here,
we ensure that the left-joined table is actually joined with ANDs inside
the outer OR.
  • Loading branch information
botimer committed Dec 11, 2023
1 parent e8459a7 commit ddfbe2c
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 5 deletions.
16 changes: 12 additions & 4 deletions lauth/app/repositories/grant_repo.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,18 @@ def for_user_and_uri(username, uri)
.left_join(users.name.dataset, userid: grants[:userid])
.left_join(institution_memberships.name.dataset, inst: grants[:inst])
.where(Sequel.ilike(uri, locations[:dlpsPath]))
.where(Sequel.or({
users[:userid] => username,
institution_memberships[:userid] => username
}))
.where(
Sequel.|(
Sequel.&(
Sequel.~(users[:userid] => nil),
{users[:userid] => username}
),
Sequel.&(
Sequel.~(institution_memberships[:userid] => nil),
{institution_memberships[:userid] => username}
)
)
)

rel = grants.class.new(ds)
rel.combine(:user, collections: :locations, institutions: {institution_memberships: :users}).to_a
Expand Down
20 changes: 19 additions & 1 deletion lauth/spec/repositories/grant_repo_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
end

context "when authorizing locations within a collection using identity-only authentication" do
context "for a member of an authorized institution" do
context "with a member of an authorized institution" do
let!(:collection) { Factory[:collection, :restricted_by_username] }
let!(:institution) { Factory[:institution] }
let!(:user) { Factory[:user, userid: "lauth-inst-member"] }
Expand All @@ -67,6 +67,24 @@

expect(grant_ids).to contain_exactly(grant.uniqueIdentifier)
end

it "finds nothing for a nonmember" do
grants = repo.for_user_and_uri("lauth-denied", "/restricted-by-username/")

expect(grants).to be_empty
end

it "finds nothing for an empty user" do
grants = repo.for_user_and_uri("", "/restricted-by-username/")

expect(grants).to be_empty
end

it "finds nothing for a nil user" do
grants = repo.for_user_and_uri(nil, "/restricted-by-username/")

expect(grants).to be_empty
end
end
end
end

0 comments on commit ddfbe2c

Please sign in to comment.