Allow having tenants with different domains

Some institutions using CONSUL have expressed interest in this feature
since some of their tenants might already have their own domains.

We've considered many options for the user interface to select whether
we're using a subdomain or a domain, like having two separate fields,
using a check box, ... In the end we've chosen radio buttons because
they make it easier to follow a logical sequence: first you decide
whether you're introducing a domain or subdomain, and then you enter it.

We've also considered hiding this option and assuming "if it's got a
dot, it's a domain". However, this wouldn't work with nested subdomains
and it wouldn't work with domains which are simply machine names.

Note that a group of radio buttons (or check boxes) is difficult to
style when the text of the label might expand over more than one line
(as is the case here on small screens); in this case, most solutions
result in the second line of the label appearing immediately under the
radio button, instead of being aligned with the first line of the label.
That's why I've added a container for the input+label combination.

.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/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) }
 
@@ -273,6 +344,20 @@ describe Tenant do
       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/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"