From 0fada75c86b2c9b5f229d893d987dbfc5feab01f Mon Sep 17 00:00:00 2001 From: Syphax bouazzouni Date: Wed, 24 Apr 2024 02:55:48 +0200 Subject: [PATCH 1/3] Feature: Add accessibility security to ontology metadata & content search results (#74) * add ontology accessibility restriction to ontology metadata search * add ontology accessibility restriction to ontology content search * add search results accessibility security test --- controllers/search_controller.rb | 32 +++++++-- .../test_search_models_controller.rb | 71 +++++++++++++++++++ 2 files changed, 99 insertions(+), 4 deletions(-) diff --git a/controllers/search_controller.rb b/controllers/search_controller.rb index 9a354f08..9f701714 100644 --- a/controllers/search_controller.rb +++ b/controllers/search_controller.rb @@ -22,7 +22,7 @@ class SearchController < ApplicationController format = params.fetch("hasOntologyLanguage", "").split(',') is_of_type = params.fetch("isOfType", "").split(',') has_format = params.fetch("hasFormat", "").split(',') - visibility = params["visibility"]&.presence || "public" + visibility = params["visibility"] show_views = params["show_views"] == 'true' sort = params.fetch("sort", "score desc, ontology_name_sort asc, ontology_acronym_sort asc") page, page_size = page_params @@ -30,12 +30,12 @@ class SearchController < ApplicationController fq = [ 'resource_model:"ontology_submission"', 'submissionStatus_txt:ERROR_* OR submissionStatus_txt:"RDF" OR submissionStatus_txt:"UPLOADED"', - "ontology_viewingRestriction_t:#{visibility}", groups.map { |x| "ontology_group_txt:\"http://data.bioontology.org/groups/#{x.upcase}\"" }.join(' OR '), categories.map { |x| "ontology_hasDomain_txt:\"http://data.bioontology.org/categories/#{x.upcase}\"" }.join(' OR '), languages.map { |x| "naturalLanguage_txt:\"#{x.downcase}\"" }.join(' OR '), ] + fq << "ontology_viewingRestriction_t:#{visibility}" unless visibility.blank? fq << "!ontology_viewOf_t:*" unless show_views fq << format.map { |x| "hasOntologyLanguage_t:\"http://data.bioontology.org/ontology_formats/#{x}\"" }.join(' OR ') unless format.blank? @@ -75,7 +75,15 @@ class SearchController < ApplicationController old_resource_id = acronyms_ids[acronym] old_id = old_resource_id.split('/').last.to_i rescue 0 - if acronym.blank? || old_id && id && (id <= old_id) + already_found = (old_id && id && (id <= old_id)) + not_restricted = (doc["ontology_viewingRestriction_t"]&.eql?('public') || current_user&.admin?) + user_not_restricted = not_restricted || + Array(doc["ontology_viewingRestriction_txt"]).any? {|u| u.split(' ').last == current_user&.username} || + Array(doc["ontology_acl_txt"]).any? {|u| u.split(' ').last == current_user&.username} + + user_restricted = !user_not_restricted + + if acronym.blank? || already_found || user_restricted total_found -= 1 next end @@ -99,10 +107,26 @@ class SearchController < ApplicationController get '/content' do query = params[:query] || params[:q] page, page_size = page_params + ontologies = params.fetch("ontologies", "").split(',') + + unless current_user&.admin? + restricted_acronyms = restricted_ontologies_to_acronyms(params) + ontologies = ontologies.empty? ? restricted_acronyms : ontologies & restricted_acronyms + end + + types = params.fetch("types", "").split(',') qf = params.fetch("qf", "") + qf = [ + "ontology_t^100 resource_id^10", + "http___www.w3.org_2004_02_skos_core_prefLabel_txt^30", + "http___www.w3.org_2004_02_skos_core_prefLabel_t^30", + "http___www.w3.org_2000_01_rdf-schema_label_txt^30", + "http___www.w3.org_2000_01_rdf-schema_label_t^30", + ].join(' ') if qf.blank? + fq = [] fq << ontologies.map { |x| "ontology_t:\"#{x}\"" }.join(' OR ') unless ontologies.blank? @@ -117,7 +141,7 @@ class SearchController < ApplicationController docs = resp["response"]["docs"] - reply 200,page_object(docs, total_found) + reply 200, page_object(docs, total_found) end end diff --git a/test/controllers/test_search_models_controller.rb b/test/controllers/test_search_models_controller.rb index 851c7a31..6b9cef29 100644 --- a/test/controllers/test_search_models_controller.rb +++ b/test/controllers/test_search_models_controller.rb @@ -55,6 +55,77 @@ def test_collection_search assert_equal 2, res['response']['numFound'] end + def test_search_security + count, acronyms, bro = LinkedData::SampleData::Ontology.create_ontologies_and_submissions({ + process_submission: true, + process_options: { process_rdf: true, extract_metadata: false, generate_missing_labels: false}, + acronym: "BROSEARCHTEST", + name: "BRO Search Test", + file_path: "./test/data/ontology_files/BRO_v3.2.owl", + ont_count: 1, + submission_count: 1, + ontology_type: "VALUE_SET_COLLECTION" + }) + + count, acronyms, mccl = LinkedData::SampleData::Ontology.create_ontologies_and_submissions({ + process_submission: true, + process_options: { process_rdf: true, extract_metadata: false, generate_missing_labels: false}, + acronym: "MCCLSEARCHTEST", + name: "MCCL Search Test", + file_path: "./test/data/ontology_files/CellLine_OWL_BioPortal_v1.0.owl", + ont_count: 1, + submission_count: 1 + }) + + + subs = LinkedData::Models::OntologySubmission.all + subs.each do |s| + s.bring_remaining + s.index_all_data(Logger.new($stdout)) + end + + + allowed_user = User.new({ + username: "allowed", + email: "test1@example.org", + password: "12345" + }) + allowed_user.save + + blocked_user = User.new({ + username: "blocked", + email: "test2@example.org", + password: "12345" + }) + blocked_user.save + + bro = bro.first + bro.bring_remaining + bro.acl = [allowed_user] + bro.viewingRestriction = "private" + bro.save + + self.class.enable_security + get "/search/ontologies?query=#{bro.acronym}&apikey=#{blocked_user.apikey}" + response = MultiJson.load(last_response.body)["collection"] + assert_empty response.select{|x| x["ontology_acronym_text"].eql?(bro.acronym)} + + get "/search/ontologies/content?q=*Research_Lab_Management*&apikey=#{blocked_user.apikey}" + assert last_response.ok? + res = MultiJson.load(last_response.body) + assert_equal 0, res['totalCount'] + + get "/search/ontologies?query=#{bro.acronym}&apikey=#{allowed_user.apikey}" + response = MultiJson.load(last_response.body)["collection"] + refute_empty response.select{|x| x["ontology_acronym_text"].eql?(bro.acronym)} + + get "/search/ontologies/content?q=*Research_Lab_Management*&apikey=#{allowed_user.apikey}" + assert last_response.ok? + res = MultiJson.load(last_response.body) + assert_equal 1, res['totalCount'] + + self.class.reset_security(false) + end def test_ontology_metadata_search count, acronyms, bro = LinkedData::SampleData::Ontology.create_ontologies_and_submissions({ From 73309f605a55051314eb248738e0ef85b500997e Mon Sep 17 00:00:00 2001 From: Syphax bouazzouni Date: Mon, 29 Apr 2024 08:30:04 +0200 Subject: [PATCH 2/3] fix: enable user creation notification (#76) --- controllers/users_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/users_controller.rb b/controllers/users_controller.rb index d647c9dc..58e7667f 100644 --- a/controllers/users_controller.rb +++ b/controllers/users_controller.rb @@ -99,14 +99,14 @@ class UsersController < ApplicationController private - def create_user + def create_user(send_notifications: true) params ||= @params user = User.find(params["username"]).first error 409, "User with username `#{params["username"]}` already exists" unless user.nil? params.delete("role") unless current_user.admin? user = instance_from_params(User, params) if user.valid? - user.save(send_notifications: false) + user.save(send_notifications: send_notifications) else error 422, user.errors end From 523009140494f031b5e33dc1aa52b01c453dae9d Mon Sep 17 00:00:00 2001 From: Syphax bouazzouni Date: Wed, 1 May 2024 20:34:49 +0200 Subject: [PATCH 3/3] Fix: Invalidating cache on insert & fix Redis warning (#77) * Merge pull request https://github.com/ncbo/ontologies_api/pull/120from ncbo/remove_redis-activesupport Remove redis activesupport * use the branch development of sparql client --- Gemfile | 7 ++- Gemfile.lock | 45 +++++++++---------- config/rack_attack.rb | 7 ++- .../test_search_models_controller.rb | 11 +++-- 4 files changed, 33 insertions(+), 37 deletions(-) diff --git a/Gemfile b/Gemfile index a1f48928..bd445a1e 100644 --- a/Gemfile +++ b/Gemfile @@ -30,9 +30,8 @@ gem 'rack-timeout' gem 'redis-rack-cache', '~> 2.0' # Data access (caching) -gem 'redis', '~> 4.8.1' -gem 'redis-activesupport' -gem 'redis-store', '1.9.1' +gem 'redis' +gem 'redis-store', '~>1.10' # Monitoring gem 'cube-ruby', require: 'cube' @@ -50,8 +49,8 @@ gem 'redcarpet' gem 'ncbo_annotator', git: 'https://github.com/ontoportal-lirmm/ncbo_annotator.git', branch: 'development' gem 'ncbo_cron', git: 'https://github.com/ontoportal-lirmm/ncbo_cron.git', branch: 'master' gem 'ncbo_ontology_recommender', git: 'https://github.com/ncbo/ncbo_ontology_recommender.git', branch: 'master' -gem 'sparql-client', github: 'ontoportal-lirmm/sparql-client', branch: 'master' gem 'goo', github: 'ontoportal-lirmm/goo', branch: 'development' +gem 'sparql-client', github: 'ontoportal-lirmm/sparql-client', branch: 'development' gem 'ontologies_linked_data', git: 'https://github.com/ontoportal-lirmm/ontologies_linked_data.git', branch: 'development' group :development do diff --git a/Gemfile.lock b/Gemfile.lock index e1fe41a7..9984c044 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -11,7 +11,7 @@ GIT GIT remote: https://github.com/ontoportal-lirmm/goo.git - revision: 0e554fce49713ce4d5a742a06c2fb59a547caf47 + revision: b2a635fb1e8206e6e3010be4dbe033b47eb58481 branch: development specs: goo (0.0.2) @@ -57,7 +57,7 @@ GIT GIT remote: https://github.com/ontoportal-lirmm/ontologies_linked_data.git - revision: 026c9c46baf6c0d638568528c2adcb7bcb1c2796 + revision: 1278507e6ae8224edca746c7c150775ec2210195 branch: development specs: ontologies_linked_data (0.0.1) @@ -77,13 +77,12 @@ GIT GIT remote: https://github.com/ontoportal-lirmm/sparql-client.git - revision: 180c818f7715baac64b2699bb452ef5c756f62c5 - branch: master + revision: 82302db1bfaec6593f3ea26917d7f2bb2dd485ce + branch: development specs: - sparql-client (1.0.1) - json_pure (>= 1.4) - net-http-persistent (= 2.9.4) - rdf (>= 1.0) + sparql-client (3.2.2) + net-http-persistent (~> 4.0, >= 4.0.2) + rdf (~> 3.2, >= 3.2.11) GIT remote: https://github.com/palexander/rack-post-body-to-params.git @@ -109,7 +108,7 @@ GEM multi_json (~> 1.0) addressable (2.8.6) public_suffix (>= 2.0.2, < 6.0) - airbrussh (1.5.1) + airbrussh (1.5.2) sshkit (>= 1.6.1, != 1.7.0) backports (3.25.0) base64 (0.2.0) @@ -131,6 +130,7 @@ GEM sshkit (~> 1.3) coderay (1.1.3) concurrent-ruby (1.2.3) + connection_pool (2.4.1) crack (1.0.0) bigdecimal rexml @@ -235,7 +235,6 @@ GEM rdf (>= 2.2.8, < 4.0) json-schema (2.8.1) addressable (>= 2.4) - json_pure (2.7.2) jwt (2.8.1) base64 kgio (2.11.4) @@ -249,7 +248,7 @@ GEM net-imap net-pop net-smtp - method_source (1.0.0) + method_source (1.1.0) mime-types (3.5.2) mime-types-data (~> 3.2015) mime-types-data (3.2024.0305) @@ -261,7 +260,8 @@ GEM multi_json (1.15.0) multipart-post (2.4.0) mutex_m (0.2.0) - net-http-persistent (2.9.4) + net-http-persistent (4.0.2) + connection_pool (~> 2.2) net-imap (0.4.10) date net-protocol @@ -277,7 +277,7 @@ GEM net-protocol net-ssh (7.2.3) netrc (0.11.0) - newrelic_rpm (9.8.0) + newrelic_rpm (9.9.0) oj (2.18.5) omni_logger (0.1.4) logger @@ -324,15 +324,15 @@ GEM rdf (~> 3.2) rexml (~> 3.2) redcarpet (3.6.0) - redis (4.8.1) - redis-activesupport (5.3.0) - activesupport (>= 3, < 8) - redis-store (>= 1.3, < 2) + redis (5.2.0) + redis-client (>= 0.22.0) + redis-client (0.22.1) + connection_pool redis-rack-cache (2.2.1) rack-cache (>= 1.10, < 2) redis-store (>= 1.6, < 2) - redis-store (1.9.1) - redis (>= 4, < 5) + redis-store (1.10.0) + redis (>= 4, < 6) representable (3.2.0) declarative (< 0.1.0) trailblazer-option (>= 0.1.1, < 0.2.0) @@ -381,7 +381,7 @@ GEM rack-test sinatra (~> 1.4.0) tilt (>= 1.3, < 3) - sshkit (1.22.1) + sshkit (1.22.2) base64 mutex_m net-scp (>= 1.1.2) @@ -451,10 +451,9 @@ DEPENDENCIES rack-timeout rake (~> 10.0) redcarpet - redis (~> 4.8.1) - redis-activesupport + redis redis-rack-cache (~> 2.0) - redis-store (= 1.9.1) + redis-store (~> 1.10) request_store shotgun! simplecov diff --git a/config/rack_attack.rb b/config/rack_attack.rb index 60d2e3de..88a3e8d6 100644 --- a/config/rack_attack.rb +++ b/config/rack_attack.rb @@ -3,15 +3,14 @@ puts "(API) >> Throttling enabled at #{LinkedData::OntologiesAPI.settings.req_per_second_per_ip} req/sec" require 'rack/attack' -require 'redis-activesupport' use Rack::Attack attack_redis_host_port = { host: LinkedData::OntologiesAPI.settings.http_redis_host, - port: LinkedData::OntologiesAPI.settings.http_redis_port + port: LinkedData::OntologiesAPI.settings.http_redis_port, + db: 1 } -attack_store = ActiveSupport::Cache::RedisStore.new(attack_redis_host_port) -Rack::Attack.cache.store = attack_store +Rack::Attack.cache.store = Redis.new(attack_redis_host_port) safe_ips = LinkedData::OntologiesAPI.settings.safe_ips ||= Set.new safe_ips.each do |safe_ip| diff --git a/test/controllers/test_search_models_controller.rb b/test/controllers/test_search_models_controller.rb index 6b9cef29..8f9a95fb 100644 --- a/test/controllers/test_search_models_controller.rb +++ b/test/controllers/test_search_models_controller.rb @@ -18,7 +18,8 @@ def test_show_all_collection get '/admin/search/collections' assert last_response.ok? res = MultiJson.load(last_response.body) - assert_equal res["collections"].sort, Goo.search_connections.keys.map(&:to_s).sort + array = %w[agents_metadata ontology_data ontology_metadata prop_search_core1 term_search_core1] + assert_equal res["collections"].sort , array.sort end def test_collection_schema @@ -81,7 +82,7 @@ def test_search_security subs = LinkedData::Models::OntologySubmission.all subs.each do |s| s.bring_remaining - s.index_all_data(Logger.new($stdout)) + s.index_all(Logger.new($stdout)) end @@ -416,7 +417,7 @@ def test_agents_search def test_search_data count, acronyms, bro = LinkedData::SampleData::Ontology.create_ontologies_and_submissions({ process_submission: true, - process_options: { process_rdf: true, extract_metadata: false, generate_missing_labels: false}, + process_options: { process_rdf: true, extract_metadata: false, index_all_data: true, generate_missing_labels: false}, acronym: "BROSEARCHTEST", name: "BRO Search Test", file_path: "./test/data/ontology_files/BRO_v3.2.owl", @@ -427,7 +428,7 @@ def test_search_data count, acronyms, mccl = LinkedData::SampleData::Ontology.create_ontologies_and_submissions({ process_submission: true, - process_options: { process_rdf: true, extract_metadata: false, generate_missing_labels: false}, + process_options: { process_rdf: true, extract_metadata: false, index_all_data: true, generate_missing_labels: false}, acronym: "MCCLSEARCHTEST", name: "MCCL Search Test", file_path: "./test/data/ontology_files/CellLine_OWL_BioPortal_v1.0.owl", @@ -439,8 +440,6 @@ def test_search_data subs = LinkedData::Models::OntologySubmission.all count = [] subs.each do |s| - s.bring_remaining - s.index_all_data(Logger.new($stdout)) count << Goo.sparql_query_client.query("SELECT (COUNT( DISTINCT ?id) as ?c) FROM <#{s.id}> WHERE {?id ?p ?v}") .first[:c] .to_i