authority_success logging

[forced-request]

This PR introduces the authority_success controller method, which can be utilized to log successful authorization events. This must be enabled within the configuration by setting config.log_success = true within the initializer.

This method will log via Authority.logger.info by default, but can be overloaded in the controller similar to how authority_forbidden is. As an example:

class ApplicationController < ActionController::Base
  # Send 'em back where they came from with a slap on the wrist
  def authority_forbidden(error)
    log_security_event("failure", nil, "Authority Failure")

    Authority.logger.warn(error.message)
    redirect_to request.referrer.presence || root_path, :alert => 'You are not authorized to complete that action.'
  end

  def authority_success(*opts)
    log_security_event("success", nil, "Authority Success")
  end
end

This may not be the best way to handle things. For instance, you may prefer that the authority_success action operating on an object similar to SecurityViolation. I didn't see the need for this for my use case.

What do you think?

[nathanl]

A couple of thoughts:

• We don't actually know that they successfully performed the action, only that they successfully made it through the authorization step. We should log something like "user was authorized to perform the #{action} to the resource #{resource}"
• Is there a good reason to make this a controller method, or a method the user can override? authority_forbidden renders 403 (a controller responsibility) and some people might want to show 404 or something else (so it probably needs to be overridable). But it seems like Authority.enforce could just say "if the log_attempts option is true, log the attempt."

[forced-request]

1. Fair point. I think the function name makes sense, since we successfully passed through authority.. but the default message should be changed. Will fix that.

2. For my personal need I have to overwrite it because I need to capture specific information in the request such as path details and controller methods. Also figured that if it were a controller method other users would see the need to extend it as well since it's similar to the authority_forbidden action. At first I did have this being logged via Authority.enforce, but overriding that was less clean than just making it a controller method.

Really up to you! It's easy to make the change, but I think it's more restrictive.

[nathanl]

Makes sense.

One other thought - if authority_success can be overridden to do anything the user wants, the config option log_success isn't necessarily an accurate name. What if we don't even have a config option, and authority_success has a default implementation that does nothing? Then the instructions are "if you want to log successful authorization or do anything else about it, define authority_success however you like."

[forced-request]

@nathanl I think what you said makes sense. I made those changes. It's much simpler now. I also removed the test coverage since the function doesn't actually do anything.

[nathanl]

👍