use school id for safeguarding

Author: peconia

app/controllers/api/school_members_controller.rb

@@ -32,7 +32,8 @@ def create_teacher_safeguarding_flag
       ProfileApiClient.create_safeguarding_flag(
         token: current_user.token,
         flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher],
-        email: current_user.email
+        email: current_user.email,
+        school_id: @school.id
       )
     end
 
@@ -42,7 +43,8 @@ def create_owner_safeguarding_flag
       ProfileApiClient.create_safeguarding_flag(
         token: current_user.token,
         flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner],
-        email: current_user.email
+        email: current_user.email,
+        school_id: @school.id
       )
     end
   end

app/controllers/api/school_students_controller.rb

@@ -195,7 +195,8 @@ def create_teacher_safeguarding_flag
       ProfileApiClient.create_safeguarding_flag(
         token: current_user.token,
         flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher],
-        email: current_user.email
+        email: current_user.email,
+        school_id: @school.id
       )
     end
 
@@ -205,7 +206,8 @@ def create_owner_safeguarding_flag
       ProfileApiClient.create_safeguarding_flag(
         token: current_user.token,
         flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner],
-        email: current_user.email
+        email: current_user.email,
+        school_id: @school.id
       )
     end
   end

app/services/school_onboarding_service.rb

@@ -14,6 +14,11 @@ def onboard(token:)
 
       ProfileApiClient.create_school(token:, id: school.id, code: school.code)
     end
+  rescue ProfileApiClient::UnauthorizedError
+    # Do not log noise to sentry. 
+    # TODO: consider returning a separate error here to distinguish from other errors and return 401 from the API, not 422
+    Rails.logger.warn { "Failed to onboard school #{@school.id}: user is unauthorized" }
+    false
   rescue StandardError => e
     Sentry.capture_exception(e)
     Rails.logger.error { "Failed to onboard school #{@school.id}: #{e.message}" }

lib/profile_api_client.rb

@@ -8,7 +8,7 @@ class ProfileApiClient
 
   # rubocop:disable Naming/MethodName
   School = Data.define(:id, :schoolCode, :updatedAt, :createdAt, :discardedAt)
-  SafeguardingFlag = Data.define(:id, :userId, :flag, :email, :createdAt, :updatedAt, :discardedAt)
+  SafeguardingFlag = Data.define(:id, :userId, :schoolId, :flag, :email, :createdAt, :updatedAt, :discardedAt)
   Student = Data.define(:id, :schoolId, :name, :username, :createdAt, :updatedAt, :discardedAt, :email, :ssoProviders)
   # rubocop:enable Naming/MethodName
 
@@ -27,6 +27,8 @@ def initialize(errors)
     end
   end
 
+  class UnauthorizedError < Error; end
+
   class UnexpectedResponse < Error
     attr_reader :response_status, :response_headers, :response_body
 
@@ -50,6 +52,7 @@ def create_school(token:, id:, code:)
         }
       end
 
+      unauthorized!(response)
       raise UnexpectedResponse, response unless response.status == 201
 
       School.new(**response.body)
@@ -58,6 +61,7 @@ def create_school(token:, id:, code:)
     def school_student(token:, school_id:, student_id:)
       response = connection(token).get("/api/v1/schools/#{school_id}/students/#{student_id}")
 
+      unauthorized!(response)
       raise UnexpectedResponse, response unless response.status == 200
 
       build_student(response.body)
@@ -70,6 +74,7 @@ def list_school_students(token:, school_id:, student_ids:)
         request.body = student_ids
       end
 
+      unauthorized!(response)
       raise UnexpectedResponse, response unless response.status == 200
 
       response.body.map { |attrs| build_student(attrs) }
@@ -86,6 +91,7 @@ def create_school_student(token:, username:, password:, name:, school_id:)
         }]
       end
 
+      unauthorized!(response)
       raise UnexpectedResponse, response unless response.status == 201
 
       response.body.deep_symbolize_keys
@@ -103,6 +109,7 @@ def validate_school_students(token:, students:, school_id:)
         request.headers['Content-Type'] = 'application/json'
       end
 
+      unauthorized!(response)
       raise UnexpectedResponse, response unless response.status == 200
     rescue Faraday::UnprocessableEntityError => e
       raise Student422Error, JSON.parse(e.response_body)['errors']
