ErwinM / acts_as_tenant

Easy multi-tenancy for Rails in a shared database setup.
MIT License
1.57k stars 264 forks source link

User to have access to many tenants #231

Open timmyd87 opened 4 years ago

timmyd87 commented 4 years ago

I love this gem. It's amazingly simple to use and thank you for building it!

One enhancement I would love to see is the ability for a user to have many accounts/tenants so they can jump between them. I'm not talking about sharing tenant data (contacts, posts etc) between tenants, but simply so a user is unscoped and the current_tenant methos looks for all possible tenants rather than a single object.

A user could then jump between tenants as they need, sign up for more accounts etc etc.

I have never contributed to a Gem before but wold be keen to give this a go. Thoughts?

excid3 commented 4 years ago

I like this idea and could certainly see this being useful for some apps I'm working on.

Are you envisioning that the interface would change such that if you passed in an Enumerable to set_current_tenant it would use those when setting the tenant ID for the queries?

Something like this?

set_current_tenant(current_user.accounts)
ActsAsTenant.with_tenant(current_user.accounts) do
end
yshmarov commented 4 years ago

@timmyd87 the gem leaves that for you to configure, depending on your needs.

basically you will need to do the following:

  1. human can create a user with a unique email (basic devise)
  2. user can create a tenant
  3. when a user creates a tenant, he automatically becomes a member (user has_many_and_belongs_to tenant through member)
  4. current_user will have access to a list of tenants where he is a member
  5. when current_user "clicks on" a tenant, the application "sets_current_tenant" based on your preferred method (namespace / subdomain / tenant_id / whatever)
  6. user can invite existing or non-existing application users to become a member (devise invitable)

here's an app where I was playing around with what you want to achieve (unfinished) https://github.com/yshmarov/actsastenant-demo

p.s. @excid3 haha you're fast

timmyd87 commented 4 years ago

Wow, thanks for the fast responses @excid3 and @yshmarov!!

So i can only go on what I've used in the past which is the Milia Gem. It's implementation did what you're suggesting @excid3 and find all tenants for a user and simply returned the first one in that list. Here's the code from their lib:

`def set_current_tenant( tenant_id = nil )

  if user_signed_in?

    @_my_tenants ||= current_user.tenants  # gets all possible tenants for user

    tenant_id ||= session[:tenant_id]   # use session tenant_id ?

    if tenant_id.nil?  # no arg; find automatically based on user
      tenant_id = @_my_tenants.first.id  # just pick the first one
    else   # validate the specified tenant_id before setup
      raise InvalidTenantAccess unless @_my_tenants.any?{|tu| tu.id == tenant_id}
    end

  else   # user not signed in yet...
    tenant_id = nil   # an impossible tenant_id
  end

  __milia_change_tenant!( tenant_id )
  trace_tenanting( "set_current_tenant" )

  true    # before filter ok to proceed
end`

So I'm looking for something similar. @yshmarov i like the approach here but to clarify this would require a user to come to a landing page that listed out all tenants/accounts which when one was selected it would pass the tenant id and set the tenant?

timmyd87 commented 4 years ago

Sorry @yshmarov i now understand what you were saying. I've been able to build additional account/tenant creation which is all working, just working on a 'switch tenant' action. I'll post my solution here.

timmyd87 commented 4 years ago

Righto, so i've had a go at this and have been able to successfully:

The problem I'm facing is as i have a rails api application and I'm making requests with a web tokens and can't see a way to persist a tenant. I have tried session storage but that is reset to nil on every request along with the current_tenant so i end up back with the last account created every time I try and change the tenant.

` def find_current_tenant

if ActsAsTenant.current_tenant.nil? && (session[:account].blank?)

  current_account = current_user.accounts.last

  session[:account] = current_account

else

  current_account = session[:account]

end

  set_current_tenant(current_account)

end`

Like the example from @yshmarov I could try and pass the :account_id params on every request however I would need to pass that in to the JWT payload which is defined in my User Model...and you can't access the ActsAsTenant.current_tenant method in models.

