don't use query for doi permission check. #386

app/controllers/dois_controller.rb

@@ -233,17 +233,9 @@ def index
   def show
     # only show findable DOIs to anonymous users and role user
     # use current_user role to determine permissions to access draft and registered dois
-    options = filter_doi_by_role(current_user).merge(doi: params[:id])
-    puts options
-    if options[:provider_symbol]
-      doi = Doi.joins(:client, :provider).where(["dataset.doi = :doi and (allocator.symbol = :provider_symbol or dataset.aasm_state = 'findable')", options]).where(options).first
-    elsif options[:client_symbol]
-      doi = Doi.joins(:client).where(["dataset.doi = :doi and (datacentre.symbol = :client_symbol or dataset.aasm_state = 'findable')", options]).first
-    else
-      doi = Doi.where(options).first
-    end
-
-    fail ActiveRecord::RecordNotFound unless doi.present?
+    # instead of using ability
+    doi = Doi.where(doi: params[:id]).first
+    fail ActiveRecord::RecordNotFound if not_allowed_by_doi_and_user(doi: doi, user: current_user)
 
     respond_to do |format|
       format.json do

app/controllers/works_controller.rb

@@ -147,7 +147,7 @@ def set_doi
     bm = Benchmark.ms {
       @doi = Doi.where(doi: params[:id], aasm_state: "findable").first
     }
-    fail ActiveRecord::RecordNotFound unless @doi.present?
+    fail ActiveRecord::RecordNotFound if @doi.blank?
     logger.warn method: "GET", path: "/works/#{@doi.doi}", message: "Request DB /works/#{@doi.doi}", duration: bm
   end
 

app/models/concerns/authenticable.rb

@@ -153,19 +153,15 @@ def secure_compare(a, b)
       res == 0
     end
 