@@ -119,6 +126,7 @@ def create_school_students(token:, students:, school_id:, preflight: false)
         request.headers['Content-Type'] = 'application/json'
       end
 
+      unauthorized!(response)
       raise UnexpectedResponse, response unless [200, 201].include?(response.status)
 
       response.body.deep_symbolize_keys
@@ -136,6 +144,7 @@ def create_school_students_sso(token:, students:, school_id:)
         request.headers['Content-Type'] = 'application/json'
       end
 
+      unauthorized!(response)
       raise UnexpectedResponse, response unless [200, 201].include?(response.status)
 
       response.body.map(&:deep_symbolize_keys)
@@ -154,6 +163,7 @@ def update_school_student(token:, school_id:, student_id:, name: nil, username:
         }.compact
       end
 
+      unauthorized!(response)
       raise UnexpectedResponse, response unless response.status == 200
 
       build_student(response.body)
@@ -166,28 +176,32 @@ def delete_school_student(token:, school_id:, student_id:)
 
       response = connection(token).delete("/api/v1/schools/#{school_id}/students/#{student_id}")
 
+      unauthorized!(response)
       raise UnexpectedResponse, response unless response.status == 204
     end
 
     def safeguarding_flags(token:)
       response = connection(token).get('/api/v1/safeguarding-flags')
 
+      unauthorized!(response)
       raise UnexpectedResponse, response unless response.status == 200
 
       response.body.map { |flag| SafeguardingFlag.new(**flag.symbolize_keys) }
     end
 
-    def create_safeguarding_flag(token:, flag:, email:)
+    def create_safeguarding_flag(token:, flag:, email:, school_id:)
       response = connection(token).post('/api/v1/safeguarding-flags') do |request|
-        request.body = { flag:, email: }
+        request.body = { flag:, email:, schoolId: school_id }
       end
 
+      unauthorized!(response)
       raise UnexpectedResponse, response unless [201, 303].include?(response.status)
     end
 
     def delete_safeguarding_flag(token:, flag:)
       response = connection(token).delete("/api/v1/safeguarding-flags/#{flag}")
 
+      unauthorized!(response)
       raise UnexpectedResponse, response unless response.status == 204
     end
 
@@ -197,7 +211,7 @@ def connection(token)
       Faraday.new(ENV.fetch('IDENTITY_URL')) do |faraday|
         faraday.request :json
         faraday.response :json
-        faraday.response :raise_error
+        faraday.response :raise_error, allowed_statuses: [401]
         faraday.headers = {
           'Accept' => 'application/json',
           'Authorization' => "Bearer #{token}",
@@ -218,6 +232,7 @@ def handle_student_creation_error(faraday_error)
       end
     end
 
+
     def build_student(attrs)
       symbolized_attrs = attrs.symbolize_keys
 
@@ -229,5 +244,10 @@ def build_student(attrs)
 
       Student.new(**symbolized_attrs)
     end
+
+    def unauthorized!(response)
+      # The API is only available to verified non-student users that are over 13. Others get a 401.
+      raise UnauthorizedError, 'Profile API unauthorized' if response.status == 401
+    end
   end
 end

spec/features/school_member/listing_school_members_spec.rb

@@ -94,12 +94,12 @@
 
   it 'creates the school owner safeguarding flag' do
     get("/api/schools/#{school.id}/students", headers:)
-    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email)
+    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email, school_id: school.id)
   end
 
   it 'does not create the school teacher safeguarding flag' do
     get("/api/schools/#{school.id}/students", headers:)
-    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: owner.email)
+    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: owner.email, school_id: school.id)
   end
 
   it "responds with nil attributes for students if the user profile doesn't exist" do
@@ -136,14 +136,14 @@
     authenticated_in_hydra_as(teacher)
 
     get("/api/schools/#{school.id}/students", headers:)
-    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email)
+    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email, school_id: school.id)
   end
 
   it 'creates the school teacher safeguarding flag when the user is a school teacher' do
     teacher = create(:teacher, school:)
     authenticated_in_hydra_as(teacher)
 
     get("/api/schools/#{school.id}/students", headers:)
-    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email)
+    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email, school_id: school.id)
   end
 end

spec/features/school_student/creating_a_batch_of_school_students_spec.rb

@@ -56,12 +56,12 @@
 
   it 'creates the school owner safeguarding flag' do
     post("/api/schools/#{school.id}/students/batch", headers:, params:)
