apsislabs / phi_attrs

HIPAA compliant PHI access logging for Ruby on Rails.
MIT License
27 stars 3 forks source link

Add shortcut for Rails controllers #4

Closed wkirby closed 5 years ago

wkirby commented 6 years ago

In the context of a rails controller, we can provide a shorthand, so that we don't have to pass in an identifier or a reason. We can assume a current_user and an identification method (probably default to :email) and allow configuration of these.

For the reason, we can do something like:

def current_request
    "#{request.request_method} #{controller_name}::#{action_name}"
  end

This will give the reason as something like GET application_controller::index.

Crisfole commented 5 years ago

I assume the syntax for this would be something like:

allow_phi! PatientInformation

or

allow_phi! @patient_information

Where the controller then calls allow_phi! on the target?

wkirby commented 5 years ago

Yeah. The work here is to basically set some assumptions about how we would get the current user and reason information. We could basically expect a current_user.email method to exist, and define a current_phi_reason on the ApplicationController. This would give the end user an easy way to override these values, and let us shortcut in an ActionController context.

HenryKeiter commented 5 years ago

I think it might be more valuable to, rather than adding a specific default, add a helper that defines a couple of dummy default methods and let the user override them if they want a default. Then we hook up allow_phi! to look for defaults here if it receives any blank values. For example:

# You can redefine these methods to provide sensible defaults.
# For example:
#   include AuthenticationHelper # (your own implementation)
#   def default_phi_user
#     current_user.email
#   end
#
# This way you don't have to provide the same values all over your codebase.
#
module PhiAttrsHelper
  def default_phi_user
    nil
  end

  def default_phi_reason
    nil
  end
end

What do folks think about an approach like this?

egreer commented 5 years ago

Fixed in: https://github.com/apsislabs/phi_attrs/pull/34 https://github.com/apsislabs/phi_attrs/pull/37