Skip to content

Commit 916c66a

Browse files
Store school email domains (#787)
## Status Closes [#1325](RaspberryPiFoundation/digital-editor-issues#1325) ## What's changed? - Add a `school_email_domains` table - Add a `SchoolEmailDomain` model - Add a `SchoolEmailDomainValidator` that normalises and checks domains - Update association on `School` - Add `valid_domain?` method on `School` - Add and update specs
1 parent 294d991 commit 916c66a

10 files changed

Lines changed: 315 additions & 0 deletions

Gemfile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ gem 'paper_trail'
3838
gem 'pg', '~> 1.6'
3939
gem 'postmark-rails'
4040
gem 'propshaft'
41+
gem 'public_suffix', '~> 7.0'
4142
gem 'puma', '~> 8.0'
4243
gem 'rack_content_type_default', '~> 1.1'
4344
gem 'rack-cors'

Gemfile.lock

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,7 @@ DEPENDENCIES
615615
postmark-rails
616616
propshaft
617617
pry-byebug
618+
public_suffix (~> 7.0)
618619
puma (~> 8.0)
619620
rack-cors
620621
rack_content_type_default (~> 1.1)

app/models/school.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ class School < ApplicationRecord
66
has_many :projects, dependent: :nullify
77
has_many :roles, dependent: :nullify
88
has_many :school_projects, dependent: :nullify
9+
has_many :school_email_domains, dependent: :destroy
910

1011
VALID_URL_REGEX = %r{\A(?:https?://)?(?:www.)?[a-z0-9]+([-.]{1}[a-z0-9]+)*\.[a-z]{2,63}(\.[a-z]{2,63})*(/.*)?\z}ix
1112

@@ -116,6 +117,13 @@ def import_in_progress?
116117
.exists?(description: id)
117118
end
118119

120+
def valid_domain?(candidate_domain)
121+
validated_domain = SchoolEmailDomainValidator.call(candidate_domain)
122+
school_email_domains.exists?(domain: validated_domain)
123+
rescue ::SchoolEmailDomainValidator::Error
124+
false
125+
end
126+
119127
private
120128

121129
# Ensure the reference is nil, not an empty string

app/models/school_email_domain.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# frozen_string_literal: true
2+
3+
class SchoolEmailDomain < ApplicationRecord
4+
belongs_to :school
5+
6+
validates :domain, presence: true
7+
validates :domain, uniqueness: { scope: :school_id }
8+
9+
before_validation :validate_domain
10+
11+
private
12+
13+
def validate_domain
14+
self.domain = SchoolEmailDomainValidator.call(domain)
15+
rescue ::SchoolEmailDomainValidator::Error => e
16+
errors.add(:domain, e.error_code)
17+
end
18+
end
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
# frozen_string_literal: true
2+
3+
class CreateSchoolEmailDomains < ActiveRecord::Migration[7.2]
4+
def change
5+
create_table :school_email_domains, id: :uuid do |t|
6+
t.references :school, null: false, foreign_key: true, type: :uuid
7+
t.string :domain, null: false
8+
9+
t.timestamps
10+
end
11+
12+
add_index :school_email_domains, %i[school_id domain], unique: true
13+
end
14+
end

db/schema.rb

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
# frozen_string_literal: true
2+
3+
module SchoolEmailDomainValidator
4+
class Error < StandardError
5+
def error_code
6+
:invalid
7+
end
8+
end
9+
10+
class BlankDomainError < Error
11+
def error_code
12+
:blank
13+
end
14+
end
15+
16+
class InvalidURIError < Error
17+
def error_code
18+
:invalid_uri
19+
end
20+
end
21+
22+
class InvalidHostError < Error
23+
def error_code
24+
:invalid_host
25+
end
26+
end
27+
28+
class PublicSuffixError < Error
29+
def error_code
30+
:invalid_public_suffix
31+
end
32+
end
33+
34+
def self.call(domain)
35+
raise BlankDomainError if domain.blank?
36+
37+
validate_domain(domain)
38+
end
39+
40+
def self.validate_domain(domain)
41+
value = domain.strip.downcase
42+
# Add a scheme unless it already has one, so URI can parse it
43+
value = "http://#{value}" unless %r{\A[a-z][a-z0-9+\-.]*://}i.match?(value)
44+
uri = URI.parse(value)
45+
host = uri.host&.delete_suffix('.')
46+
47+
validate_host(host)
48+
rescue URI::InvalidURIError
49+
raise InvalidURIError
50+
end
51+
52+
def self.validate_host(host)
53+
# School email domains are sent to Profile, which validates them using a "fully qualified domain name" regex
54+
# Any changes to accounts_host_format must align with the pattern that Profile applies in its app/lib/fqdn.js
55+
accounts_host_format =
56+
/\A\s*(?:[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?\.)+[A-Za-z]{2,63}\s*\z/i
57+
58+
raise InvalidHostError unless host&.match?(accounts_host_format)
59+
60+
raise PublicSuffixError, 'domain has no registered public suffix' unless PublicSuffix.valid?(host)
61+
62+
host
63+
end
64+
end
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
# frozen_string_literal: true
2+
3+
require 'rails_helper'
4+
5+
RSpec.describe SchoolEmailDomainValidator do
6+
describe '.call' do
7+
it 'returns success with a normalised host for a valid domain' do
8+
result = described_class.call('example.com')
9+
10+
expect(result).to eq('example.com')
11+
end
12+
13+
context 'with a valid domain' do
14+
it 'extracts the host for http' do
15+
result = described_class.call('http://mail.school.edu/path?query=1')
16+
expect(result).to eq('mail.school.edu')
17+
end
18+
19+
it 'extracts the host for https' do
20+
result = described_class.call('https://mail.school.edu/path?query=1')
21+
expect(result).to eq('mail.school.edu')
22+
end
23+
24+
it 'removes a trailing dot' do
25+
result = described_class.call('example.edu.')
26+
expect(result).to eq('example.edu')
27+
end
28+
29+
it 'lowercases the host' do
30+
result = described_class.call('EXAMPLE.EDU')
31+
expect(result).to eq('example.edu')
32+
end
33+
34+
it 'removes a leading @' do
35+
result = described_class.call('@example.edu')
36+
expect(result).to eq('example.edu')
37+
end
38+
39+
it 'accepts a subdomain' do
40+
result = described_class.call('mail.example.edu')
41+
expect(result).to eq('mail.example.edu')
42+
end
43+
44+
it 'accepts a hostname er a multi-part public suffix' do
45+
result = described_class.call('school.example.co.uk')
46+
expect(result).to eq('school.example.co.uk')
47+
end
48+
49+
it 'accepts a district-style with a multi-part public suffix' do
50+
result = described_class.call('school.k12.tx.us')
51+
expect(result).to eq('school.k12.tx.us')
52+
end
53+
end
54+
55+
context 'when the domain is blank' do
56+
it 'raises BlankDomain' do
57+
expect { described_class.call('') }.to raise_error(SchoolEmailDomainValidator::BlankDomainError)
58+
end
59+
end
60+
61+
context 'when the domain has an invalid URI' do
62+
it 'raises InvalidURIError' do
63+
expect { described_class.call('https://exa mple.com') }.to raise_error(SchoolEmailDomainValidator::InvalidURIError)
64+
end
65+
end
66+
67+
context 'when the host does not match accounts_host_format' do
68+
it 'raises InvalidHostError' do
69+
expect { described_class.call('school_domain.edu') }
70+
.to raise_error(SchoolEmailDomainValidator::InvalidHostError)
71+
end
72+
end
73+
74+
context 'when the host fails PublicSuffix validation' do
75+
it 'raises PublicSuffixError' do
76+
expect { described_class.call('co.uk') }
77+
.to raise_error(SchoolEmailDomainValidator::PublicSuffixError)
78+
end
79+
end
80+
end
81+
end
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
# frozen_string_literal: true
2+
3+
require 'rails_helper'
4+
5+
RSpec.describe SchoolEmailDomain do
6+
subject(:school_email_domain) { described_class.create!(school:, domain:) }
7+
8+
let(:school) { create(:school, creator_id: SecureRandom.uuid) }
9+
let(:domain) { 'example.edu' }
10+
11+
describe 'associations' do
12+
it { is_expected.to belong_to(:school) }
13+
it { is_expected.to validate_presence_of(:domain) }
14+
it { is_expected.to be_valid }
15+
end
16+
17+
context 'with a valid domain' do
18+
it 'accepts the domain' do
19+
expect(school_email_domain.domain).to eq('example.edu')
20+
end
21+
22+
describe 'domain normalisation' do
23+
let(:domain) { '@EXAMPLE.EDU.' }
24+
25+
it 'normalises a domain' do
26+
expect(school_email_domain.domain).to eq('example.edu')
27+
end
28+
end
29+
30+
describe 'domain uniqueness' do
31+
context 'when the proposed domain matches the existing record' do
32+
subject(:school_email_domain) { described_class.new(school:, domain:) }
33+
34+
let(:domain) { 'example.edu' }
35+
36+
before do
37+
described_class.create!(school:, domain: 'example.edu')
38+
school_email_domain.valid?
39+
end
40+
41+
it 'rejects the duplicate' do
42+
expect(school_email_domain).not_to be_valid
43+
end
44+
45+
it 'records :taken on domain' do
46+
expect(school_email_domain.errors.of_kind?(:domain, :taken)).to be(true)
47+
end
48+
end
49+
50+
context 'when the proposed domain matches after normalisation' do
51+
subject(:school_email_domain) { described_class.new(school:, domain:) }
52+
53+
let(:domain) { 'http://EXAMPLE.EDU' }
54+
55+
before do
56+
described_class.create!(school:, domain: 'example.edu')
57+
school_email_domain.valid?
58+
end
59+
60+
it 'rejects the duplicate' do
61+
expect(school_email_domain).not_to be_valid
62+
end
63+
64+
it 'records :taken on domain' do
65+
expect(school_email_domain.errors.of_kind?(:domain, :taken)).to be(true)
66+
end
67+
end
68+
69+
it 'allows the same domain for a different school' do
70+
described_class.create!(school:, domain: 'example.edu')
71+
other_school = create(:school, creator_id: SecureRandom.uuid)
72+
other_school_email_domain = described_class.new(school: other_school, domain: 'example.edu')
73+
74+
expect(other_school_email_domain).to be_valid
75+
end
76+
end
77+
end
78+
79+
context 'with an invalid domain' do
80+
it { is_expected.not_to allow_value('').for(:domain) }
81+
it { is_expected.not_to allow_value(' ').for(:domain) }
82+
it { is_expected.not_to allow_value('http://').for(:domain) }
83+
it { is_expected.not_to allow_value('edu').for(:domain) }
84+
it { is_expected.not_to allow_value('com').for(:domain) }
85+
it { is_expected.not_to allow_value('co.uk').for(:domain) }
86+
it { is_expected.not_to allow_value('http://invalid uri').for(:domain) }
87+
it { is_expected.not_to allow_value('-wrong.edu').for(:domain) }
88+
end
89+
end

spec/models/school_spec.rb

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,12 @@
3535
expect(school.roles.size).to eq(2)
3636
end
3737

38+
it 'has many school email domains' do
39+
SchoolEmailDomain.create!(school:, domain: 'example.edu')
40+
SchoolEmailDomain.create!(school:, domain: 'other.edu')
41+
expect(school.school_email_domains.size).to eq(2)
42+
end
43+
3844
context 'when a school is destroyed' do
3945
let!(:school_class) { create(:school_class, school:, teacher_ids: [teacher.id]) }
4046
let!(:lesson_1) { create(:lesson, user_id: teacher.id, school_class:) }
@@ -88,6 +94,11 @@
8894
school.destroy!
8995
expect(role.reload.school_id).to be_nil
9096
end
97+
98+
it 'also destroys school email domains' do
99+
SchoolEmailDomain.create!(school:, domain: 'example.edu')
100+
expect { school.destroy! }.to change(SchoolEmailDomain, :count).by(-1)
101+
end
91102
end
92103
end
93104

@@ -671,4 +682,22 @@
671682
expect(school.reopen).to be(false)
672683
end
673684
end
685+
686+
describe '#valid_domain?' do
687+
let(:valid_domain) { 'valid.edu' }
688+
let(:unregistered_domain) { 'invalid.edu' }
689+
let(:invalid_domain) { 'not a domain' }
690+
691+
before do
692+
SchoolEmailDomain.create!(school:, domain: valid_domain)
693+
end
694+
695+
it 'returns true when school has registered the email domain' do
696+
expect(school.valid_domain?(valid_domain)).to be(true)
697+
end
698+
699+
it 'returns false when school has not registered the email domain' do
700+
expect(school.valid_domain?(unregistered_domain)).to be(false)
701+
end
702+
end
674703
end

0 commit comments

Comments
 (0)