Merge pull request #5007 from consul/tenant_domains

Allow using domains in tenants

.rubocop.yml

@@ -248,6 +248,7 @@ Rails/DurationArithmetic:
 Rails/DynamicFindBy:
   Enabled: true
   Whitelist:
+    - find_by_domain
     - find_by_slug_or_id
     - find_by_slug_or_id!
     - find_by_manager_login

app/assets/javascripts/admin/tenants/form.js

@@ -0,0 +1,20 @@
+(function() {
+  "use strict";
+  App.AdminTenantsForm = {
+    initialize: function() {
+      var form = $(".admin .tenant-form");
+      var inputs = $("input[name$='[schema_type]']", form);
+      var label = $("label[for$='schema']", form);
+
+      inputs.on("change", function() {
+        label.text(label.data("schema-type-" + $(this).val()));
+      });
+
+      inputs.each(function() {
+        if ($(this).is(":checked")) {
+          $(this).trigger("change");
+        }
+      });
+    }
+  };
+}).call(this);

app/assets/javascripts/application.js

@@ -167,6 +167,7 @@ var initialize_modules = function() {
   }
   App.AdminBudgetsWizardCreationStep.initialize();
   App.AdminMachineLearningScripts.initialize();
+  App.AdminTenantsForm.initialize();
   App.AdminVotationTypesFields.initialize();
   App.BudgetEditAssociations.initialize();
   App.BudgetHideMoney.initialize();

app/assets/stylesheets/admin/budgets/form.scss

@@ -9,20 +9,12 @@
   }
 
   > fieldset {
-    border-top: 4px solid $admin-border-color;
+    @include admin-fieldset-separator;
     clear: both;
     margin-top: $line-height * 1.5;
 
     &:first-of-type {
       margin-top: 0;
     }
-
-    legend {
-      color: $admin-text;
-      font-size: $small-font-size;
-      font-weight: bold;
-      padding-right: $line-height / 2;
-      text-transform: uppercase;
-    }
   }
 }

app/assets/stylesheets/admin/tenants/form.scss

@@ -0,0 +1,19 @@
+.admin .tenant-form {
+  > fieldset {
+    @include admin-fieldset-separator;
+    margin-top: $line-height;
+  }
+
+  .radio-and-label {
+    display: flex;
+    margin-bottom: $line-height / 3;
+
+    &:last-of-type {
+      margin-bottom: $line-height * 2 / 3;
+    }
+
+    input {
+      margin-bottom: 0;
+    }
+  }
+}

app/assets/stylesheets/mixins/forms.scss

@@ -187,3 +187,15 @@
 %public-form {
   @include public-form;
 }
+
+@mixin admin-fieldset-separator {
+  border-top: 4px solid $admin-border-color;
+
+  > legend {
+    color: $admin-text;
+    font-size: $small-font-size;
+    font-weight: bold;
+    padding-right: $line-height / 2;
+    text-transform: uppercase;
+  }
+}

app/components/admin/tenants/form_component.html.erb

@@ -1,7 +1,19 @@
-<%= form_for [:admin, tenant] do |f| %>
+<%= form_for [:admin, tenant], html: { class: "tenant-form" } do |f| %>
   <%= render "shared/errors", resource: tenant %>
 
   <%= f.text_field :name %>
-  <%= f.text_field :schema %>
+
+  <fieldset>
+    <legend><%= attribute_name(:url) %></legend>
+    <div class="radio-and-label">
+      <%= f.radio_button :schema_type, :subdomain, label: t("admin.tenants.form.use_subdomain", domain: domain) %>
+    </div>
+    <div class="radio-and-label">
+      <%= f.radio_button :schema_type, :domain, label: t("admin.tenants.form.use_domain") %>
+    </div>
+
+    <%= f.text_field :schema, label_options: { data: schema_labels_per_schema_type } %>
+  </fieldset>
+
   <%= f.submit %>
 <% end %>

app/components/admin/tenants/form_component.rb

@@ -4,4 +4,20 @@ class Admin::Tenants::FormComponent < ApplicationComponent
   def initialize(tenant)
     @tenant = tenant
   end
+
+  private
+
+    def attribute_name(attribute)
+      Tenant.human_attribute_name(attribute)
+    end
+
+    def domain
+      Tenant.default_domain
+    end
+
+    def schema_labels_per_schema_type
+      Tenant.schema_types.keys.to_h do |schema_type|
+        [:"schema_type_#{schema_type}", attribute_name(schema_type)]
+      end
+    end
 end

app/components/admin/tenants/index_component.html.erb

@@ -7,6 +7,7 @@
     <tr>
       <th><%= attribute_name(:name) %></th>
       <th><%= attribute_name(:schema) %></th>
+      <th><%= attribute_name(:url) %></th>
       <th><%= t("admin.shared.actions") %></th>
     </tr>
   </thead>
@@ -16,6 +17,7 @@
       <tr id="<%= dom_id(tenant) %>">
         <td><%= tenant.name %></td>
         <td><%= tenant.schema %></td>
+        <td><%= tenant.host %></td>
         <td>
           <%= render Admin::TableActionsComponent.new(tenant, actions: [:edit]) do |actions| %>
             <%= actions.action(:show, text: t("admin.shared.view"), path: root_url(host: tenant.host)) %>

app/controllers/admin/tenants_controller.rb

@@ -30,6 +30,6 @@ class Admin::TenantsController < Admin::BaseController
   private
 
     def tenant_params
-      params.require(:tenant).permit(:name, :schema)
+      params.require(:tenant).permit(:name, :schema, :schema_type)
     end
 end

app/models/tenant.rb

@@ -1,4 +1,6 @@
 class Tenant < ApplicationRecord
+  enum schema_type: %w[subdomain domain]
+
   validates :schema,
     presence: true,
     uniqueness: true,
@@ -10,12 +12,26 @@ class Tenant < ApplicationRecord
   after_update :rename_schema
   after_destroy :destroy_schema
 
+  def self.find_by_domain(host)
+    domain.find_by(schema: host)
+  end
+
   def self.resolve_host(host)
     return nil unless Rails.application.config.multitenancy.present?
     return nil if host.blank? || host.match?(Resolv::AddressRegex)
 
