From 3589b31e1b7577c68171e79b0e6992cd43fa588f Mon Sep 17 00:00:00 2001 From: Martin Fenner Date: Thu, 27 Feb 2020 07:18:22 +0100 Subject: [PATCH] introduce consortium_admin role. #432 --- app/models/ability.rb | 10 +++++----- app/models/concerns/authenticable.rb | 9 ++------- app/models/provider.rb | 24 ++++++++++++------------ app/models/user.rb | 13 ++++--------- spec/concerns/authenticable_spec.rb | 13 +++++++------ spec/models/ability_spec.rb | 4 ++-- spec/models/user_spec.rb | 18 +----------------- spec/requests/repositories_spec.rb | 2 +- spec/requests/works_spec.rb | 2 +- 9 files changed, 35 insertions(+), 60 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index dfadabca4..4172b8db0 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -19,17 +19,17 @@ def initialize(user) can :export, :repositories elsif user.role_id == "staff_user" can :read, :all - elsif user.role_id == "provider_admin" && user.provider_id.present? && user.consortium_id.present? + elsif user.role_id == "consortium_admin" && user.provider_id.present? can [:update, :read, :read_billing_information], Provider, symbol: user.provider_id.upcase can [:manage], Provider do |provider| - provider.consortium_id == user.consortium_id.upcase + provider.consortium_id == user.provider_id.upcase end can [:read], Provider can [:manage], ProviderPrefix do |provider_prefix| - provider_prefix.provider && provider_prefix.provider.consortium_id == user.consortium_id.upcase + provider_prefix.provider && provider_prefix.provider.consortium_id == user.provider_id.upcase end can [:manage], Client do |client| - client.provider && client.provider.consortium_id == user.consortium_id.upcase + client.provider && client.provider.consortium_id == user.provider_id.upcase end can [:manage], ClientPrefix #, :client_id => user.provider_id @@ -46,7 +46,7 @@ def initialize(user) can [:read], User can [:read], Phrase can [:read], Activity do |activity| - activity.doi.findable? || activity.doi.provider && activity.doi.provider.consortium_id == user.consortium_id.upcase + activity.doi.findable? || activity.doi.provider && activity.doi.provider.consortium_id == user.provider_id.upcase end elsif user.role_id == "provider_admin" && user.provider_id.present? can [:update, :read, :read_billing_information], Provider, symbol: user.provider_id.upcase diff --git a/app/models/concerns/authenticable.rb b/app/models/concerns/authenticable.rb index 982a7293a..0d6470073 100644 --- a/app/models/concerns/authenticable.rb +++ b/app/models/concerns/authenticable.rb @@ -143,7 +143,7 @@ def get_payload(uid: nil, user: nil, password: nil) "ROLE_ADMIN" => "staff_admin", "ROLE_DATACENTRE" => "client_admin", "ROLE_ALLOCATOR" => "provider_admin", - "ROLE_CONSORTIUM" => "provider_admin", + "ROLE_CONSORTIUM" => "consortium_admin", "ROLE_CONSORTIUM_ORGANIZATION" => "provider_admin", "ROLE_CONTRACTUAL_PROVIDER" => "provider_admin", "ROLE_FOR_PROFIT_PROVIDER" => "provider_admin", @@ -163,11 +163,6 @@ def get_payload(uid: nil, user: nil, password: nil) "client_id" => uid, "password" => password, ) - elsif user.role_name == "ROLE_CONSORTIUM" - payload.merge!( - "provider_id" => uid, - "consortium_id" => uid, - ) elsif uid != "admin" payload.merge!( "provider_id" => uid, @@ -194,6 +189,7 @@ def not_allowed_by_doi_and_user(doi: nil, user: nil) return false if doi.aasm_state == "findable" return true if user.blank? return false if %w(staff_admin staff_user).include?(user.role_id) + return false if %w(consortium_admin).include?(user.role_id) && user.provider_id.present? && user.provider_id == doi.client.consortium_id return false if %w(provider_admin provider_user).include?(user.role_id) && user.provider_id.present? && user.provider_id == doi.provider_id return false if %w(client_admin client_user user temporary).include?(user.role_id) && user.client_id.present? && user.client_id == doi.client_id return true @@ -251,7 +247,6 @@ def generate_token(attributes={}) uid: attributes.fetch(:uid, "0000-0001-5489-3594"), name: attributes.fetch(:name, "Josiah Carberry"), email: attributes.fetch(:email, nil), - consortium_id: attributes.fetch(:consortium_id, nil), provider_id: attributes.fetch(:provider_id, nil), client_id: attributes.fetch(:client_id, nil), role_id: attributes.fetch(:role_id, "staff_admin"), diff --git a/app/models/provider.rb b/app/models/provider.rb index e52adaf2a..68669f5f1 100644 --- a/app/models/provider.rb +++ b/app/models/provider.rb @@ -438,15 +438,15 @@ def member_type_label def member_type_labels { - "ROLE_MEMBER" => "Member Only", - "ROLE_ALLOCATOR" => "Direct Member", - "ROLE_CONSORTIUM" => "Consortium", + "ROLE_MEMBER" => "Member Only", + "ROLE_ALLOCATOR" => "Direct Member", + "ROLE_CONSORTIUM" => "Consortium", "ROLE_CONSORTIUM_ORGANIZATION" => "Consortium Organization", "ROLE_CONTRACTUAL_PROVIDER" => "Contractual Member", - "ROLE_ADMIN" => "DataCite admin", - "ROLE_DEV" => "DataCite admin", - "ROLE_FOR_PROFIT_PROVIDER" => "For-profit Provider", - "ROLE_REGISTRATION_AGENCY" => "DOI Registration Agency" + "ROLE_ADMIN" => "DataCite admin", + "ROLE_DEV" => "DataCite admin", + "ROLE_FOR_PROFIT_PROVIDER" => "For-profit Provider", + "ROLE_REGISTRATION_AGENCY" => "DOI Registration Agency", } end @@ -461,13 +461,13 @@ def member_type=(value) def member_types { - "ROLE_MEMBER" => "member_only", - "ROLE_ALLOCATOR" => "direct_member", - "ROLE_CONSORTIUM" => "consortium", + "ROLE_MEMBER" => "member_only", + "ROLE_ALLOCATOR" => "direct_member", + "ROLE_CONSORTIUM" => "consortium", "ROLE_CONSORTIUM_ORGANIZATION" => "consortium_organization", "ROLE_CONTRACTUAL_PROVIDER" => "contractual_member", - "ROLE_FOR_PROFIT_PROVIDER" => "for_profit_provider", - "ROLE_REGISTRATION_AGENCY" => "registration_agency" + "ROLE_FOR_PROFIT_PROVIDER" => "for_profit_provider", + "ROLE_REGISTRATION_AGENCY" => "registration_agency", } end diff --git a/app/models/user.rb b/app/models/user.rb index b2c41b9d0..950dcc8ce 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -11,7 +11,7 @@ class User # include helper module for caching infrequently changing resources include Cacheable - attr_accessor :name, :uid, :email, :role_id, :jwt, :password, :consortium_id, :provider_id, :client_id, :beta_tester, :errors + attr_accessor :name, :uid, :email, :role_id, :jwt, :password, :provider_id, :client_id, :beta_tester, :errors def initialize(credentials, options={}) if credentials.present? && options.fetch(:type, "").downcase == "basic" @@ -29,7 +29,7 @@ def initialize(credentials, options={}) payload = { "uid" => uid, "name" => payload["name"], - "email" => payload["email"] + "email" => payload["email"], } @jwt = encode_token(payload.merge(iat: Time.now.to_i, exp: Time.now.to_i + 3600 * 24 * 30)) @@ -48,7 +48,6 @@ def initialize(credentials, options={}) @email = payload.fetch("email", nil) @password = payload.fetch("password", nil) @role_id = payload.fetch("role_id", nil) - @consortium_id = payload.fetch("consortium_id", nil) @provider_id = payload.fetch("provider_id", nil) @client_id = payload.fetch("client_id", nil) @beta_tester = payload.fetch("beta_tester", false) @@ -76,10 +75,6 @@ def is_beta_tester? beta_tester end - def consortium_id - provider_id if provider && provider.role_name == "ROLE_CONSORTIUM" - end - def provider return nil unless provider_id.present? @@ -112,7 +107,7 @@ def self.reset(username) "role_id" => "temporary", "name" => user.name, "client_id" => client_id, - "provider_id" => provider_id + "provider_id" => provider_id, }.compact jwt = encode_token(payload.merge(iat: Time.now.to_i, exp: Time.now.to_i + 3600 * 24)) @@ -127,7 +122,7 @@ def self.reset(username) fields = [ { title: "Account ID", value: uid.upcase}, { title: "Name", value: user.name, short: true }, - { title: "Contact email", value: user.system_email, short: true } + { title: "System email", value: user.system_email, short: true } ] slack_title = subject + (response[:status] == 200 ? " Sent" : " Failed") level = response[:status] == 200 ? "good" : "danger" diff --git a/spec/concerns/authenticable_spec.rb b/spec/concerns/authenticable_spec.rb index 372164115..1f9c65fbc 100644 --- a/spec/concerns/authenticable_spec.rb +++ b/spec/concerns/authenticable_spec.rb @@ -94,8 +94,8 @@ expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false end - it "provider_admin consortium" do - token = User.generate_token(role_id: "provider_admin", consortium_id: "datacite", provider_id: "datacite") + it "consortium_admin" do + token = User.generate_token(role_id: "consortium_admin", provider_id: "datacite") subject = User.new(token) expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false end @@ -144,7 +144,8 @@ end context "draft doi" do - let(:provider) { create(:provider, symbol: "DATACITE") } + let(:consortium) { create(:provider, symbol: "DC", role_name: "ROLE_CONSORTIUM") } + let(:provider) { create(:provider, symbol: "DATACITE", role_name: "ROLE_CONSORTIUM_ORGANIZATION") } let(:client) { create(:client, provider: provider, symbol: "DATACITE.RPH") } let(:doi) { create(:doi, client: client) } @@ -160,8 +161,8 @@ expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false end - it "provider_admin consortium" do - token = User.generate_token(role_id: "provider_admin", consortium_id: "datacite", provider_id: "datacite") + it "consortium_admin" do + token = User.generate_token(role_id: "consortium_admin", provider_id: "dc") subject = User.new(token) expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false end @@ -275,7 +276,7 @@ it "consortium" do subject = create(:provider, role_name: "ROLE_CONSORTIUM", password_input: "12345") - expect(subject.decode_auth_param(username: subject.symbol, password: "12345")).to eq("uid"=>subject.symbol.downcase, "name"=>subject.name, "email"=>subject.system_email, "role_id"=>"provider_admin", "provider_id"=>subject.symbol.downcase, "consortium_id"=>subject.symbol.downcase) + expect(subject.decode_auth_param(username: subject.symbol, password: "12345")).to eq("uid"=>subject.symbol.downcase, "name"=>subject.name, "email"=>subject.system_email, "role_id"=>"consortium_admin", "provider_id"=>subject.symbol.downcase) end end end diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 5b9bb5d17..af567719a 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -147,8 +147,8 @@ it{ is_expected.not_to be_able_to(:destroy, doi) } end - context "when is a provider admin for a consortium" do - let(:token){ User.generate_token(role_id: "provider_admin", provider_id: consortium.symbol.downcase, consortium_id: consortium.symbol.downcase) } + context "when is a consortium admin" do + let(:token){ User.generate_token(role_id: "consortium_admin", provider_id: consortium.symbol.downcase) } it{ is_expected.to be_able_to(:read, user) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 3c9a7cf70..aecb26786 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -26,10 +26,6 @@ expect(user.role_id).to eq("staff_admin") end - it "has no consortium_id" do - expect(user.consortium_id).to be_nil - end - it "has no provider_id" do expect(user.provider_id).to be_nil end @@ -50,10 +46,6 @@ expect(user.role_id).to eq("provider_admin") end - it "has no consortium_id" do - expect(user.consortium_id).to be_nil - end - it "has provider" do expect(user.provider_id).to eq(provider.symbol.downcase) expect(user.provider.name).to eq(provider.name) @@ -72,11 +64,7 @@ describe 'User attributes' do it "has role_id" do - expect(user.role_id).to eq("provider_admin") - end - - it "has consortium_id" do - expect(user.consortium_id).to eq(provider.symbol.downcase) + expect(user.role_id).to eq("consortium_admin") end it "has provider" do @@ -100,10 +88,6 @@ expect(user.role_id).to eq("client_admin") end - it "has no consortium_id" do - expect(user.consortium_id).to be_nil - end - it "has provider_id" do expect(user.provider_id).to eq(client.symbol.downcase.split(".").first) end diff --git a/spec/requests/repositories_spec.rb b/spec/requests/repositories_spec.rb index 5581107d6..314a24c7e 100644 --- a/spec/requests/repositories_spec.rb +++ b/spec/requests/repositories_spec.rb @@ -6,7 +6,7 @@ let(:provider) { create(:provider, consortium: consortium, role_name: "ROLE_CONSORTIUM_ORGANIZATION", password_input: "12345") } let!(:client) { create(:client, provider: provider, client_type: "repository") } let(:bearer) { User.generate_token(role_id: "provider_admin", provider_id: provider.symbol.downcase) } - let(:consortium_bearer) { User.generate_token(role_id: "provider_admin", provider_id: consortium.symbol.downcase, consortium_id: consortium.symbol.downcase) } + let(:consortium_bearer) { User.generate_token(role_id: "consortium_admin", provider_id: consortium.symbol.downcase) } let(:params) do { "data" => { "type" => "clients", "attributes" => { diff --git a/spec/requests/works_spec.rb b/spec/requests/works_spec.rb index d4a61a7b1..d7b9b6a7b 100644 --- a/spec/requests/works_spec.rb +++ b/spec/requests/works_spec.rb @@ -14,7 +14,7 @@ describe 'GET /works', elasticsearch: true do let!(:dois) { create_list(:doi, 3, client: client, event: "publish") } - + before do Doi.import sleep 1