Basically it's close but no cigar. What would you suggest? Is there a way to access the current_tenant method in models I'm missing? What's best practice here?

senorlocksmith commented 4 years ago

@yshmarov what happened to your test repo here: https://github.com/yshmarov/acts-as-tenant

I would love to take a look at how you handled multi tenant user sign-ons.

yshmarov commented 4 years ago

@senorlocksmith I've renamed it, you can access it here https://github.com/yshmarov/actsastenant-demo

I didn't think anybody would be looking at it:)

taschetto commented 4 years ago

Like the example from @yshmarov I could try and pass the :account_id params on every request however I would need to pass that in to the JWT payload which is defined in my User Model...and you can't access the ActsAsTenant.current_tenant method in models.

I have this Tenantable concern which I include on "tenantable" controllers.

# frozen_string_literal: true

# app/controllers/concerns/tenantable.rb
require 'active_support/concern'

module Tenantable
  extend ActiveSupport::Concern

  included do
    set_current_tenant_through_filter
    before_action :set_workspace
  end

  private

  def set_workspace
    workspace_id = request.headers['Workspace']
    current_workspace = Workspace.find(workspace_id)
    set_current_tenant(current_workspace)
  end
end

So my front-end app just needs to send a Workspace header containing the workspace id (which is my tenant).

yshmarov commented 4 years ago

@taschetto I prefer the idea of setting current_tenantnot in session, but in user model with a tenant_idfield. The user can switch tenant_idfrom one tenant where he is a member to another one. This way you can access current_tenantin User Modelsimply by going

def current_tenant_in_this_member
  self.tenant
end

Concerns (as you have) are also a very good way to go with acts_as_tenantfor including it into specific controllers without duplicating code. Nice!

taschetto commented 4 years ago

@taschetto I prefer the idea of setting current_tenantnot in session, but in user model with a tenant_idfield. The user can switch tenant_idfrom one tenant where he is a member to another one. This way you can access current_tenantin User Modelsimply by going

def current_tenant_in_this_member
  self.tenant
end

Concerns (as you have) are also a very good way to go with acts_as_tenantfor including it into specific controllers without duplicating code. Nice!

@yshmarov but how can a user be part of multiple tenants if have a single tenant_id?

yshmarov commented 4 years ago

@taschetto Imagine slack or trello. A user can be a member in multiple teams there and he can switch from one team/board to another. You can have a method to switch from one tenant to another (change user.tenant_id) Here's the basic architecture:

obraz

Here's a source code of a very basic implementation https://github.com/yshmarov/actsastenant-demo/

And here's a much more advanced demo of all the beauty of acts_as_tenant put to the maximum https://saas.corsego.com/

pldavid2 commented 4 years ago

Hello @yshmarov. Is the http://ewlit.herokuapp.com/ link ok? I was checking it but seems link an empty demo heroku page.

I would like to check if that demo can help me, I was looking for some similar feature, i have a resource i need to be listed for two different "tenants", so i was thinking on having the resource with multiple tenants ids, but I can't figure out if there is a way to resolve it with the gem, for example making the query clause custom for this model

Of course i can get it working without having the resource with the acts_as_tenant and add the tenant on the where clause directly.

The User/Member/Tenant schema is not good for me unfortunately cause I have different user types, and the member in my case has a lot of information so i prefer not to have the member duped with all its information.

Thanks

yshmarov commented 4 years ago

@pldavid2 sorry, I've updated the link to heroku. Now it's correct

yshmarov commented 4 years ago

@pldavid2 in your case maybe you don't need multitenancy - maybe just role-based access to specific data? gem rolify can help with that.

pldavid2 commented 4 years ago

Hello @yshmarov thanks for the update!

The thing is I have different kind of users (where I already use rolify), and each of these user types can have different tenants so that is where I use acts_as_tenants. But additionally, one of those "user types" can belong to more than one tenant, and i would like to avoid having the user type information dupped. If I'm able to use multi tenants here would be great, maybe is just a matter of being able to override the query clause created by acts_as_tenant for a resource. If not, i will need to avoid using acts_as_tenant on this user_type.