-    # filter results based on user permissions
-    def filter_doi_by_role(user)
-      return { aasm_state: "findable" } if user.blank?
-
-      if %w(staff_admin staff_user).include?(user.role_id)
-        {}
-      elsif %w(provider_admin provider_user).include?(user.role_id) && user.provider_id.present?
-        { :provider_symbol => user.provider_id.upcase }
-      elsif %w(client_admin client_user user temporary).include?(user.role_id) && user.client_id.present?
-        { :client_symbol => user.client_id.upcase }
-      else
-        { aasm_state: "findable" }
-      end
+    # check user permissions
+    def not_allowed_by_doi_and_user(doi: nil, user: nil)
+      return true if doi.blank?
+      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(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
     end
   end
 

spec/concerns/authenticable_spec.rb

@@ -54,59 +54,123 @@
     end
   end
 
-  describe "filter_doi_by_role" do
-    it "staff_admin" do
-      token = User.generate_token(role_id: "staff_admin")
-      subject = User.new(token)
-      expect(subject.filter_doi_by_role(subject)).to eq({})
-    end
-
-    it "staff_user" do
-      token = User.generate_token(role_id: "staff_user")
-      subject = User.new(token)
-      expect(subject.filter_doi_by_role(subject)).to eq({})
-    end
-
-    it "provider_admin" do
-      token = User.generate_token(role_id: "provider_admin", provider_id: "datacite")
-      subject = User.new(token)
-      expect(subject.filter_doi_by_role(subject)).to eq(:provider_symbol=>"DATACITE")
-    end
-
-    it "provider_user" do
-      token = User.generate_token(role_id: "provider_user", provider_id: "datacite")
-      subject = User.new(token)
-      expect(subject.filter_doi_by_role(subject)).to eq(:provider_symbol=>"DATACITE")
-    end
-
-    it "client_admin" do
-      token = User.generate_token(role_id: "client_admin", client_id: "datacite.rph")
-      subject = User.new(token)
-      expect(subject.filter_doi_by_role(subject)).to eq(:client_symbol=>"DATACITE.RPH")
-    end
-
-    it "client_user" do
-      token = User.generate_token(role_id: "client_user", client_id: "datacite.rph")
-      subject = User.new(token)
-      expect(subject.filter_doi_by_role(subject)).to eq(:client_symbol=>"DATACITE.RPH")
-    end
-
-    it "user" do
-      token = User.generate_token(role_id: "user")
-      subject = User.new(token)
-      expect(subject.filter_doi_by_role(subject)).to eq(:aasm_state=>"findable")
-    end
-
-    it "temporary" do
-      token = User.generate_token(role_id: "temporary")
-      subject = User.new(token)
-      expect(subject.filter_doi_by_role(subject)).to eq(:aasm_state=>"findable")
-    end
-
-    it "anonymous" do
-      token = User.generate_token(role_id: "anonymous")
-      subject = User.new(token)
-      expect(subject.filter_doi_by_role(subject)).to eq(:aasm_state=>"findable")
+  describe "not_allowed_by_doi_and_user" do
+    context "findable doi" do
+      let(:doi) { create(:doi, event: "publish") }
+
+      it "staff_admin" do
+        token = User.generate_token(role_id: "staff_admin")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false
+      end
+
+      it "staff_user" do
+        token = User.generate_token(role_id: "staff_user")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false
+      end
+
+      it "provider_admin" do
+        token = User.generate_token(role_id: "provider_admin", provider_id: "datacite")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false
+      end
+
+      it "provider_user" do
+        token = User.generate_token(role_id: "provider_user", provider_id: "datacite")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false
+      end
+
+      it "client_admin" do
+        token = User.generate_token(role_id: "client_admin", client_id: "datacite.rph")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false
+      end
+
+      it "client_user" do
+        token = User.generate_token(role_id: "client_user", client_id: "datacite.rph")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false
+      end
+
+      it "user" do
+        token = User.generate_token(role_id: "user")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false
+      end
+
+      it "temporary" do
+        token = User.generate_token(role_id: "temporary")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false
+      end
+
+      it "anonymous" do
+        token = User.generate_token(role_id: "anonymous")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false
+      end
+    end
+
+    context "draft doi" do
+      let(:provider) { create(:provider, symbol: "DATACITE") }
+      let(:client) { create(:client, provider: provider, symbol: "DATACITE.RPH") }
+      let(:doi) { create(:doi, client: client) }
+
+      it "staff_admin" do
+        token = User.generate_token(role_id: "staff_admin")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false
+      end
+
+      it "staff_user" do
+        token = User.generate_token(role_id: "staff_user")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false
+      end
+
+      it "provider_admin" do
+        token = User.generate_token(role_id: "provider_admin", provider_id: "datacite")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false
+      end
+
+      it "provider_user" do
+        token = User.generate_token(role_id: "provider_user", provider_id: "datacite")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false
+      end
+
+      it "client_admin" do
+        token = User.generate_token(role_id: "client_admin", client_id: "datacite.rph")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false
+      end
+
+      it "client_user" do
+        token = User.generate_token(role_id: "client_user", client_id: "datacite.rph")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be false
+      end
+
+      it "user" do
+        token = User.generate_token(role_id: "user")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be true
+      end
+
+      it "temporary" do
+        token = User.generate_token(role_id: "temporary")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be true
+      end
+
+      it "anonymous" do
+        token = User.generate_token(role_id: "anonymous")
+        subject = User.new(token)
+        expect(subject.not_allowed_by_doi_and_user(doi: doi, user: subject)).to be true
+      end
     end
   end
 

spec/requests/dois_spec.rb

@@ -77,6 +77,19 @@
       end
     end
 
+    context 'provider_admin' do
+      let(:provider_bearer) { Client.generate_token(role_id: "provider_admin", uid: provider.symbol, provider_id: provider.symbol.downcase, password: provider.password) }
+      let(:provider_headers) { { 'HTTP_ACCEPT'=>'application/vnd.api+json', 'HTTP_AUTHORIZATION' => 'Bearer ' + provider_bearer }}
+
+
+      it 'returns the Doi' do
+        get "/dois/#{doi.doi}", nil, provider_headers
+
+        expect(last_response.status).to eq(200)
+        expect(json.dig('data', 'attributes', 'doi')).to eq(doi.doi.downcase)
+      end
+    end
+
     context 'anonymous user' do
       it 'returns the Doi' do
         get "/dois/#{doi.doi}"