anycable / anycable-rails

AnyCable for Ruby on Rails applications
https://anycable.io
MIT License
499 stars 35 forks source link

Tenant getting lost during identifier serialization #179

Closed jhubert closed 1 year ago

jhubert commented 1 year ago

Tell us about your environment

Ruby version: 3.1.2

anycable gem version: 1.2.5

anycable-rails gem version: 1.3.5

grpc gem version: 1.52.0

What did you do?

We're following the guidance for multi-tenancy from the anycable.io site but we're seeing the tenant get lost when the objects are serialized.

Our GlobalID configuration takes the current tenant into account. So, if you call:

Apartment::Tenant.switch('acme') do
  User.new(id: 1).to_gid.to_s
end

It would return gid://acme/User/1

Here is a simplified version of what we're doing:

module ApplicationCable
  class Connection < ActionCable::Connection::Base
    around_command :using_current_tenant

    identified_by :current_tenant, :current_user

    delegate :session, to: :request

    def connect
      self.current_tenant = session[:current_tenant].presence
      self.current_user = using_current_tenant { User.find_by(id: session[:current_user_id]) }
    end

    private

    def using_current_tenant(&block)
      Apartment::Tenant.switch(current_tenant, &block)
    end
  end
end
class NotificationChannel < ApplicationCable::Channel
  def subscribed
    using_current_tenant do
      stream_for current_user
      Connected::Notification.connect(current_user)
    end
  end

  def unsubscribed
    using_current_tenant do
      Connected::Notification.disconnect(current_user)
    end
  end
end

What did you expect to happen?

We would expect the current_user to exist in the channel subscribed and unsubscribed method.

What actually happened?

The user deserialization fails because the GlobalID::Locate command happens in the current tenant, but the serialization of the object to a globalid happens outside of the tenant. This causes a user in the Acme tenant to be serialized in the default tenant, and then it's not locatable when the channel tries to identify it.

connection_identifiers: "{"current_user_gid":"gid://public/User/1"}"

You can test this by setting GlobalID.app to the current tenant before serialization / deserialization.

GlobalID.app = Apartment::Tenant.current

We have been able to work around this by setting the identifier to a gid_param, which is then magically rehydrated by AnyCable but that's very unexpected behavior.

module ApplicationCable
  class Connection < ActionCable::Connection::Base
    ...

    def connect
      ...
      self.current_user = using_current_tenant { User.find_by(id: session[:current_user_id])&.to_gid_param }
    end

    ...
  end
end

The other issue with doing that is that when we use the redis adapter in tests and dev mode, the tests fail because ActionCable doesn't rehydrate globalids automatically when loading the identifiers like AnyCable does.

palkan commented 1 year ago

but the serialization of the object to a globalid happens outside of the tenant

Yeah, that's the problem.

Where do you set GlobalID.app = Apartment::Tenant.current when running AnyCable / Action Cable?

palkan commented 1 year ago

So, I've been thinking about a proper way to handle this, and I see two options:

1) Memoize the value of User#to_gid_param and warm it up right within the #using_current_tenant block:

# user.rb
class User
  def to_gid_param(...)
    @gid_param ||= super
  end
end

# connection.rb
self.current_user = using_current_tenant { User.find_by(id: session[:current_user_id]).tap { _1&.to_gid_param } }

2) Set current tenant for the whole request, i.e., Apartment::Tenant.switch!(current_tenant) and reset it in the AnyCable middleware:

class ApartmentResetMiddleware < AnyCable::Middleware
  def call(handler, request, meta)
    yield
  ensure
    Apartment::Tenant.switch!("public")
  end
end

AnyCable.middleware.use(ApartmentResetMiddleware)

The second one looks better and less hacky.

palkan commented 1 year ago

Okay, I think, even better solution would be to rely on Rails executor to reset the state:

module ApplicationCable
  class Connection < ActionCable::Connection::Base
    before_command :use_current_tenant

    # ...

    private

    def use_current_tenant
      Apartment::Tenant.switch!(current_tenant)
    end
  end
end

# initializers/anycable.rb
AnyCable.configure_server do
  Rails.application.executor.to_complete { Apartment::Tenant.switch!("public") }
end

Updated the post to mention this caveat.