faaf2ec028
This commit does three things: 1. Documents a method 3. Implies the question: "Do we want to use class_attribute in Forem's codebase?" 2. Switches from an inferrence to an explicit (and configurable) In my experience, I want to favor "explicit" declarations instead of inferring what they should be. In this case, the inferrence is perhaps adequate. But as I look to `ApplicationController::PUBLIC_CONTROLLERS`, I think that is a prime case for a `class_attribute`. (The `api_action` happened to be the lowest hanging fruit to begin the conversation.) We still need some clarity into the `verify_private_forem` method as it looks like it's doing a few different things. There is precedence for using `class_attribute` found in [`UniqueCrossModelSlugValidator.model_and_attribute_name_for_uniqueness_test`][1] (also introduced by me). [1]:https://github.com/forem/forem/blob/main/app/validators/unique_cross_model_slug_validator.rb
116 lines
4.3 KiB
Ruby
116 lines
4.3 KiB
Ruby
module Api
|
|
module V0
|
|
class ApiController < ApplicationController
|
|
protect_from_forgery with: :exception, prepend: true
|
|
|
|
include ValidRequest
|
|
|
|
respond_to :json
|
|
|
|
# Informs the application that all actions taking by this controller (and it's subclasses) are
|
|
# considered an api_action.
|
|
self.api_action = true
|
|
|
|
rescue_from ActionController::ParameterMissing do |exc|
|
|
error_unprocessable_entity(exc.message)
|
|
end
|
|
|
|
rescue_from ActiveRecord::RecordInvalid do |exc|
|
|
error_unprocessable_entity(exc.message)
|
|
end
|
|
|
|
rescue_from ActiveRecord::RecordNotFound, with: :error_not_found
|
|
|
|
rescue_from Pundit::NotAuthorizedError, with: :error_unauthorized
|
|
|
|
protected
|
|
|
|
def error_unprocessable_entity(message)
|
|
render json: { error: message, status: 422 }, status: :unprocessable_entity
|
|
end
|
|
|
|
def error_unauthorized
|
|
render json: { error: "unauthorized", status: 401 }, status: :unauthorized
|
|
end
|
|
|
|
def error_not_found
|
|
render json: { error: "not found", status: 404 }, status: :not_found
|
|
end
|
|
|
|
# @note This method is performing both authentication and authorization. The user suspended
|
|
# should be something added to the corresponding pundit policy.
|
|
def authenticate!
|
|
user = authenticate_with_api_key_or_current_user
|
|
return error_unauthorized unless user
|
|
return error_unauthorized if @user.suspended?
|
|
|
|
true
|
|
end
|
|
|
|
def authorize_super_admin
|
|
error_unauthorized unless @user.super_admin?
|
|
end
|
|
|
|
# Checks if the user is authenticated, sets @user to nil otherwise
|
|
#
|
|
# @return [User, NilClass]
|
|
#
|
|
# @see #pundit_user
|
|
# @see #authenticate_with_api_key_or_current_user
|
|
#
|
|
# @note We could memoize the `@user ||=` but Rubocop wants to rename that to
|
|
# `authenticate_with_api_key_or_current_user` which would be bad as descendant classes
|
|
# have chosen to reference the `@user` instance variable. Intsead [@jeremyf] is
|
|
# favoring leaving this method as is to reduce impact, and having `#pundit_user` do the
|
|
# memoization.
|
|
#
|
|
def authenticate_with_api_key_or_current_user
|
|
@user = authenticate_with_api_key || current_user
|
|
end
|
|
|
|
# Checks if the user is authenticated, if not respond with an HTTP 401 Unauthorized
|
|
#
|
|
# @see #authenticate_with_api_key_or_current_user
|
|
def authenticate_with_api_key_or_current_user!
|
|
# [@jeremyf] Note, I'm not relying on the other method setting the instance variable, but
|
|
# instead relying on the returned value. This insulates us from an implementation detail
|
|
# (namely should we use @user or current_user, which is a bit soupy in the API controller).
|
|
user = authenticate_with_api_key_or_current_user
|
|
error_unauthorized unless user
|
|
end
|
|
|
|
private
|
|
|
|
# @note By default pundit_user is an alias of "#current_user". However, as "#current_user"
|
|
# only tells part of the story, we need to roll our own. That means checking if we have
|
|
# `@user` (which is set in #authenticate_with_api_key_or_current_user) but if that's not
|
|
# present, call the method.
|
|
#
|
|
# @return [User, NilClass]
|
|
#
|
|
# @note [@jeremyf] is choosing to reference the instance variable (e.g. `@user`) and if that's
|
|
# nil to call the `authenticate_with_api_key_or_current_user`. This way I'm not
|
|
# altering the implementation details of the `authenticate_with_api_key_or_current_user`
|
|
# function by introducing memoization.
|
|
#
|
|
# @see #authenticate_with_api_key_or_current_user
|
|
def pundit_user
|
|
# What's going on here?
|
|
@pundit_user ||= @user || authenticate_with_api_key_or_current_user
|
|
end
|
|
|
|
def authenticate_with_api_key
|
|
api_key = request.headers["api-key"]
|
|
return unless api_key
|
|
|
|
api_secret = ApiSecret.includes(:user).find_by(secret: api_key)
|
|
return unless api_secret
|
|
|
|
# guard against timing attacks
|
|
# see <https://www.slideshare.net/NickMalcolm/timing-attacks-and-ruby-on-rails>
|
|
secure_secret = ActiveSupport::SecurityUtils.secure_compare(api_secret.secret, api_key)
|
|
return api_secret.user if secure_secret
|
|
end
|
|
end
|
|
end
|
|
end
|