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_userreturns a user ORnil - In Grape context,
current_userreturns a user OR renders anUnauthorizedHTTP 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_usershould just returnniland not render an error page: Lot of impact, potentially adding authentication logic to all controllers - Add a
don't use current_userflag to API classes: I think this is best but I could not find a way of adding this. - Catch
Unauthorizederror in before hook: Not possible becausecurrent_useris heavily memoized - Override
set_current_organizationmethod: This worked withAPI::Scim::InstanceScimbut not withAPI::NpmProjectPackagescontroller. - 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.