From 99d38c8990e7af2e1dd14931c017085ac98f8c42 Mon Sep 17 00:00:00 2001 From: Richard Hallett Date: Fri, 19 Oct 2018 15:12:11 +0200 Subject: [PATCH 1/7] Only return landing page results for authenticated --- app/controllers/dois_controller.rb | 1 + app/serializers/doi_serializer.rb | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/app/controllers/dois_controller.rb b/app/controllers/dois_controller.rb index 2a1a75ea3..522d8d622 100644 --- a/app/controllers/dois_controller.rb +++ b/app/controllers/dois_controller.rb @@ -97,6 +97,7 @@ def index }.compact options[:include] = @include options[:is_collection] = true + options[:is_authenticated] = !!@dois.current_user render json: DoiSerializer.new(@dois, options).serialized_json, status: :ok end diff --git a/app/serializers/doi_serializer.rb b/app/serializers/doi_serializer.rb index b08309a41..952ed4944 100644 --- a/app/serializers/doi_serializer.rb +++ b/app/serializers/doi_serializer.rb @@ -42,6 +42,10 @@ class DoiSerializer object.xml_encoded end + attribute :landing_page, if: Proc.new { |object, params| + params && params[:is_authenticated] == true + } + attribute :landing_page do |object| { status: object.last_landing_page_status, "content-type" => object.last_landing_page_content_type, From 7769b79a40c706b611c56a55cacd0550c364d24e Mon Sep 17 00:00:00 2001 From: Richard Hallett Date: Thu, 25 Oct 2018 14:15:09 +0200 Subject: [PATCH 2/7] Use abilities to define access to viewing landing page results --- .ruby-version | 1 + app/controllers/dois_controller.rb | 13 ++++- app/models/ability.rb | 14 ++++- app/serializers/doi_serializer.rb | 7 +-- spec/requests/dois_spec.rb | 90 +++++++++++++++++++++++++++++- 5 files changed, 115 insertions(+), 10 deletions(-) create mode 100644 .ruby-version diff --git a/.ruby-version b/.ruby-version new file mode 100644 index 000000000..0bee604df --- /dev/null +++ b/.ruby-version @@ -0,0 +1 @@ +2.3.3 diff --git a/app/controllers/dois_controller.rb b/app/controllers/dois_controller.rb index 522d8d622..3c4e1b8b6 100644 --- a/app/controllers/dois_controller.rb +++ b/app/controllers/dois_controller.rb @@ -97,7 +97,9 @@ def index }.compact options[:include] = @include options[:is_collection] = true - options[:is_authenticated] = !!@dois.current_user + options[:params] = { + :read_landing_page_results => current_ability.can?(:read_landing_page_results, @doi) + } render json: DoiSerializer.new(@dois, options).serialized_json, status: :ok end @@ -108,6 +110,9 @@ def show options = {} options[:include] = @include options[:is_collection] = false + options[:params] = { + :read_landing_page_results => current_ability.can?(:read_landing_page_results, @doi) + } render json: DoiSerializer.new(@doi, options).serialized_json, status: :ok end @@ -149,6 +154,9 @@ def create options = {} options[:include] = @include options[:is_collection] = false + options[:params] = { + :read_landing_page_results => current_ability.can?(:read_landing_page_results, @doi) + } render json: DoiSerializer.new(@doi, options).serialized_json, status: :created, location: @doi else @@ -190,6 +198,9 @@ def update options = {} options[:include] = @include options[:is_collection] = false + options[:params] = { + :read_landing_page_results => current_ability.can?(:read_landing_page_results, @doi) + } render json: DoiSerializer.new(@doi, options).serialized_json, status: exists ? :ok : :created else diff --git a/app/models/ability.rb b/app/models/ability.rb index 0607f995a..ef0acdd7c 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -16,8 +16,10 @@ def initialize(user) cannot [:new, :create], Doi do |doi| doi.client.blank? || !doi.client.prefixes.where(prefix: doi.prefix).first end + can [:read_landing_page_results], Doi elsif user.role_id == "staff_user" can :read, :all + can [:read_landing_page_results], Doi elsif user.role_id == "provider_admin" && user.provider_id.present? can [:update, :read], Provider, :symbol => user.provider_id.upcase can [:manage], ProviderPrefix, :provider_id => user.provider_id @@ -30,22 +32,24 @@ def initialize(user) # can [:read, :update], Doi, :provider_id => user.provider_id # end can [:read, :transfer], Doi, :provider_id => user.provider_id - can [:read], Doi do |doi| + can [:read], Doi do |doi| doi.findable? end can [:read], User can [:read], Phrase + can [:read_landing_page_results], Doi, :provider_id => user.provider_id elsif user.role_id == "provider_user" && user.provider_id.present? can [:read], Provider, :symbol => user.provider_id.upcase can [:read], ProviderPrefix, :provider_id => user.provider_id can [:read], Client, :provider_id => user.provider_id can [:read], ClientPrefix#, :client_id => user.client_id can [:read], Doi, :provider_id => user.provider_id - can [:read], Doi do |doi| + can [:read], Doi do |doi| doi.findable? end can [:read], User can [:read], Phrase + can [:read_landing_page_results], Doi, :provider_id => user.provider_id elsif user.role_id == "client_admin" && user.client_id.present? can [:read, :update], Client, :symbol => user.client_id.upcase can [:read], ClientPrefix, :client_id => user.client_id @@ -64,6 +68,7 @@ def initialize(user) end can [:read], User can [:read], Phrase + can [:read_landing_page_results], Doi, :client_id => user.client_id elsif user.role_id == "client_user" && user.client_id.present? can [:read], Client, :symbol => user.client_id.upcase can [:read], ClientPrefix, :client_id => user.client_id @@ -73,19 +78,22 @@ def initialize(user) end can [:read], User can [:read], Phrase + can [:read_landing_page_results], Doi, :client_id => user.client_id elsif user.role_id == "user" can [:read, :update], Provider, :symbol => user.provider_id.upcase if user.provider_id.present? can [:read, :update], Client, :symbol => user.client_id.upcase if user.client_id.present? can [:read], Doi, :client_id => user.client_id if user.client_id.present? - can [:read], Doi do |doi| + can [:read], Doi do |doi| doi.findable? end can [:read], User, :id => user.id can [:read], Phrase + can [:read_landing_page_results], Doi, :client_id => user.client_id if user.client_id.present? elsif user.role_id == "anonymous" can [:read], Doi do |doi| doi.findable? end + cannot [:read_landing_page_results], Doi end end end diff --git a/app/serializers/doi_serializer.rb b/app/serializers/doi_serializer.rb index 952ed4944..5e8693ee1 100644 --- a/app/serializers/doi_serializer.rb +++ b/app/serializers/doi_serializer.rb @@ -42,14 +42,11 @@ class DoiSerializer object.xml_encoded end - attribute :landing_page, if: Proc.new { |object, params| - params && params[:is_authenticated] == true - } - - attribute :landing_page do |object| + attribute :landing_page, if: Proc.new { |object, params| params && params[:read_landing_page_results] == true} do |object| { status: object.last_landing_page_status, "content-type" => object.last_landing_page_content_type, checked: object.last_landing_page_status_check, "result" => object.try(:last_landing_page_status_result) } end + end diff --git a/spec/requests/dois_spec.rb b/spec/requests/dois_spec.rb index d008c851d..5acc805a3 100644 --- a/spec/requests/dois_spec.rb +++ b/spec/requests/dois_spec.rb @@ -1687,7 +1687,7 @@ expect(json.dig('data', 'attributes', 'state')).to eq("registered") end end - + context 'landing page schema-org-id hash' do let(:url) { "https://blog.datacite.org/re3data-science-europe/" } let(:xml) { "PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iVVRGLTgiPz48cmVzb3VyY2UgeG1sbnM6eHNpPSJodHRwOi8vd3d3LnczLm9yZy8yMDAxL1hNTFNjaGVtYS1pbnN0YW5jZSIgeG1sbnM9Imh0dHA6Ly9kYXRhY2l0ZS5vcmcvc2NoZW1hL2tlcm5lbC00IiB4c2k6c2NoZW1hTG9jYXRpb249Imh0dHA6Ly9kYXRhY2l0ZS5vcmcvc2NoZW1hL2tlcm5lbC00IGh0dHA6Ly9zY2hlbWEuZGF0YWNpdGUub3JnL21ldGEva2VybmVsLTQvbWV0YWRhdGEueHNkIj48aWRlbnRpZmllciBpZGVudGlmaWVyVHlwZT0iRE9JIj4xMC4yNTQ5OS94dWRhMnB6cmFocm9lcXBlZnZucTV6dDZkYzwvaWRlbnRpZmllcj48Y3JlYXRvcnM+PGNyZWF0b3I+PGNyZWF0b3JOYW1lPklhbiBQYXJyeTwvY3JlYXRvck5hbWU+PG5hbWVJZGVudGlmaWVyIHNjaGVtZVVSST0iaHR0cDovL29yY2lkLm9yZy8iIG5hbWVJZGVudGlmaWVyU2NoZW1lPSJPUkNJRCI+MDAwMC0wMDAxLTYyMDItNTEzWDwvbmFtZUlkZW50aWZpZXI+PC9jcmVhdG9yPjwvY3JlYXRvcnM+PHRpdGxlcz48dGl0bGU+U3VibWl0dGVkIGNoZW1pY2FsIGRhdGEgZm9yIEluQ2hJS2V5PVlBUFFCWFFZTEpSWFNBLVVIRkZGQU9ZU0EtTjwvdGl0bGU+PC90aXRsZXM+PHB1Ymxpc2hlcj5Sb3lhbCBTb2NpZXR5IG9mIENoZW1pc3RyeTwvcHVibGlzaGVyPjxwdWJsaWNhdGlvblllYXI+MjAxNzwvcHVibGljYXRpb25ZZWFyPjxyZXNvdXJjZVR5cGUgcmVzb3VyY2VUeXBlR2VuZXJhbD0iRGF0YXNldCI+U3Vic3RhbmNlPC9yZXNvdXJjZVR5cGU+PHJpZ2h0c0xpc3Q+PHJpZ2h0cyByaWdodHNVUkk9Imh0dHBzOi8vY3JlYXRpdmVjb21tb25zLm9yZy9zaGFyZS15b3VyLXdvcmsvcHVibGljLWRvbWFpbi9jYzAvIj5ObyBSaWdodHMgUmVzZXJ2ZWQ8L3JpZ2h0cz48L3JpZ2h0c0xpc3Q+PC9yZXNvdXJjZT4=" } @@ -1853,6 +1853,94 @@ end end + describe 'GET /dois/ linkcheck results' do + let(:last_landing_page_status_result) { { + "error" => nil, + "redirect-count" => 0, + "redirect-urls" => [], + "download-latency" => 200, + "has-schema-org" => true, + "schema-org-id" => "10.14454/10703", + "dc-identifier" => nil, + "citation-doi" => nil, + "body-has-pid" => true + } } + + # Setup an initial DOI with results will check permissions against. + let(:doi) { + create( + :doi, doi: "10.24425/2210181332", + client: client, + state: "findable", + event: 'publish', + last_landing_page_status_result: last_landing_page_status_result + ) + } + + # Create a different dummy client and a doi with entry associated + # This is so we can test clients accessing others information + let(:other_client) { create(:client, provider: provider, symbol: 'DATACITE.DOESNTEXIST', password: 'notarealpassword') } + let(:other_doi) { + create( + :doi, doi: "10.24425/2210181332", + client: other_client, + state: "findable", + event: 'publish', + last_landing_page_status_result: last_landing_page_status_result + ) + } + + context 'anonymous get' do + let(:headers) { { 'ACCEPT'=>'application/vnd.api+json', 'CONTENT_TYPE'=>'application/vnd.api+json' } } + before { get "/dois/#{doi.doi}", headers: headers} + + it 'returns without link_check_results' do + puts json + expect(json.dig('data', 'attributes', 'doi')).to eq(doi.doi) + expect(json.dig('data', 'attributes', 'landing-page', 'result')).to eq(nil) + end + end + + context 'client authorised get own dois' do + let(:bearer) { User.generate_token(role_id: "client_admin", client_id: client.symbol.downcase) } + let(:headers) { { 'ACCEPT'=>'application/vnd.api+json', 'CONTENT_TYPE'=>'application/vnd.api+json', 'Authorization' => 'Bearer ' + bearer } } + + before { get "/dois/#{doi.doi}", headers: headers } + + it 'returns with link_check_results' do + expect(json.dig('data', 'attributes', 'doi')).to eq(doi.doi) + expect(json.dig('data', 'attributes', 'landing-page', 'result')).to eq(last_landing_page_status_result) + end + end + + + context 'client authorised try get diff dois landing data' do + let(:bearer) { User.generate_token(role_id: "client_admin", client_id: client.symbol.downcase) } + let(:headers) { { 'ACCEPT'=>'application/vnd.api+json', 'CONTENT_TYPE'=>'application/vnd.api+json', 'Authorization' => 'Bearer ' + bearer } } + + before { get "/dois/#{other_doi.doi}", headers: headers } + + it 'returns with link_check_results' do + expect(json.dig('data', 'attributes', 'doi')).to eq(other_doi.doi) + expect(json.dig('data', 'attributes', 'landing-page', 'result')).to eq(nil) + end + end + + + context 'authorised staff admin read' do + let(:bearer) { User.generate_token(role_id: "client_admin", client_id: client.symbol.downcase) } + let(:headers) { { 'ACCEPT'=>'application/vnd.api+json', 'CONTENT_TYPE'=>'application/vnd.api+json', 'Authorization' => 'Bearer ' + admin_bearer } } + + before { get "/dois/#{doi.doi}", headers: headers } + + it 'returns with link_check_results' do + expect(json.dig('data', 'attributes', 'doi')).to eq(doi.doi) + expect(json.dig('data', 'attributes', 'landing-page', 'result')).to eq(last_landing_page_status_result) + end + end + + end + describe 'GET /dois/random?prefix' do before { get "/dois/random?prefix=#{prefix.prefix}", headers: headers } From c2b70c7757381a52284f3e3f210c06afe9645659 Mon Sep 17 00:00:00 2001 From: Richard Hallett Date: Tue, 30 Oct 2018 10:26:43 +0100 Subject: [PATCH 3/7] Remove .ruby-version - accidental commit --- .ruby-version | 1 - 1 file changed, 1 deletion(-) delete mode 100644 .ruby-version diff --git a/.ruby-version b/.ruby-version deleted file mode 100644 index 0bee604df..000000000 --- a/.ruby-version +++ /dev/null @@ -1 +0,0 @@ -2.3.3 From 0e8ce766cafcd39753df192b22ca6e66d84bbda0 Mon Sep 17 00:00:00 2001 From: Richard Hallett Date: Tue, 30 Oct 2018 10:27:03 +0100 Subject: [PATCH 4/7] Ignore rbenv file --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index b08eac57a..9fcfc5cca 100644 --- a/.gitignore +++ b/.gitignore @@ -52,3 +52,4 @@ doc/dependencies* !.env.example !.env.travis docker-compose.override.yml +.ruby-version \ No newline at end of file From 17c891fa74ff42b61913ac27cbbc89912918beac Mon Sep 17 00:00:00 2001 From: Richard Hallett Date: Tue, 30 Oct 2018 12:27:47 +0100 Subject: [PATCH 5/7] Pass ability to serialiser to do tests there --- app/controllers/dois_controller.rb | 14 ++++++++++---- app/serializers/doi_serializer.rb | 5 ++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/app/controllers/dois_controller.rb b/app/controllers/dois_controller.rb index 51c363b21..a98a4107f 100644 --- a/app/controllers/dois_controller.rb +++ b/app/controllers/dois_controller.rb @@ -74,6 +74,9 @@ def index }.compact options[:include] = @include options[:is_collection] = true + options[:params] = { + :current_ability => current_ability, + } render json: DoiSerializer.new(@dois, options).serialized_json, status: :ok else @@ -169,7 +172,7 @@ def index options[:include] = @include options[:is_collection] = true options[:params] = { - :read_landing_page_results => current_ability.can?(:read_landing_page_results, @doi) + :current_ability => current_ability, } render json: DoiSerializer.new(@dois, options).serialized_json, status: :ok @@ -183,7 +186,7 @@ def show options[:include] = @include options[:is_collection] = false options[:params] = { - :read_landing_page_results => current_ability.can?(:read_landing_page_results, @doi) + :current_ability => current_ability, } render json: DoiSerializer.new(@doi, options).serialized_json, status: :ok @@ -205,6 +208,9 @@ def validate options = {} options[:include] = @include options[:is_collection] = false + options[:params] = { + :current_ability => current_ability, + } render json: DoiSerializer.new(@doi, options).serialized_json, status: :ok end @@ -227,7 +233,7 @@ def create options[:include] = @include options[:is_collection] = false options[:params] = { - :read_landing_page_results => current_ability.can?(:read_landing_page_results, @doi) + :current_ability => current_ability, } render json: DoiSerializer.new(@doi, options).serialized_json, status: :created, location: @doi @@ -271,7 +277,7 @@ def update options[:include] = @include options[:is_collection] = false options[:params] = { - :read_landing_page_results => current_ability.can?(:read_landing_page_results, @doi) + :current_ability => current_ability, } render json: DoiSerializer.new(@doi, options).serialized_json, status: exists ? :ok : :created diff --git a/app/serializers/doi_serializer.rb b/app/serializers/doi_serializer.rb index 5e8693ee1..0fea3a694 100644 --- a/app/serializers/doi_serializer.rb +++ b/app/serializers/doi_serializer.rb @@ -42,7 +42,10 @@ class DoiSerializer object.xml_encoded end - attribute :landing_page, if: Proc.new { |object, params| params && params[:read_landing_page_results] == true} do |object| + attribute :landing_page, if: Proc.new { + |object, params| + params && params[:current_ability].can?(:read_landing_page_results, object) == true + } do |object| { status: object.last_landing_page_status, "content-type" => object.last_landing_page_content_type, checked: object.last_landing_page_status_check, From 6d62874f20bd25d8bc65a5ff3ae6cc1e283a3d54 Mon Sep 17 00:00:00 2001 From: Richard Hallett Date: Tue, 30 Oct 2018 12:31:00 +0100 Subject: [PATCH 6/7] Update permissions for landing results --- app/models/ability.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index ef0acdd7c..fbbccc6a8 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -88,12 +88,10 @@ def initialize(user) end can [:read], User, :id => user.id can [:read], Phrase - can [:read_landing_page_results], Doi, :client_id => user.client_id if user.client_id.present? elsif user.role_id == "anonymous" can [:read], Doi do |doi| doi.findable? end - cannot [:read_landing_page_results], Doi end end end From 137e38ec555acefc6436c4fd4dc650ca8107155f Mon Sep 17 00:00:00 2001 From: Richard Hallett Date: Tue, 30 Oct 2018 12:48:21 +0100 Subject: [PATCH 7/7] Only check ability if we have it passed in --- app/serializers/doi_serializer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/serializers/doi_serializer.rb b/app/serializers/doi_serializer.rb index 0fea3a694..880473178 100644 --- a/app/serializers/doi_serializer.rb +++ b/app/serializers/doi_serializer.rb @@ -44,7 +44,7 @@ class DoiSerializer attribute :landing_page, if: Proc.new { |object, params| - params && params[:current_ability].can?(:read_landing_page_results, object) == true + params[:current_ability] && params[:current_ability].can?(:read_landing_page_results, object) == true } do |object| { status: object.last_landing_page_status, "content-type" => object.last_landing_page_content_type,