alexventuraio commented 3 years ago

Hi guys, I need to create a multitenant Rails app that basically follows these requirements. And I think this discussion is in some way related to what I want to build.

Screen Shot 2020-11-17 at 13 52 59

Basically the platform can accept Admin Users to signup and are able to create multiple Restaurants and for each restaurant connect a unique Domain name to be accessed by customers.

Can anybody share any thoughts on how to tackle it or the best approach to follow?

yshmarov commented 3 years ago

But additionally, one of those "user types" can belong to more than one tenant, and i would like to avoid having the user type information dupped.

@pldavid2 can you rephrase and give another example?

yshmarov commented 3 years ago

Can anybody share any thoughts on how to tackle it or the best approach to follow?

@alexventuraio

alexventuraio commented 3 years ago

Can anybody share any thoughts on how to tackle it or the best approach to follow?

@alexventuraio

  • Not to overcomplicate things, I would stick with One User model.

  • 2x HABTM relationships between User model and Restaurant (as an Employee and as a Customer)

  • Restaurant = Tenant. Plates, Combos, etc have a restaurant_id and have acts_as_tenant: restaurant

Thx @yshmarov for your response!

Two questions:

yshmarov commented 3 years ago

it's getting more and more complicated 😂

  • when a new Admin user(the owner of the subscription) is registering, how to scope its tenants? Do every singo tenant restaurant should handle a user_id column?

  • What about customers registering for a given restaurant and be visible for other tenant Restaurants belonging to the same Admin user, should they have a tenant_id column?
  1. User opens a restaurants public page
  2. User registers in a restaurant (= create a column in Customer table with user_id and restaurant_id)
  3. So now you need a scope, a way to "group" restaurants.

With HABTM: I would create a table like RestaurantGroup where admins can group restaurants that belong to them, and give each restaurant a restaurant_group_id. Than the customers would be scoped like Customer.where(self.tenant.restaurant_group_id: current_tenant.restaurant_group_id)

Without HABTM: User.rb has_many :customers, through: :restaurants and go @customers = current_tenant.user.customers

pldavid2 commented 3 years ago

But additionally, one of those "user types" can belong to more than one tenant, and i would like to avoid having the user type information dupped.

@pldavid2 can you rephrase and give another example?

Hello @yshmarov .

Well to give my easier use case, basically I have companies and clients. Both of them are users on my system.

Companies have a tenant and or only shown to clients that have that tenant. Clients can have one or more tenants (but not all).

As I need to filter both companies and clients by tenant in several places of my app is useful for me to in the act_as_tenant as gem, and was trying to avoid removing the act_as_tenant in the client model.

yshmarov commented 3 years ago

@pldavid2 see my answer to @alexventuraio - it should be also valid for you. @pldavid2 this thread might be interesting for you: https://github.com/ErwinM/acts_as_tenant/pull/191

Basically to make a platform where a user can be both:

I would make it this way:: Screenshot 2020-11-26 005214

Basically user has_many :vendors as client and as employee.

@hazelsparrow @borisd - this is the kind of architecture you are building, right?

lewiseason commented 3 years ago

My needs are more similar to what Chris said near the beginning of the thread:

Are you envisioning that the interface would change such that if you passed in an Enumerable to set_current_tenant it would use those when setting the tenant ID for the queries?

The application is where one of my tenants has their own resources, but can also manage other tenants' resources on their behalf. Instead of switching between tenants, I'd like the "on behalf of" tenant to have the resources scoped to all the tenants they have access to.

For example (assuming an Order model with acts_as_tenant :organisation):

class OrdersController < ApplicationController
  set_current_tenant_through_filter
  before_action :set_tenant

  def index
    @orders = Order.all
  end

  def set_tenant
    set_current_tenant(Organisation.where(id: current_user.delegated_organisations))
  end
end

I basically have two questions:

  1. is this different enough to this issue and #191 that I should open a new issue, and
  2. is there interest in this, and is it worth opening as an issue

I'm basically planning to ~monkey-patch~ (ed: probably an extra concern) acts_as_tenant for my needs to make the following change:

diff --git a/lib/acts_as_tenant/model_extensions.rb b/lib/acts_as_tenant/model_extensions.rb
index b9a1030..d96b38a 100644
--- a/lib/acts_as_tenant/model_extensions.rb
+++ b/lib/acts_as_tenant/model_extensions.rb
@@ -21,7 +21,12 @@ def acts_as_tenant(tenant = :account, **options)
           end

           if ActsAsTenant.current_tenant
-            keys = [ActsAsTenant.current_tenant.send(pkey)].compact
+            if options[:multiple_tenants] && !ActsAsTenant.current_tenant.is_a?(ActiveRecord::Base)
+              keys = [ActsAsTenant.current_tenant.pluck(pkey)].compact
+            else
+              keys = [ActsAsTenant.current_tenant.send(pkey)].compact
+            end
+
             keys.push(nil) if options[:has_global_records]

             query_criteria = {fkey.to_sym => keys}
@@ -37,7 +42,7 @@ def acts_as_tenant(tenant = :account, **options)
         # - validate that associations belong to the tenant, currently only for belongs_to
         #
         before_validation proc { |m|
-          if ActsAsTenant.current_tenant
+          if ActsAsTenant.current_tenant && (!options[:multiple_tenants] || ActsAsTenant.is_a?(ActiveRecord::Base))
             if options[:polymorphic]
               m.send("#{fkey}=".to_sym, ActsAsTenant.current_tenant.class.to_s) if m.send(fkey.to_s).nil?
               m.send("#{polymorphic_type}=".to_sym, ActsAsTenant.current_tenant.class.to_s) if m.send(polymorphic_type.to_s).nil?

I don't have any test coverage or documentation for this, but if there's interest, I could probably tidy this up and make it into a PR. Alternatively, a PR which allows you to pass a custom proc to replace keys = [ActsAsTenant.current_tenant.send(pkey)].compact with something, and conditionally disable the before_validation block, so you can replace these with anything.

Thoughts? Sorry for hijacking this issue a bit!

timmyd87 commented 3 years ago

@lewiseason I have a very similar need for my multitenant app. I have 'partners' where i would like them to have access to multiple tenants but also persist access to their own tenant or 'partner portal' where they can view data from their accessible tenants (in their case, their business clients).

Are we on the same page? or have i misunderstood what you're trying to achieve?

lewiseason commented 3 years ago

@timmyd87 Yes, exactly. Ideally, I'd use this for our own staff too—to access the customer area of the app, you have to specify which customer you're accessing it as.

The other thing which I anticipate needing is the ability to further filter records for a partner, so they can see their own records, a specific tenant they have access to, or everything that's visible to them. With all that in mind, I'm beginning to think a simple concern, and doing this explicitly might be easier. For (untested) example:

class Account < ApplicationRecord
  belongs_to :partner, class_name: "Account"
  has_many :accessible_accounts, class_name: "Account", foreign_key: :partner_id
end

class Widget < ApplicationRecord
  include HasAccount
end

module HasAccount
  extend ActiveSupport::Concern

  included do
    belongs_to :account
  end

  class_methods do
    def for_account(account)
      where(account: account)
    end

    def accessible_by_account(account)
      where(account: [account] + account.accessible_accounts)
    end
  end
end

class ApplicationController < ActionController::Base
  # <snip>

  def current_account
    @current_account ||= begin
      # You'd want to do something more sophisticated here, but this allows
      # staff (and partners) to "impersonate" a specific account
      Account.find(session[:account_id]) || current_user.account
    end
  end
end

You don't get all the enforcement and automatic behaviour that acts_as_tenant provides with this approach obviously, but I think it's a better fit for my use-case.