diff --git a/app/controllers/api/school_email_domains_controller.rb b/app/controllers/api/school_email_domains_controller.rb new file mode 100644 index 000000000..39e3b5865 --- /dev/null +++ b/app/controllers/api/school_email_domains_controller.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Api + class SchoolEmailDomainsController < ApiController + before_action :authorize_user + load_and_authorize_resource :school + authorize_resource :school_email_domain, class: false + + def index + render json: school_email_domains, status: :ok + end + + def create + result = SchoolEmailDomain::Create.call(school: @school, domain: school_email_domain_params[:domain], token: current_user.token) + if result.success? + render json: { domain: result[:school_email_domain].domain }, status: :created + else + render json: { error: result[:error] }, status: :unprocessable_content + end + end + + private + + def school_email_domains + @school.school_email_domains.order(:created_at).pluck(:domain) + end + + def school_email_domain_params + params.expect(school_email_domain: [:domain]) + end + end +end diff --git a/app/models/ability.rb b/app/models/ability.rb index 8365cf7a3..446c54750 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -72,6 +72,7 @@ def define_school_owner_abilities(school:) can(%i[read update destroy], Lesson, school_id: school.id, visibility: %w[teachers students public]) can(%i[read destroy], Feedback, school_project: { school_id: school.id }) can(%i[exchange_code], :google_auth) + can(%i[read create], :school_email_domain) end def define_school_teacher_abilities(user:, school:) @@ -102,6 +103,7 @@ def define_school_teacher_abilities(user:, school:) can(%i[show_status unsubmit return complete], SchoolProject, project: { remixed_from_id: teacher_project_ids }) can(%i[read create destroy], Feedback, school_project: { project: { remixed_from_id: teacher_project_ids } }) can(%i[exchange_code], :google_auth) + can(%i[read create], :school_email_domain) end def define_school_student_abilities(user:, school:) diff --git a/config/locales/en.yml b/config/locales/en.yml index ce3d9c076..664fc5670 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -28,3 +28,12 @@ en: attributes: school_class: import_id: "Import id" + errors: + models: + school_email_domain: + attributes: + domain: + invalid: "is invalid" + invalid_host: "must be a fully qualified domain name" + invalid_public_suffix: "must be a registrable domain name" + invalid_uri: "must be a valid domain format" diff --git a/config/routes.rb b/config/routes.rb index b85c9f545..581c92710 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -82,6 +82,7 @@ post :batch, on: :collection, to: 'school_students#create_batch' delete :batch, on: :collection, to: 'school_students#destroy_batch' end + resources :school_email_domains, only: %i[index create], controller: 'school_email_domains' end resources :lessons, only: %i[index create show update destroy] do diff --git a/lib/concepts/school_email_domain/operations/create.rb b/lib/concepts/school_email_domain/operations/create.rb new file mode 100644 index 000000000..2059b8508 --- /dev/null +++ b/lib/concepts/school_email_domain/operations/create.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +class SchoolEmailDomain + class Create + class << self + def call(school:, domain:, token:) + response = OperationResponse.new + response[:school_email_domain] = build_domain(school, domain) + SchoolEmailDomain.transaction do + response[:school_email_domain].save! + update_profile(school, token) + end + response + rescue ActiveRecord::RecordInvalid => e + record = response[:school_email_domain] || e.record + response[:error] = record.errors.full_messages.join(', ') + response + rescue ActiveRecord::RecordNotUnique + record = response[:school_email_domain] + record.errors.add(:domain, :taken) + response[:error] = record.errors.full_messages.join(', ') + response + rescue StandardError => e + Sentry.capture_exception(e) # Send unexpected/Profile errors to Sentry + response[:error] = e.message + response + end + + private + + def build_domain(school, domain) + school.school_email_domains.build(domain:) + end + + def update_profile(school, token) + school_email_domains = school.school_email_domains.order(:created_at).pluck(:domain) + ProfileApiClient.update_school_email_domains(token:, school_id: school.id, school_email_domains:) + end + end + end +end diff --git a/spec/concepts/school_email_domain/create_spec.rb b/spec/concepts/school_email_domain/create_spec.rb new file mode 100644 index 000000000..49b541a37 --- /dev/null +++ b/spec/concepts/school_email_domain/create_spec.rb @@ -0,0 +1,179 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe SchoolEmailDomain::Create, type: :unit do + let(:school) { create(:school) } + let(:domain) { 'school.edu' } + let(:token) { UserProfileMock::TOKEN } + + before { stub_profile_api_update_school_email_domains } + + context 'with valid values' do + it 'returns a successful operation response' do + response = described_class.call(school:, domain:, token:) + expect(response.success?).to be(true) + end + + it 'creates a school email domain' do + expect { described_class.call(school:, domain:, token:) }.to change(SchoolEmailDomain, :count).by(1) + end + + it 'returns the domain in the operation response' do + response = described_class.call(school:, domain:, token:) + expect(response[:school_email_domain]).to be_a(SchoolEmailDomain) + end + + it 'assigns the domain' do + response = described_class.call(school:, domain:, token:) + expect(response[:school_email_domain].domain).to eq(domain) + end + + it 'assigns the school' do + response = described_class.call(school:, domain:, token:) + expect(response[:school_email_domain].school_id).to eq(school.id) + end + + it 'syncs the domains to Profile' do + described_class.call(school:, domain:, token:) + expect(ProfileApiClient).to have_received(:update_school_email_domains).with( + token:, + school_id: school.id, + school_email_domains: [domain] + ) + end + + context 'when multiple domains already exist' do + before do + create(:school_email_domain, school:, domain: 'first.edu') + create(:school_email_domain, school:, domain: 'second.edu') + create(:school_email_domain, school:, domain: 'third.edu') + end + + it 'syncs all domains to Profile' do + described_class.call(school:, domain:, token:) + expect(ProfileApiClient).to have_received(:update_school_email_domains).with( + token:, + school_id: school.id, + school_email_domains: ['first.edu', 'second.edu', 'third.edu', domain] + ) + end + end + end + + shared_examples 'an invalid record' do + before { allow(Sentry).to receive(:capture_exception) } + + it 'does not create a school email domain' do + expect { described_class.call(school:, domain:, token:) }.not_to change(SchoolEmailDomain, :count) + end + + it 'returns a failed operation response' do + response = described_class.call(school:, domain:, token:) + expect(response.failure?).to be(true) + end + + it 'does not send the exception to Sentry' do + described_class.call(school:, domain:, token:) + expect(Sentry).not_to have_received(:capture_exception).with(kind_of(StandardError)) + end + + it 'returns the error message in the operation response' do + response = described_class.call(school:, domain:, token:) + expect(response[:error]).to include('') + end + + it 'does not attempt to update Profile' do + described_class.call(school:, domain:, token:) + expect(ProfileApiClient).not_to have_received(:update_school_email_domains) + end + end + + context 'when domain is blank' do + let(:domain) { '' } + let(:expected_error_message) { "Domain can't be blank" } + + it_behaves_like 'an invalid record' + end + + context 'when domain is not an FQDN' do + let(:domain) { 'edu' } + let(:expected_error_message) { 'Domain must be a fully qualified domain name' } + + it_behaves_like 'an invalid record' + end + + context 'when domain has an invalid URI' do + let(:domain) { 'exa mple.com' } + let(:expected_error_message) { 'Domain must be a valid domain format' } + + it_behaves_like 'an invalid record' + end + + context 'when domain has an invalid public suffix' do + let(:domain) { 'co.uk' } + let(:expected_error_message) { 'Domain must be a registrable domain name' } + + it_behaves_like 'an invalid record' + end + + context 'when domain is a duplicate' do + before { create(:school_email_domain, school:, domain:) } + + let(:expected_error_message) { 'Domain has already been taken' } + + it_behaves_like 'an invalid record' + end + + context 'when a concurrent request creates the same domain' do + let(:expected_error_message) { 'Domain has already been taken' } + let(:school_email_domain) { SchoolEmailDomain.new(school:, domain:) } + + before do + allow(Sentry).to receive(:capture_exception) + allow(school.school_email_domains).to receive(:build).with(domain:).and_return(school_email_domain) + allow(school_email_domain).to receive(:save!).and_raise(ActiveRecord::RecordNotUnique) + end + + it_behaves_like 'an invalid record' + end + + context 'when Profile sync fails' do + let(:profile_error) do + ProfileApiClient::UnexpectedResponse.new( + instance_double(Faraday::Response, status: 500, headers: {}, body: '') + ) + end + + before do + allow(Sentry).to receive(:capture_exception) + + allow(ProfileApiClient).to receive(:update_school_email_domains) + .and_raise(profile_error) + end + + it 'attempts to sync to Profile' do + described_class.call(school:, domain:, token:) + expect(ProfileApiClient).to have_received(:update_school_email_domains).once + end + + it 'does not persist the domain' do + expect { described_class.call(school:, domain:, token:) } + .not_to change(SchoolEmailDomain, :count) + end + + it 'sends the exception to Sentry' do + described_class.call(school:, domain:, token:) + expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) + end + + it 'returns a failed operation response' do + expect(described_class.call(school:, domain:, token:)).to be_failure + end + + it 'returns the error message in the operation response' do + response = described_class.call(school:, domain:, token:) + expect(response[:error]).to eq('Unexpected response from Profile API (status code 500)') + end + end +end diff --git a/spec/factories/school_email_domain.rb b/spec/factories/school_email_domain.rb new file mode 100644 index 000000000..eb0f2802b --- /dev/null +++ b/spec/factories/school_email_domain.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :school_email_domain do + school + sequence(:domain) { |n| "domain#{n}.example.edu" } + end +end diff --git a/spec/features/school_email_domain/creating_school_email_domains_spec.rb b/spec/features/school_email_domain/creating_school_email_domains_spec.rb new file mode 100644 index 000000000..b0d686234 --- /dev/null +++ b/spec/features/school_email_domain/creating_school_email_domains_spec.rb @@ -0,0 +1,209 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Creating school email domains', type: :request do + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + let(:school) { create(:school) } + let(:domain) { 'school.edu' } + let(:params) do + { + school_email_domain: { + domain: + } + } + end + let(:owner) { create(:owner, school:, name: 'School Owner') } + + before { stub_profile_api_update_school_email_domains } + + describe '#create' do + shared_examples 'a successful school email domain creation response' do + it 'responds 201 created' do + expect(response).to have_http_status(:created) + end + + it 'creates the domain' do + expect(SchoolEmailDomain.exists?(school:, domain:)).to be(true) + end + + it 'returns the domain' do + data = JSON.parse(response.body, symbolize_names: true) + expect(data[:domain]).to eq(domain) + end + end + + shared_examples 'an unprocessable school email domain creation response' do + before do + authenticated_in_hydra_as(owner) + post("/api/schools/#{school.id}/school_email_domains", headers:, params:) + end + + it 'responds 422 Unprocessable Entity' do + expect(response).to have_http_status(:unprocessable_content) + end + + it 'returns the error in the response body' do + data = JSON.parse(response.body, symbolize_names: true) + expect(data[:error]).to match(expected_error) + end + end + + context 'with an authorised owner' do + before do + authenticated_in_hydra_as(owner) + post("/api/schools/#{school.id}/school_email_domains", headers:, params:) + end + + it_behaves_like 'a successful school email domain creation response' + end + + context 'with an authorised teacher' do + let(:teacher) { create(:teacher, school:, name: 'School Teacher') } + + before do + authenticated_in_hydra_as(teacher) + post("/api/schools/#{school.id}/school_email_domains", headers:, params:) + end + + it_behaves_like 'a successful school email domain creation response' + end + + context 'with missing params' do + it 'responds 400 Bad Request when params are missing' do + authenticated_in_hydra_as(owner) + + post("/api/schools/#{school.id}/school_email_domains", headers:) + + expect(response).to have_http_status(:bad_request) + end + end + + context 'when the user does not have access' do + it 'responds 403 Forbidden when the user is a school-owner for a different school' do + other_school = create(:school) + other_owner = create(:owner, school: other_school, name: 'School Owner') + authenticated_in_hydra_as(other_owner) + + post("/api/schools/#{school.id}/school_email_domains", headers:, params:) + expect(response).to have_http_status(:forbidden) + end + + it 'responds 403 Forbidden when the user is a school-teacher for a different school' do + other_school = create(:school) + other_teacher = create(:teacher, school: other_school, name: 'School Teacher') + authenticated_in_hydra_as(other_teacher) + + post("/api/schools/#{school.id}/school_email_domains", headers:, params:) + expect(response).to have_http_status(:forbidden) + end + + it 'responds 403 Forbidden when the user is a student at the school' do + student = create(:student, school:) + authenticated_in_hydra_as(student) + + post("/api/schools/#{school.id}/school_email_domains", headers:, params:) + expect(response).to have_http_status(:forbidden) + end + end + + context 'when the user is not authenticated' do + it 'responds 401 Unauthorized when no token is given' do + post "/api/schools/#{school.id}/school_email_domains" + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when the domain cannot be processed' do + context 'when the domain is blank' do + let(:domain) { '' } + let(:expected_error) { /Domain can't be blank/ } + + it_behaves_like 'an unprocessable school email domain creation response' + + it 'does not create a school email domain' do + authenticated_in_hydra_as(owner) + post("/api/schools/#{school.id}/school_email_domains", headers:, params:) + expect(SchoolEmailDomain.where(school:)).to be_none + end + end + + context 'when the domain is not an FQDN' do + let(:domain) { 'edu' } + let(:expected_error) { /Domain must be a fully qualified domain name/ } + + it_behaves_like 'an unprocessable school email domain creation response' + + it 'does not create a school email domain' do + authenticated_in_hydra_as(owner) + post("/api/schools/#{school.id}/school_email_domains", headers:, params:) + expect(SchoolEmailDomain.where(school:)).to be_none + end + end + + context 'when the uri is invalid' do + let(:domain) { 'exa mple.com' } + let(:expected_error) { /Domain must be a valid domain format/ } + + it_behaves_like 'an unprocessable school email domain creation response' + + it 'does not create a school email domain' do + authenticated_in_hydra_as(owner) + post("/api/schools/#{school.id}/school_email_domains", headers:, params:) + expect(SchoolEmailDomain.where(school:)).to be_none + end + end + + context 'when the public suffix is invalid' do + let(:domain) { 'co.uk' } + let(:expected_error) { /Domain must be a registrable domain name/ } + + it_behaves_like 'an unprocessable school email domain creation response' + + it 'does not create a school email domain' do + authenticated_in_hydra_as(owner) + post("/api/schools/#{school.id}/school_email_domains", headers:, params:) + expect(SchoolEmailDomain.where(school:)).to be_none + end + end + + context 'when the domain is a duplicate' do + let(:expected_error) { /Domain has already been taken/ } + + before do + create(:school_email_domain, school:, domain:) + end + + it_behaves_like 'an unprocessable school email domain creation response' + + it 'does not create a duplicate school email domain' do + authenticated_in_hydra_as(owner) + post("/api/schools/#{school.id}/school_email_domains", headers:, params:) + expect(SchoolEmailDomain.where(school:, domain:).count).to eq(1) + end + end + end + + context 'when Profile sync fails' do + let(:profile_error) do + ProfileApiClient::UnexpectedResponse.new( + instance_double(Faraday::Response, status: 500, headers: {}, body: '') + ) + end + let(:expected_error) { /Unexpected response from Profile API \(status code 500\)/ } + + before do + allow(ProfileApiClient).to receive(:update_school_email_domains).and_raise(profile_error) + end + + it_behaves_like 'an unprocessable school email domain creation response' + + it 'does not persist the domain' do + authenticated_in_hydra_as(owner) + post("/api/schools/#{school.id}/school_email_domains", headers:, params:) + + expect(SchoolEmailDomain.exists?(school:, domain:)).to be(false) + end + end + end +end diff --git a/spec/features/school_email_domain/listing_school_email_domains_spec.rb b/spec/features/school_email_domain/listing_school_email_domains_spec.rb new file mode 100644 index 000000000..87e8d8758 --- /dev/null +++ b/spec/features/school_email_domain/listing_school_email_domains_spec.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Listing school email domains', type: :request do + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + let(:school) { create(:school) } + let(:school_email_domains) { create_list(:school_email_domain, 5, school: school) } + + before do + school_email_domains + end + + describe '#index' do + shared_examples 'a successful school email domains index' do + it 'responds 200 OK' do + expect(response).to have_http_status(:ok) + end + + context 'when the school has domains' do + it 'returns a list of domains for the school' do + data = JSON.parse(response.body, symbolize_names: true) + expect(data).to match_array(school.school_email_domains.pluck(:domain)) + end + end + + context 'when domains do not belong to the school' do + let(:other_school) { create(:school) } + let(:school_email_domains) { create_list(:school_email_domain, 5, school: other_school) } + + it 'returns an empty array' do + data = JSON.parse(response.body, symbolize_names: true) + expect(data).to eq([]) + end + end + end + + context 'with an authorised owner' do + let(:owner) { create(:owner, school:, name: 'School Owner') } + + before do + authenticated_in_hydra_as(owner) + get("/api/schools/#{school.id}/school_email_domains", headers:) + end + + it_behaves_like 'a successful school email domains index' + end + + context 'with an authorised teacher' do + let(:teacher) { create(:teacher, school:, name: 'School Teacher') } + + before do + authenticated_in_hydra_as(teacher) + get("/api/schools/#{school.id}/school_email_domains", headers:) + end + + it_behaves_like 'a successful school email domains index' + end + + context 'when the user does not have access' do + it 'responds 403 Forbidden when the user is a school-owner for a different school' do + other_school = create(:school) + other_owner = create(:owner, school: other_school, name: 'School Owner') + authenticated_in_hydra_as(other_owner) + + get("/api/schools/#{school.id}/school_email_domains", headers:) + expect(response).to have_http_status(:forbidden) + end + + it 'responds 403 Forbidden when the user is a school-teacher for a different school' do + other_school = create(:school) + other_teacher = create(:teacher, school: other_school, name: 'School Teacher') + authenticated_in_hydra_as(other_teacher) + + get("/api/schools/#{school.id}/school_email_domains", headers:) + expect(response).to have_http_status(:forbidden) + end + + it 'responds 403 Forbidden when the user is a student at the school' do + student = create(:student, school:) + authenticated_in_hydra_as(student) + + get("/api/schools/#{school.id}/school_email_domains", headers:) + expect(response).to have_http_status(:forbidden) + end + end + + context 'when the user is not authenticated' do + it 'responds 401 Unauthorized when no token is given' do + get "/api/schools/#{school.id}/school_email_domains" + expect(response).to have_http_status(:unauthorized) + end + end + end +end diff --git a/spec/support/profile_api_mock.rb b/spec/support/profile_api_mock.rb index b3fae6cc5..8ed2cb3a8 100644 --- a/spec/support/profile_api_mock.rb +++ b/spec/support/profile_api_mock.rb @@ -129,4 +129,8 @@ def stub_profile_api_validate_students_with_validation_error ) ) end + + def stub_profile_api_update_school_email_domains + allow(ProfileApiClient).to receive(:update_school_email_domains).and_return(true) + end end