use school id for safeguarding flags (#666)

## Status

- Related to
[profile#2006](RaspberryPiFoundation/profile#2006
) and
[profile#2007](RaspberryPiFoundation/profile#2007)
and
[#270](RaspberryPiFoundation/digital-platform-capabilities#270)

## What's changed

When creating or reading safeguarding flags from profile api, include
the school id. This should go live so that all new safeguarding flags
get the school id, and that profile can backfill the information. After
that we can improve the profile API to verify user actions target the
correct school.

Also separated out the 401 unauthorized handling partially to stop them
getting sent to sentry. Left a TODO for actual further API changes, so
this change does not change what the api returns for now.

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}",
@@ -229,5 +243,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/creating_a_school_spec.rb

@@ -4,12 +4,12 @@
 
 RSpec.describe 'Creating a school', type: :request do
   before do
-    authenticated_in_hydra_as(owner)
+    authenticated_in_hydra_as(user)
+    stub_profile_api_create_school
   end
 
   let(:headers) { { Authorization: UserProfileMock::TOKEN } }
-  let(:school) { create(:school) }
-  let(:owner) { create(:owner, school:) }
+  let(:user) { create(:user) }
 
   let(:params) do
     {

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