diff --git a/Gemfile b/Gemfile index 1f6f0d1bb..ffe1245b5 100644 --- a/Gemfile +++ b/Gemfile @@ -38,6 +38,7 @@ gem 'paper_trail' gem 'pg', '~> 1.6' gem 'postmark-rails' gem 'propshaft' +gem 'public_suffix', '~> 7.0' gem 'puma', '~> 7.2' gem 'rack_content_type_default', '~> 1.1' gem 'rack-cors' diff --git a/Gemfile.lock b/Gemfile.lock index 1a57687fb..728c9924d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -614,6 +614,7 @@ DEPENDENCIES postmark-rails propshaft pry-byebug + public_suffix (~> 7.0) puma (~> 7.2) rack-cors rack_content_type_default (~> 1.1) diff --git a/app/models/school.rb b/app/models/school.rb index fe35f00fa..d16c1a8e7 100644 --- a/app/models/school.rb +++ b/app/models/school.rb @@ -6,6 +6,7 @@ class School < ApplicationRecord has_many :projects, dependent: :nullify has_many :roles, dependent: :nullify has_many :school_projects, dependent: :nullify + has_many :school_email_domains, dependent: :destroy VALID_URL_REGEX = %r{\A(?:https?://)?(?:www.)?[a-z0-9]+([-.]{1}[a-z0-9]+)*\.[a-z]{2,63}(\.[a-z]{2,63})*(/.*)?\z}ix @@ -116,6 +117,13 @@ def import_in_progress? .exists?(description: id) end + def valid_domain?(candidate_domain) + validated_domain = SchoolEmailDomainValidator.call(candidate_domain) + school_email_domains.exists?(domain: validated_domain) + rescue ::SchoolEmailDomainValidator::Error + false + end + private # Ensure the reference is nil, not an empty string diff --git a/app/models/school_email_domain.rb b/app/models/school_email_domain.rb new file mode 100644 index 000000000..d7c71ca66 --- /dev/null +++ b/app/models/school_email_domain.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class SchoolEmailDomain < ApplicationRecord + belongs_to :school + + validates :domain, presence: true + validates :domain, uniqueness: { scope: :school_id } + + before_validation :validate_domain + + private + + def validate_domain + self.domain = SchoolEmailDomainValidator.call(domain) + rescue ::SchoolEmailDomainValidator::Error => e + errors.add(:domain, e.error_code) + end +end diff --git a/db/migrate/20260420104937_create_school_email_domains.rb b/db/migrate/20260420104937_create_school_email_domains.rb new file mode 100644 index 000000000..0101e995c --- /dev/null +++ b/db/migrate/20260420104937_create_school_email_domains.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class CreateSchoolEmailDomains < ActiveRecord::Migration[7.2] + def change + create_table :school_email_domains, id: :uuid do |t| + t.references :school, null: false, foreign_key: true, type: :uuid + t.string :domain, null: false + + t.timestamps + end + + add_index :school_email_domains, %i[school_id domain], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index eb22dba99..be55c5e42 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2026_04_10_110000) do +ActiveRecord::Schema[7.2].define(version: 2026_04_20_104937) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -268,6 +268,15 @@ t.index ["school_id"], name: "index_school_classes_on_school_id" end + create_table "school_email_domains", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.uuid "school_id", null: false + t.string "domain", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["school_id", "domain"], name: "index_school_email_domains_on_school_id_and_domain", unique: true + t.index ["school_id"], name: "index_school_email_domains_on_school_id" + end + create_table "school_import_results", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.uuid "job_id", null: false t.uuid "user_id", null: false @@ -398,6 +407,7 @@ add_foreign_key "projects", "schools" add_foreign_key "roles", "schools" add_foreign_key "school_classes", "schools" + add_foreign_key "school_email_domains", "schools" add_foreign_key "school_project_transitions", "school_projects" add_foreign_key "school_projects", "projects" add_foreign_key "school_projects", "schools" diff --git a/lib/school_email_domain_validator.rb b/lib/school_email_domain_validator.rb new file mode 100644 index 000000000..9534020b3 --- /dev/null +++ b/lib/school_email_domain_validator.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module SchoolEmailDomainValidator + class Error < StandardError + def error_code + :invalid + end + end + + class BlankDomainError < Error + def error_code + :blank + end + end + + class InvalidURIError < Error + def error_code + :invalid_uri + end + end + + class InvalidHostError < Error + def error_code + :invalid_host + end + end + + class PublicSuffixError < Error + def error_code + :invalid_public_suffix + end + end + + def self.call(domain) + raise BlankDomainError if domain.blank? + + validate_domain(domain) + end + + def self.validate_domain(domain) + value = domain.strip.downcase + # Add a scheme unless it already has one, so URI can parse it + value = "http://#{value}" unless %r{\A[a-z][a-z0-9+\-.]*://}i.match?(value) + uri = URI.parse(value) + host = uri.host&.delete_suffix('.') + + validate_host(host) + rescue URI::InvalidURIError + raise InvalidURIError + end + + def self.validate_host(host) + accounts_host_format = + /\A\s*(?:[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?\.)+[A-Za-z]{2,63}\s*\z/i + + raise InvalidHostError unless host&.match?(accounts_host_format) + + raise PublicSuffixError, 'domain has no registered public suffix' unless PublicSuffix.valid?(host) + + host + end +end diff --git a/spec/lib/school_email_domain_validator_spec.rb b/spec/lib/school_email_domain_validator_spec.rb new file mode 100644 index 000000000..1ff466bbe --- /dev/null +++ b/spec/lib/school_email_domain_validator_spec.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe SchoolEmailDomainValidator do + describe '.call' do + it 'returns success with a normalised host for a valid domain' do + result = described_class.call('example.com') + + expect(result).to eq('example.com') + end + + context 'with a valid domain' do + it 'extracts the host for http' do + result = described_class.call('http://mail.school.edu/path?query=1') + expect(result).to eq('mail.school.edu') + end + + it 'extracts the host for https' do + result = described_class.call('https://mail.school.edu/path?query=1') + expect(result).to eq('mail.school.edu') + end + + it 'removes a trailing dot' do + result = described_class.call('example.edu.') + expect(result).to eq('example.edu') + end + + it 'lowercases the host' do + result = described_class.call('EXAMPLE.EDU') + expect(result).to eq('example.edu') + end + + it 'removes a leading @' do + result = described_class.call('@example.edu') + expect(result).to eq('example.edu') + end + + it 'accepts a subdomain' do + result = described_class.call('mail.example.edu') + expect(result).to eq('mail.example.edu') + end + + it 'accepts a hostname er a multi-part public suffix' do + result = described_class.call('school.example.co.uk') + expect(result).to eq('school.example.co.uk') + end + + it 'accepts a district-style with a multi-part public suffix' do + result = described_class.call('school.k12.tx.us') + expect(result).to eq('school.k12.tx.us') + end + end + + context 'when the domain is blank' do + it 'raises BlankDomain' do + expect { described_class.call('') }.to raise_error(SchoolEmailDomainValidator::BlankDomainError) + end + end + + context 'when the domain has an invalid URI' do + it 'raises URI::InvalidURIError' do + expect { described_class.call('https://exa mple.com') }.to raise_error(SchoolEmailDomainValidator::InvalidURIError) + end + end + + context 'when the host does not match accounts_host_format' do + it 'raises InvalidHostError' do + expect { described_class.call('school_domain.edu') } + .to raise_error(SchoolEmailDomainValidator::InvalidHostError) + end + end + + context 'when the host fails PublicSuffix validation' do + it 'raises PublicSuffixError' do + expect { described_class.call('co.uk') } + .to raise_error(SchoolEmailDomainValidator::PublicSuffixError) + end + end + end +end diff --git a/spec/models/school_email_domain_spec.rb b/spec/models/school_email_domain_spec.rb new file mode 100644 index 000000000..d8695eb60 --- /dev/null +++ b/spec/models/school_email_domain_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe SchoolEmailDomain do + subject(:school_email_domain) { described_class.create!(school:, domain:) } + + let(:school) { create(:school, creator_id: SecureRandom.uuid) } + let(:domain) { 'example.edu' } + + describe 'associations' do + it { is_expected.to belong_to(:school) } + it { is_expected.to validate_presence_of(:domain) } + it { is_expected.to be_valid } + end + + context 'with a valid domain' do + it 'accepts the domain' do + expect(school_email_domain.domain).to eq('example.edu') + end + + describe 'domain normalisation' do + let(:domain) { '@EXAMPLE.EDU.' } + + it 'normalises a domain' do + expect(school_email_domain.domain).to eq('example.edu') + end + end + + describe 'domain uniqueness' do + context 'when the proposed domain matches the existing record' do + subject(:school_email_domain) { described_class.new(school:, domain:) } + + let(:domain) { 'example.edu' } + + before do + described_class.create!(school:, domain: 'example.edu') + school_email_domain.valid? + end + + it 'rejects the duplicate' do + expect(school_email_domain).not_to be_valid + end + + it 'records :taken on domain' do + expect(school_email_domain.errors.of_kind?(:domain, :taken)).to be(true) + end + end + + context 'when the proposed domain matches after normalisation' do + subject(:school_email_domain) { described_class.new(school:, domain:) } + + let(:domain) { 'http://EXAMPLE.EDU' } + + before do + described_class.create!(school:, domain: 'example.edu') + school_email_domain.valid? + end + + it 'rejects the duplicate' do + expect(school_email_domain).not_to be_valid + end + + it 'records :taken on domain' do + expect(school_email_domain.errors.of_kind?(:domain, :taken)).to be(true) + end + end + + it 'allows the same domain for a different school' do + described_class.create!(school:, domain: 'example.edu') + other_school = create(:school, creator_id: SecureRandom.uuid) + other_school_email_domain = described_class.new(school: other_school, domain: 'example.edu') + + expect(other_school_email_domain).to be_valid + end + end + end + + context 'with an invalid domain' do + it { is_expected.not_to allow_value('').for(:domain) } + it { is_expected.not_to allow_value(' ').for(:domain) } + it { is_expected.not_to allow_value('http://').for(:domain) } + it { is_expected.not_to allow_value('edu').for(:domain) } + it { is_expected.not_to allow_value('com').for(:domain) } + it { is_expected.not_to allow_value('co.uk').for(:domain) } + it { is_expected.not_to allow_value('http://invalid uri').for(:domain) } + it { is_expected.not_to allow_value('-wrong.edu').for(:domain) } + end +end diff --git a/spec/models/school_spec.rb b/spec/models/school_spec.rb index 7aec97db1..4a2812690 100644 --- a/spec/models/school_spec.rb +++ b/spec/models/school_spec.rb @@ -35,6 +35,12 @@ expect(school.roles.size).to eq(2) end + it 'has many school email domains' do + SchoolEmailDomain.create!(school:, domain: 'example.edu') + SchoolEmailDomain.create!(school:, domain: 'other.edu') + expect(school.school_email_domains.size).to eq(2) + end + context 'when a school is destroyed' do let!(:school_class) { create(:school_class, school:, teacher_ids: [teacher.id]) } let!(:lesson_1) { create(:lesson, user_id: teacher.id, school_class:) } @@ -88,6 +94,11 @@ school.destroy! expect(role.reload.school_id).to be_nil end + + it 'also destroys school email domains' do + SchoolEmailDomain.create!(school:, domain: 'example.edu') + expect { school.destroy! }.to change(SchoolEmailDomain, :count).by(-1) + end end end @@ -671,4 +682,22 @@ expect(school.reopen).to be(false) end end + + describe '#valid_domain?' do + let(:valid_domain) { 'valid.edu' } + let(:unregistered_domain) { 'invalid.edu' } + let(:invalid_domain) { 'not a domain' } + + before do + SchoolEmailDomain.create!(school:, domain: valid_domain) + end + + it 'returns true when school has registered the email domain' do + expect(school.valid_domain?(valid_domain)).to be(true) + end + + it 'returns false when school has not registered the email domain' do + expect(school.valid_domain?(unregistered_domain)).to be(false) + end + end end