uclibs / ucrate

Scholar@UC: University of Cincinnati's self-submission institutional repository
https://scholar.uc.edu
Other
5 stars 3 forks source link

Warn 993 fix distinct error sql vunlerability #1112

Closed Janell-Huyck closed 8 months ago

Janell-Huyck commented 8 months ago

Fixes #993

Fixes the warning for DISTINCT error sql vulnerability

The origin of the DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s): "DISTINCT agent_id". warning was inside the hyrax gem file . In there, we had some SQL queries similar to this:

Hyrax::CollectionTypeParticipant.where(hyrax_collection_type_id: collection_type.id,
                                               agent_type: agent_type,
                                               access: access).pluck('DISTINCT agent_id')

The Rails 5 and earlier way of fixing this is to add Arel.sql so that you have this query:

Hyrax::CollectionTypeParticipant.where(hyrax_collection_type_id: collection_type.id,
                                               agent_type: agent_type,
                                               access: access).pluck(Arel.sql('DISTINCT agent_id'))

Using Arel.sql like that tells the app that what we're looking for with our query is a safe thing to look for and that it can just proceed without further checks.

In Rails 6.0+, the Arel.sq support is removed, and we now need to have the queries written in a chain like this:

        Hyrax::CollectionTypeParticipant.where(hyrax_collection_type_id: collection_type.id,
                                               agent_type: agent_type,
                                               access: access)
                                        .select(:agent_id)
                                        .distinct
                                        .pluck(:agent_id)

This PR fixes the "DISTINCT" warning and updates the rest of the methods in the file to use the Rails 6.0+ friendly query style. It overwrites the entire file and includes the gem's original spec file to test that our methods and new queries are working correctly.

Modifying the queries made the file a few lines longer, and it extends past the allowed # of lines for the file (111 lines instead of 100). We decided to add this file to the rubocop.yml list to ignore the length of the file.