Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tenant getting lost during identifier serialization #179

Closed
jhubert opened this issue Feb 10, 2023 · 3 comments
Closed

Tenant getting lost during identifier serialization #179

jhubert opened this issue Feb 10, 2023 · 3 comments

Comments

@jhubert
Copy link

jhubert commented Feb 10, 2023

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
Copy link
Member

palkan commented Feb 10, 2023

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
Copy link
Member

palkan commented Mar 10, 2023

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 } }
  1. 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
Copy link
Member

palkan commented Oct 15, 2023

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.

@palkan palkan closed this as completed Oct 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants