Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions app/controllers/api/school_members_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
6 changes: 4 additions & 2 deletions app/controllers/api/school_students_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
5 changes: 5 additions & 0 deletions app/services/school_onboarding_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}" }
Expand Down
27 changes: 23 additions & 4 deletions lib/profile_api_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -27,6 +27,8 @@ def initialize(errors)
end
end

class UnauthorizedError < Error; end

class UnexpectedResponse < Error
attr_reader :response_status, :response_headers, :response_body

Expand All @@ -50,6 +52,7 @@ def create_school(token:, id:, code:)
}
end

unauthorized!(response)
raise UnexpectedResponse, response unless response.status == 201

School.new(**response.body)
Expand All @@ -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)
Expand All @@ -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) }
Expand All @@ -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
Expand All @@ -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']
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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

Expand All @@ -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}",
Expand Down Expand Up @@ -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
6 changes: 3 additions & 3 deletions spec/features/school/creating_a_school_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
8 changes: 4 additions & 4 deletions spec/features/school_member/listing_school_members_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions spec/features/school_student/listing_school_students_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading