Unscoped finds in `find_with`

[paulodeon]

When implementing the find_with proc in the scim_attributes_map of the Group class we can write something like this:

          find_with: ->(scim_list_entry) {
            id   = scim_list_entry['value']
            type = scim_list_entry['type'] || 'User' # Some online examples omit 'type' and believe 'User' will be assumed

            case type.downcase
            when 'user' then User.find_by(uuid: id)
            when 'group' then ScimGroup.find_by(uuid: id)
            else raise Scimitar::InvalidSyntaxError, "Unrecognised type #{type.inspect}"
            end
          }

Unfortunately this is a security risk where if a malicious actor with an account had the user id of a user from another account/tenant they could access details of that user.

Is there any way to pass additional information into that function so that we could scope the finds to the tenant?

[pond]

Initial thoughts:

SCIM API calls can only be made from an authorised caller, and the authorised caller should be issued per-tenant, with their lookups scoped by tenancy.

That's how we do it, anyway and is one of the only places in the system that we justify use of default_scope (as a mixin Concern used by any tenant-scoped model to make sure that lookups are always appropriately scoped, with an exception thrown if there's no current user or current user tenancy).

[pond]

Oh, in addition - also check out ActiveSupport::CurrentAttributes. The concept of a current request's scoped user or wider configuration issues like determined tenancy, time zone, locale etc. can be set in a before-action in ApplicationController and thread-safe stored there for the duration of request, which makes access to such properties at all levels of the stack very easy. It's all very well swallowing a design patterns book and trying for perfect MVC separation but of course this often leads to more complex and obtuse solutions when it comes to very important things like "needing to know a current user at the model layer". For example, given business logic is "supposed to" live in models, knowing the current user at that layer is an extremely common requirement. Passing it down through parameters is not a particularly clean or simple solution.

[paulodeon]

Hey @pond thanks for coming back so quick - we have implemented the CurrentAttributes solution for the time being.

Perhaps find_with could be extended to take a second parameter which would be the whole request - then it could be responsible for selecting the right scope.

Or the example in the codebase could be updated to use CurrentAttributes

[pond]

This is still a definite maybe, but I've no idea if I'll actually do it one day or when. Even so, I'm keeping this issue open indefinitely as a reminder and am just posting this update to note that "this isn't dead, no matter how long it goes without further updates".