-    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email)
+    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email, school_id: school.id)
   end
 
   it 'does not create the school teacher safeguarding flag' do
     post("/api/schools/#{school.id}/students/batch", headers:, params:)
-    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: owner.email)
+    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: owner.email, school_id: school.id)
   end
 
   it 'responds 202 Accepted' do
@@ -113,15 +113,15 @@
     authenticated_in_hydra_as(teacher)
 
     post("/api/schools/#{school.id}/students/batch", headers:, params:)
-    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email)
+    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email, school_id: school.id)
   end
 
   it 'creates the school teacher safeguarding flag when the user is a school-teacher' do
     teacher = create(:teacher, school:)
     authenticated_in_hydra_as(teacher)
 
     post("/api/schools/#{school.id}/students/batch", headers:, params:)
-    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email)
+    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email, school_id: school.id)
   end
 
   it 'responds 422 Unprocessable Entity when params are invalid' do

spec/features/school_student/creating_a_school_student_spec.rb

@@ -25,12 +25,12 @@
 
   it 'creates the school owner safeguarding flag' do
     post("/api/schools/#{school.id}/students", headers:, params:)
-    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email)
+    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email, school_id: school.id)
   end
 
   it 'does not create the school teacher safeguarding flag' do
     post("/api/schools/#{school.id}/students", headers:, params:)
-    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: owner.email)
+    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: owner.email, school_id: school.id)
   end
 
   it 'responds 204 No Content' do
@@ -51,15 +51,15 @@
     authenticated_in_hydra_as(teacher)
 
     post("/api/schools/#{school.id}/students", headers:, params:)
-    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email)
+    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email, school_id: school.id)
   end
 
   it 'creates the school teacher safeguarding flag when the user is a school teacher' do
     teacher = create(:teacher, school:)
     authenticated_in_hydra_as(teacher)
 
     post("/api/schools/#{school.id}/students", headers:, params:)
-    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email)
+    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email, school_id: school.id)
   end
 
   it 'responds 400 Bad Request when params are missing' do

spec/features/school_student/deleting_a_school_student_spec.rb

@@ -22,12 +22,12 @@
 
   it 'creates the school owner safeguarding flag' do
     delete("/api/schools/#{school.id}/students/#{student_id}", headers:)
-    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email)
+    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email, school_id: school.id)
   end
 
   it 'does not create the school teacher safeguarding flag' do
     delete("/api/schools/#{school.id}/students/#{student_id}", headers:)
-    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: owner.email)
+    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: owner.email, school_id: school.id)
   end
 
   it 'responds 200 OK' do
@@ -61,15 +61,15 @@
     authenticated_in_hydra_as(teacher)
 
     delete("/api/schools/#{school.id}/students/#{student_id}", headers:)
-    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email)
+    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email, school_id: school.id)
   end
 
   it 'does not create the school teacher safeguarding flag when logged in as a teacher' do
     teacher = create(:teacher, school:)
     authenticated_in_hydra_as(teacher)
 
     delete("/api/schools/#{school.id}/students/#{student_id}", headers:)
-    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email)
+    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email, school_id: school.id)
   end
 
   it 'responds 403 Forbidden when the user is a school-student' do

spec/features/school_student/listing_school_students_spec.rb

@@ -22,12 +22,12 @@
 
   it 'creates the school owner safeguarding flag' do
     get("/api/schools/#{school.id}/students", headers:)
-    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email)
+    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email, school_id: school.id)
   end
 
   it 'does not create the school teacher safeguarding flag' do
     get("/api/schools/#{school.id}/students", headers:)
-    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: owner.email)
+    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: owner.email, school_id: school.id)
   end
 
   it 'responds 200 OK when the user is a school-teacher' do
@@ -43,15 +43,15 @@
     authenticated_in_hydra_as(teacher)
 
     get("/api/schools/#{school.id}/students", headers:)
-    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email)
+    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner], email: owner.email, school_id: school.id)
   end
 
   it 'creates the school teacher safeguarding flag when the user is a school teacher' do
     teacher = create(:teacher, school:)
     authenticated_in_hydra_as(teacher)
 
     get("/api/schools/#{school.id}/students", headers:)
-    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email)
+    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher], email: teacher.email, school_id: school.id)
   end
 
   it 'responds with the school students JSON' do