Skip to content

Draft: Don't rely on current_user when a Grape controller is using custom authentication

Rutger Wessels requested to merge 482503-rest-api-set-current-user into master

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 OR nil
  • In Grape context, current_user returns a user OR renders an Unauthorized 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:

  1. current_user should just return nil and not render an error page: Lot of impact, potentially adding authentication logic to all controllers
  2. 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.
  3. Catch Unauthorized error in before hook: Not possible because current_user is heavily memoized
  4. Override set_current_organization method: This worked with API::Scim::InstanceScim but not with API::NpmProjectPackages controller.
  5. 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.

Edited by Rutger Wessels

Merge request reports

Loading