From d654072552a7bdc0ebf81672f03df8364695874c Mon Sep 17 00:00:00 2001 From: Rob Kaufman Date: Wed, 9 Dec 2020 21:13:51 -0800 Subject: [PATCH] Institution visibility with roles (#1656) * Test institution visible works with elevated user model The changes made in #1653 restict access to avoid users from other tenants from access institution visible works (which should only be visible to users of the tenant) by requiring the user to have a role on the tenant. This test attempts to cover the cases for access to institution visible works: end users from within and outside the work's account as well as users with roles within and outside the work's account. * Add a role to all users if they register directly ot a tenant * let it be, let it be * no expect in before block * spec cleanup Co-authored-by: Chris Colvard --- app/models/user.rb | 2 + db/schema.rb | 3 +- spec/models/ability_spec.rb | 4 +- spec/models/user_spec.rb | 8 +- spec/requests/institution_visibility_spec.rb | 82 ++++++++++++++++++++ 5 files changed, 92 insertions(+), 7 deletions(-) create mode 100644 spec/requests/institution_visibility_spec.rb diff --git a/app/models/user.rb b/app/models/user.rb index da63bcb5f..1ec40aa5f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -75,5 +75,7 @@ def groups def add_default_roles add_role :admin, Site.instance unless self.class.joins(:roles).where("roles.name = ?", "admin").any? || Account.global_tenant? + # Role for any given site + add_role :registered, Site.instance end end diff --git a/db/schema.rb b/db/schema.rb index 9f82c942a..cc14e675d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_06_01_204556) do +ActiveRecord::Schema.define(version: 2020_07_30_194003) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -24,6 +24,7 @@ t.integer "fcrepo_endpoint_id" t.string "name" t.integer "redis_endpoint_id" + t.boolean "is_public", default: false t.index ["cname", "tenant"], name: "index_accounts_on_cname_and_tenant" t.index ["cname"], name: "index_accounts_on_cname", unique: true t.index ["fcrepo_endpoint_id"], name: "index_accounts_on_fcrepo_endpoint_id", unique: true diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 446f685e7..424f6bcbc 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -21,8 +21,8 @@ describe "#user_groups" do subject { ability.user_groups } - it "does not have the registered group" do - expect(subject).not_to include 'registered' + it "does have the registered group as they are created on this tenant" do + expect(subject).to include 'registered' end it "does not have the admin group" do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 0f472d94b..7ff0b42b1 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -34,7 +34,7 @@ subject { FactoryBot.create(:admin) } it 'fetches the global roles assigned to the user' do - expect(subject.site_roles.pluck(:name)).to match_array ['admin'] + expect(subject.site_roles.pluck(:name)).to match_array ['admin', 'registered'] end end @@ -42,11 +42,11 @@ subject { FactoryBot.create(:user) } it 'assigns global roles to the user' do - expect(subject.site_roles.pluck(:name)).to be_empty + expect(subject.site_roles.pluck(:name)).to match_array ['registered'] - subject.update(site_roles: ['admin']) + subject.update(site_roles: ['admin', 'registered']) - expect(subject.site_roles.pluck(:name)).to match_array ['admin'] + expect(subject.site_roles.pluck(:name)).to match_array ['admin', 'registered'] end it 'removes roles' do diff --git a/spec/requests/institution_visibility_spec.rb b/spec/requests/institution_visibility_spec.rb new file mode 100644 index 000000000..d850e050d --- /dev/null +++ b/spec/requests/institution_visibility_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +RSpec.describe 'Insitution visiblity work access', type: :request, clean: true, multitenant: true do + let(:account) { create(:account) } + let(:account2) { create(:account) } + let(:tenant_user_attributes) { attributes_for(:user) } + let(:tenant2_user_attributes) { attributes_for(:user) } + let(:work) { create(:work, visibility: 'authenticated') } + + let(:tenant_user) do + post "http://#{account.cname}/users", params: { user: { + display_name: tenant_user_attributes[:name], + email: tenant_user_attributes[:email], + password: tenant_user_attributes[:password], + password_confirmation: tenant_user_attributes[:password] + } } + User.last + end + + let(:tenant2_user) do + post "http://#{account2.cname}/users", params: { user: { + display_name: tenant2_user_attributes[:name], + email: tenant2_user_attributes[:email], + password: tenant2_user_attributes[:password], + password_confirmation: tenant2_user_attributes[:password] + } } + User.last + end + + before do + WebMock.disable! + Apartment::Tenant.create(account.tenant) + Apartment::Tenant.switch(account.tenant) do + Site.update(account: account) + work + end + Apartment::Tenant.create(account2.tenant) + Apartment::Tenant.switch(account2.tenant) do + Site.update(account: account2) + end + + # sign up user 1 at account 1 + tenant_user + # and sign up user 2 at account 2 + tenant2_user + end + + after do + WebMock.enable! + Apartment::Tenant.drop(account.tenant) + Apartment::Tenant.drop(account2.tenant) + end + + describe 'as an end-user' do + it 'allows access for users of the tenant' do + login_as tenant_user, scope: :user + get "http://#{account.cname}/concern/generic_works/#{work.id}" + expect(response.status).to eq(200) + end + + it 'does not allow access for users of other tenants' do + login_as tenant2_user, scope: :user + get "http://#{account.cname}/concern/generic_works/#{work.id}" + expect(response.status).to eq(401) + end + end + + describe 'when a user is invited' do + before do + Apartment::Tenant.switch(account.tenant) do + site = Site.instance + tenant2_user.add_role('manager', site) + end + end + + it 'now allows access for users of the tenant' do + login_as tenant2_user, scope: :user + get "http://#{account.cname}/concern/generic_works/#{work.id}" + expect(response.status).to eq(200) + end + end +end