Draft: Don't rely on current_user when a Grape controller is using custom authentication
What does this MR do and why?
For Cells, we need to set a Current.organization
property for determining the Organization a request should be applied to.
This is derived from the path, or the current user.
Initially, this logic was added to a Grape API before
hook and we used @current_user
as a parameter. However, in that before hook, @current_user
is always nil
. A natural solution was to switch to current_user
but that fails if a Grape controller is using a custom authentication. For example, when a token is passed in private-token
header that is NOT a Personal Access Token. In that case, the current_user
method will actually render a 'Unauthorized' HTTP header.
- In Rails context,
current_user
returns a user ORnil
- In Grape context,
current_user
returns a user OR renders anUnauthorized
HTTP header
It still makes sense to involve current_user
(and not @current_user
) when determining the Current Organization. But in a few cases, we need to skip using current_user
.
The problem I have is HOW to 'mark' or 'see' or 'determine' the Grape Controllers that can't use current_user
. I considered several options:
-
current_user
should just returnnil
and not render an error page: Lot of impact, potentially adding authentication logic to all controllers - Add a
don't use current_user
flag to API classes: I think this is best but I could not find a way of adding this. - Catch
Unauthorized
error in before hook: Not possible becausecurrent_user
is heavily memoized - Override
set_current_organization
method: This worked withAPI::Scim::InstanceScim
but not withAPI::NpmProjectPackages
controller. - Skip setting organization for certain endpoints. This is what I am trying now.
Related to #482503 (closed)
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.