-    host_domain = allowed_domains.find { |domain| host == domain || host.ends_with?(".#{domain}") }
-    host.delete_prefix("www.").sub(/\.?#{host_domain}\Z/, "").presence
+    host_without_www = host.delete_prefix("www.")
+
+    if find_by_domain(host)
+      host
+    elsif find_by_domain(host_without_www)
+      host_without_www
+    else
+      host_domain = allowed_domains.find { |domain| host == domain || host.ends_with?(".#{domain}") }
+      schema = host_without_www.sub(/\.?#{host_domain}\Z/, "").presence
+
+      schema unless find_by_domain(schema)
+    end
   end
 
   def self.allowed_domains
@@ -35,6 +51,14 @@ class Tenant < ApplicationRecord
     default_url_options[:host]
   end
 
+  def self.default_domain
+    if default_host == "localhost"
+      "lvh.me"
+    else
+      default_host
+    end
+  end
+
   def self.current_url_options
     default_url_options.merge(host: current_host)
   end
@@ -46,10 +70,10 @@ class Tenant < ApplicationRecord
   def self.host_for(schema)
     if schema == "public"
       default_host
-    elsif default_host == "localhost"
-      "#{schema}.lvh.me"
+    elsif find_by_domain(schema)
+      schema
     else
-      "#{schema}.#{default_host}"
+      "#{schema}.#{default_domain}"
     end
   end
 

config/locales/en/activerecord.yml

@@ -382,7 +382,10 @@ en:
       tag:
         name: "Type the name of the topic"
       tenant:
-        schema: "Subdomain"
+        domain: "Domain"
+        schema: "Domain / Subdomain"
+        subdomain: "Subdomain"
+        url: "URL"
       topic:
         title: "Title"
         description: "Initial text"

config/locales/en/admin.yml

@@ -1632,6 +1632,9 @@ en:
     tenants:
       create:
         notice: Tenant created successfully
+      form:
+        use_subdomain: "Use a subdomain in the %{domain} domain to access this tenant"
+        use_domain: "Use a different domain to access this tenant"
       index:
         create: Create tenant
       new:

config/locales/es/activerecord.yml

@@ -382,7 +382,10 @@ es:
       tag:
         name: "Escribe el nombre del tema"
       tenant:
-        schema: "Subdominio"
+        domain: "Dominio"
+        schema: "Dominio / Subdominio"
+        subdomain: "Subdominio"
+        url: "URL"
       topic:
         title: "Título"
         description: "Texto inicial"

config/locales/es/admin.yml

@@ -1631,6 +1631,9 @@ es:
     tenants:
       create:
         notice: Entidad creada correctamente
+      form:
+        use_subdomain: "Utiliza un subdominio en el dominio %{domain} para acceder a esta entidad"
+        use_domain: "Utiliza un dominio distinto para acceder a esta entidad"
       index:
         create: Crear entidad
       new:

db/migrate/20221120123254_add_schema_type_to_tenants.rb

@@ -0,0 +1,5 @@
+class AddSchemaTypeToTenants < ActiveRecord::Migration[6.0]
+  def change
+    add_column :tenants, :schema_type, :integer, null: false, default: 0
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2022_09_15_154808) do
+ActiveRecord::Schema.define(version: 2022_11_20_123254) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -1561,6 +1561,7 @@ ActiveRecord::Schema.define(version: 2022_09_15_154808) do
     t.string "schema"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.integer "schema_type", default: 0, null: false
     t.index ["name"], name: "index_tenants_on_name", unique: true
     t.index ["schema"], name: "index_tenants_on_schema", unique: true
   end

spec/controllers/concerns/tenant_variants_spec.rb

@@ -20,4 +20,11 @@ describe TenantVariants do
     get :index
     expect(response.body).to eq '[:"random-name"]'
   end
+
+  it "keeps dots in the variant names" do
+    allow(Tenant).to receive(:current_schema).and_return("random.domain")
+
+    get :index
+    expect(response.body).to eq '[:"random.domain"]'
+  end
 end

spec/factories/administration.rb

@@ -102,5 +102,9 @@ FactoryBot.define do
   factory :tenant do
     sequence(:name) { |n| "Tenant #{n}" }
     sequence(:schema) { |n| "subdomain#{n}" }
+
+    trait :domain do
+      schema_type { :domain }
+    end
   end
 end

spec/factory_bot/strategy/insert.rb

@@ -0,0 +1,24 @@
+module FactoryBot
+  module Strategy
+    class Insert
+      def initialize
+        @strategy = FactoryBot.strategy_by_name(:attributes_for).new
+      end
+
+      delegate :association, to: :@strategy
+
+      def result(evaluation)
+        build_class = evaluation.instance_variable_get(:@attribute_assigner)
+                                .instance_variable_get(:@build_class)
+
+        timestamps = { created_at: Time.current, updated_at: Time.current }.select do |attribute, _|
+          build_class.has_attribute?(attribute)
+        end
+
+        build_class.insert!(timestamps.merge(@strategy.result(evaluation)))
+      end
+    end
+
+    FactoryBot.register_strategy(:insert, Insert)
+  end
+end

spec/models/abilities/administrator_spec.rb

@@ -183,17 +183,16 @@ describe Abilities::Administrator do
       it { should be_able_to :update, Tenant }
       it { should_not be_able_to :destroy, Tenant }
 
-      it "does not allow administrators from other tenants to manage tenants " do
-        create(:tenant, schema: "subsidiary")
-
-        Tenant.switch("subsidiary") do
-          admin = create(:administrator).user
-
-          expect(admin).not_to be_able_to :create, Tenant
-          expect(admin).not_to be_able_to :read, Tenant
-          expect(admin).not_to be_able_to :update, Tenant
-          expect(admin).not_to be_able_to :destroy, Tenant
+      context "administrators from other tenants" do
+        before do
+          insert(:tenant, schema: "subsidiary")
+          allow(Tenant).to receive(:current_schema).and_return("subsidiary")
         end
+
+        it { should_not be_able_to :create, Tenant }
+        it { should_not be_able_to :read, Tenant }
+        it { should_not be_able_to :update, Tenant }
+        it { should_not be_able_to :destroy, Tenant }
       end
     end
   end

spec/models/setting_spec.rb

@@ -193,11 +193,10 @@ describe Setting do
     end
 
     it "returns the tenant name for other tenants" do
-      create(:tenant, schema: "new", name: "New Institution")
+      insert(:tenant, schema: "new", name: "New Institution")
+      allow(Tenant).to receive(:current_schema).and_return("new")
 
-      Tenant.switch("new") do
-        expect(Setting.default_org_name).to eq "New Institution"
-      end
+      expect(Setting.default_org_name).to eq "New Institution"
     end
   end
 

spec/models/tenant_spec.rb

@@ -74,6 +74,49 @@ describe Tenant do
       expect(Tenant.resolve_host("www.mercury.anotherconsul.dev")).to eq "mercury.anotherconsul.dev"
     end
 
+    it "returns full domains when there's a tenant with a domain including the host" do
+      insert(:tenant, :domain, schema: "saturn.consul.dev")
+
+      expect(Tenant.resolve_host("saturn.consul.dev")).to eq "saturn.consul.dev"
+    end
+
+    it "returns subdomains when there's a subdomain-type tenant with that domain" do
+      insert(:tenant, schema: "saturn.consul.dev")
+
+      expect(Tenant.resolve_host("saturn.consul.dev")).to eq "saturn"
+    end
+
+    it "returns nil when a domain is accessed as a subdomain" do
+      insert(:tenant, :domain, schema: "saturn.dev")
+
+      expect(Tenant.resolve_host("saturn.dev.consul.dev")).to be nil
+    end
+
+    it "returns nested subdomains when there's a subdomain-type tenant with nested subdomains" do
+      insert(:tenant, schema: "saturn.dev")
+
+      expect(Tenant.resolve_host("saturn.dev.consul.dev")).to eq "saturn.dev"
+    end
+
+    it "returns domains when there are two tenants resolving to the same domain" do
+      insert(:tenant, schema: "saturn")
+      insert(:tenant, :domain, schema: "saturn.consul.dev")
+
+      expect(Tenant.resolve_host("saturn.consul.dev")).to eq "saturn.consul.dev"
+    end
+
+    it "returns domains when there's a tenant using the default host" do
+      insert(:tenant, :domain, schema: "consul.dev")
+
+      expect(Tenant.resolve_host("consul.dev")).to eq "consul.dev"
+    end
+
+    it "returns domains including www when the tenant contains it" do
+      insert(:tenant, :domain, schema: "www.consul.dev")
+
+      expect(Tenant.resolve_host("www.consul.dev")).to eq "www.consul.dev"
+    end
+
     context "multitenancy disabled" do
       before { allow(Rails.application.config).to receive(:multitenancy).and_return(false) }
 
@@ -151,6 +194,18 @@ describe Tenant do
 
       expect(Tenant.host_for("uranus")).to eq "uranus.lvh.me"
     end
+
+    it "ignores the default host when given a full domain" do
+      insert(:tenant, :domain, schema: "whole.galaxy")
+
+      expect(Tenant.host_for("whole.galaxy")).to eq "whole.galaxy"
+    end
+
+    it "uses the default host when given nested subdomains" do
+      insert(:tenant, schema: "whole.galaxy")
+
+      expect(Tenant.host_for("whole.galaxy")).to eq "whole.galaxy.consul.dev"
+    end
   end
 
   describe ".current_secrets" do
@@ -230,6 +285,22 @@ describe Tenant do
     end
   end
 
+  describe "scopes" do
+    describe ".domain" do
+      it "returns tenants with domain schema type" do
+        insert(:tenant, schema_type: :domain, schema: "full.domain")
+
+        expect(Tenant.domain.pluck(:schema)).to eq ["full.domain"]
+      end
+
+      it "does not return tenants with subdomain schema type" do
+        insert(:tenant, schema_type: :subdomain, schema: "nested.subdomain")
+
+        expect(Tenant.domain).to be_empty
+      end
+    end
+  end
+
   describe "validations" do
     let(:tenant) { build(:tenant) }
 
@@ -243,7 +314,7 @@ describe Tenant do
     end
 
     it "is not valid with an already existing schema" do
-      expect(create(:tenant, schema: "subdomainx")).to be_valid
+      insert(:tenant, schema: "subdomainx")
       expect(build(:tenant, schema: "subdomainx")).not_to be_valid
     end
 
@@ -270,9 +341,23 @@ describe Tenant do
     end
 
     it "is not valid with an already existing name" do
-      expect(create(:tenant, name: "Name X")).to be_valid
+      insert(:tenant, name: "Name X")
       expect(build(:tenant, name: "Name X")).not_to be_valid
     end
+
+    context "Domain schema type" do
+      before { tenant.schema_type = :domain }
+
+      it "is valid with domains" do
+        tenant.schema = "my.domain"
+        expect(tenant).to be_valid
+      end
+
+      it "is valid with domains which are machine names" do
+        tenant.schema = "localmachine"
+        expect(tenant).to be_valid
+      end
+    end
   end
 
   describe "#create_schema" do

spec/spec_helper.rb

@@ -3,6 +3,7 @@ require "email_spec"
 require "devise"
 require "knapsack_pro"
 
+Dir["./spec/factory_bot/**/*.rb"].sort.each { |f| require f }
 Dir["./spec/models/concerns/*.rb"].each { |f| require f }
 Dir["./spec/support/**/*.rb"].sort.each { |f| require f }
 Dir["./spec/shared/**/*.rb"].sort.each  { |f| require f }

spec/system/admin/tenants_spec.rb

@@ -3,26 +3,51 @@ require "rails_helper"
 describe "Tenants", :admin, :seed_tenants do
   before { allow(Tenant).to receive(:default_host).and_return("localhost") }
 
-  scenario "Create" do
-    visit admin_root_path
+  describe "Create" do
+    scenario "Tenant with subdomain" do
+      visit admin_root_path
 
-    within("#side_menu") do
-      click_link "Settings"
-      click_link "Multitenancy"
+      within("#side_menu") do
+        click_link "Settings"
+        click_link "Multitenancy"
+      end
+
+      click_link "Create tenant"
+      fill_in "Subdomain", with: "earth"
+      fill_in "Name", with: "Earthlings"
+      click_button "Create tenant"
+
+      expect(page).to have_content "Tenant created successfully"
+
+      within("tr", text: "earth") do
+        expect(page).to have_content "earth.lvh.me"
+
+        click_link "View"
+      end
+
+      expect(current_host).to eq "http://earth.lvh.me"
+      expect(page).to have_current_path root_path
+      expect(page).to have_link "Sign in"
     end
 
-    click_link "Create tenant"
-    fill_in "Subdomain", with: "earth"
-    fill_in "Name", with: "Earthlings"
-    click_button "Create tenant"
+    scenario "Tenant with domain" do
+      visit new_admin_tenant_path
 
-    expect(page).to have_content "Tenant created successfully"
+      choose "Use a different domain to access this tenant"
+      fill_in "Domain", with: "earth.lvh.me"
+      fill_in "Name", with: "Earthlings"
+      click_button "Create tenant"
 
-    within("tr", text: "earth") { click_link "View" }
+      within("tr", text: "earth") do
+        expect(page).to have_content "earth.lvh.me"
 
-    expect(current_host).to eq "http://earth.lvh.me"
-    expect(page).to have_current_path root_path
-    expect(page).to have_link "Sign in"
+        click_link "View"
+      end
+
+      expect(current_host).to eq "http://earth.lvh.me"
+      expect(page).to have_current_path root_path
+      expect(page).to have_link "Sign in"
+    end
   end
 
   scenario "Update" do
@@ -31,6 +56,10 @@ describe "Tenants", :admin, :seed_tenants do
     visit admin_tenants_path
     within("tr", text: "moon") { click_link "Edit" }
 
+    expect(page).to have_field "Use a subdomain in the lvh.me domain to access this tenant",
+                               type: :radio,
+                               checked: true
+
     fill_in "Subdomain", with: "the-moon"
     click_button "Update tenant"