Reject unassociated teacher accounts in Join Code flow

app/models/user.rb

@@ -21,6 +21,7 @@ class User
     username
     roles
     sso_providers
+    sub
   ].freeze
 
   attr_accessor(*ATTRIBUTES)
@@ -53,6 +54,10 @@ def student?
     Role.student.exists?(user_id: id)
   end
 
+  def student_account_type?
+    sub.to_s.starts_with?('student:')
+  end
+
   def admin?
     parsed_roles.include?('editor-admin')
   end

app/services/join_status_service.rb

@@ -38,7 +38,7 @@ def existing_user_join_status
 
   # The user has no role in this school yet: may they join as a new student?
   def new_user_join_status
-    return :not_a_student if user_has_non_student_role?
+    return :not_a_student if user_has_non_student_account_type?
     return :wrong_school if user_in_different_school?
     return :domain_mismatch unless @school.email_domain_in_school_domains?(@user.email)
 
@@ -62,8 +62,8 @@ def user_has_role_in_school?
     Role.exists?(school: @school, user_id: @user.id)
   end
 
-  def user_has_non_student_role?
-    Role.where(user_id: @user.id).where.not(role: Role.roles[:student]).exists?
+  def user_has_non_student_account_type?
+    !@user.student_account_type?
   end
 
   def user_in_different_school?

spec/factories/user.rb

@@ -7,6 +7,7 @@
     email { Faker::Internet.email }
     username { nil }
     sso_providers { [] }
+    sub { id }
 
     factory :admin_user do
       roles { 'editor-admin' }
@@ -20,6 +21,7 @@
       email { nil }
       username { Faker::Internet.username }
       sso_providers { [] } # standard students have no SSO providers
+      sub { "student:#{id}" }
 
       trait :sso do
         email { Faker::Internet.email }

spec/models/user_spec.rb

@@ -74,6 +74,10 @@
         expect(user.username).to be_nil
       end
 
+      it 'returns a user with the correct sub' do
+        expect(user.sub).to eq owner.id
+      end
+
       context 'when BYPASS_OAUTH is true' do
         around do |example|
           ClimateControl.modify(BYPASS_OAUTH: 'true') do
@@ -118,6 +122,10 @@
       it 'returns a user without an email' do
         expect(user.email).to be_nil
       end
+
+      it 'returns a user with the correct sub' do
+        expect(user.sub).to eq "student:#{student.id}"
+      end
     end
 
     it 'returns nil when the access token is invalid' do
@@ -266,6 +274,20 @@
     end
   end
 
+  describe '#student_account_type?' do
+    it 'returns true for a student account' do
+      expect(build(:student)).to be_student_account_type
+    end
+
+    it 'returns false for a non-student account' do
+      expect(build(:user)).not_to be_student_account_type
+    end
+
+    it 'returns false for a teacher account' do
+      expect(build(:teacher)).not_to be_student_account_type
+    end
+  end
+
   describe '#parsed_roles' do
     it 'returns array of role names when roles is set to comma-separated string' do
       user = build(:user, roles: 'role-1,role-2')

spec/requests/join_controller_spec.rb

@@ -5,7 +5,9 @@
 RSpec.describe 'Join endpoint' do
   let(:school) { create(:school, code: '12-34-56') }
   let(:school_class) { create(:school_class, school:, join_code: 'B123-C456') }
-  let(:student) { create(:user, email: 'student@example.edu') }
+  let(:student) { build(:student, email: 'student@example.edu') }
+  let(:teacher) { build(:teacher, email: 'teacher@example.edu') }
+  let(:owner) { build(:owner, email: 'owner@example.edu') }
   let(:headers) { { Authorization: UserProfileMock::TOKEN } }
 
   before do
@@ -40,8 +42,8 @@
       end
     end
 
-    context 'when the user is authenticated' do
-      before { authenticated_in_hydra_as(student) }
+    context 'when the user is authenticated as a student' do
+      before { authenticated_in_hydra_as(student, :student) }
 
       it 'returns status: joinable when the user can join' do
         get "/api/join/#{school_class.join_code}", headers: headers
@@ -70,62 +72,70 @@
         expect(data[:status]).to eq('wrong_school')
       end
 
-      it 'returns status: joinable_as_teacher when the user is a teacher of this school not yet in the class' do
-        create(:teacher_role, school:, user_id: student.id)
+      context 'when the email domain is not registered for the school' do
+        let(:student) { build(:student, email: 'student@other.edu') }
 
-        get "/api/join/#{school_class.join_code}", headers: headers
+        it 'returns status: domain_mismatch' do
+          get "/api/join/#{school_class.join_code}", headers: headers
 
-        data = JSON.parse(response.body, symbolize_names: true)
-        expect(data[:status]).to eq('joinable_as_teacher')
+          data = JSON.parse(response.body, symbolize_names: true)
+          expect(data[:status]).to eq('domain_mismatch')
+        end
+
+        it 'returns status: joinable when the user is already a student of the school' do
+          create(:student_role, school:, user_id: student.id)
+
+          get "/api/join/#{school_class.join_code}", headers: headers
+
+          data = JSON.parse(response.body, symbolize_names: true)
+          expect(data[:status]).to eq('joinable')
+        end
       end
+    end
 
-      it 'returns status: already_member when the user is already a teacher in the class' do
-        create(:teacher_role, school:, user_id: student.id)
-        ClassTeacher.create!(school_class:, teacher_id: student.id)
+    context 'when the user is authenticated as a teacher' do
+      before { authenticated_in_hydra_as(teacher) }
+
+      it 'returns status: joinable_as_teacher when the user is a teacher of this school not yet in the class' do
+        create(:teacher_role, school:, user_id: teacher.id)
 
         get "/api/join/#{school_class.join_code}", headers: headers
 
         data = JSON.parse(response.body, symbolize_names: true)
-        expect(data[:status]).to eq('already_member')
+        expect(data[:status]).to eq('joinable_as_teacher')
       end
 
-      it 'returns status: owner when the user is an owner of this school' do
-        create(:owner_role, school:, user_id: student.id)
+      it 'returns status: already_member when the user is already a teacher in the class' do
+        create(:teacher_role, school:, user_id: teacher.id)
+        ClassTeacher.create!(school_class:, teacher_id: teacher.id)
 
         get "/api/join/#{school_class.join_code}", headers: headers
 
         data = JSON.parse(response.body, symbolize_names: true)
-        expect(data[:status]).to eq('owner')
+        expect(data[:status]).to eq('already_member')
       end
 
       it 'returns status: not_a_student for a teacher of a different school (not wrong_school)' do
         other_school = create(:school)
-        create(:teacher_role, school: other_school, user_id: student.id)
+        create(:teacher_role, school: other_school, user_id: teacher.id)
 
         get "/api/join/#{school_class.join_code}", headers: headers
 
         data = JSON.parse(response.body, symbolize_names: true)
         expect(data[:status]).to eq('not_a_student')
       end
+    end
 
-      context 'when the email domain is not registered for the school' do
-        let(:student) { create(:user, email: 'student@other.edu') }
-
-        it 'returns status: domain_mismatch' do
-          get "/api/join/#{school_class.join_code}", headers: headers
-
-          data = JSON.parse(response.body, symbolize_names: true)
-          expect(data[:status]).to eq('domain_mismatch')
-        end
+    context 'when the user is authenticated as an owner' do
+      before { authenticated_in_hydra_as(owner) }
 
-        it 'returns status: joinable when the user is already a student of the school' do
-          create(:student_role, school:, user_id: student.id)
+      it 'returns status: owner when the user is an owner of this school' do
+        create(:owner_role, school:, user_id: owner.id)
 
-          get "/api/join/#{school_class.join_code}", headers: headers
+        get "/api/join/#{school_class.join_code}", headers: headers
 
-          data = JSON.parse(response.body, symbolize_names: true)
-          expect(data[:status]).to eq('joinable')
-        end
+        data = JSON.parse(response.body, symbolize_names: true)
+        expect(data[:status]).to eq('owner')
       end
     end
   end
@@ -138,8 +148,8 @@
       end
     end
 
-    context 'when the user is authenticated' do
-      before { authenticated_in_hydra_as(student) }
+    context 'when the user is authenticated as a student' do
+      before { authenticated_in_hydra_as(student, :student) }
 
       it 'adds the user to the school and class and returns a redirect URL' do
         expect do
@@ -188,8 +198,24 @@
         expect(data[:error]).to eq('wrong_school')
       end
 
+      it 'responds with 404 when the join code does not exist' do
+        post '/api/join/INVALID123', headers: headers
+        expect(response).to have_http_status(:not_found)
+      end
+
+      # rubocop:disable RSpec/AnyInstance
+      it 'responds with 500 when action_status returns an unexpected value' do
+        allow_any_instance_of(Api::JoinController).to receive(:action_status).and_return(:something_unexpected)
+
+        post "/api/join/#{school_class.join_code}", headers: headers
+
+        expect(response).to have_http_status(:internal_server_error)
+        expect(response.body).to include('Unexpected join action_status')
+      end
+      # rubocop:enable RSpec/AnyInstance
+
       context 'when the email domain is not registered for the school' do
-        let(:student) { create(:user, email: 'student@other.edu') }
+        let(:student) { build(:student, email: 'student@other.edu') }
 
         it 'responds with 403 domain_mismatch and does not enroll the user' do
           expect do
@@ -213,9 +239,13 @@
           expect(data[:redirect_url]).to eq("/school/#{school.code}/class/#{school_class.code}")
         end
       end
+    end
+
+    context 'when the user is authenticated as a teacher' do
+      before { authenticated_in_hydra_as(teacher) }
 
       it 'adds the user to the class as a teacher and returns a redirect URL' do
-        create(:teacher_role, school:, user_id: student.id)
+        create(:teacher_role, school:, user_id: teacher.id)
         school_class # force creation before the request
 
         expect do
@@ -225,14 +255,14 @@
         expect(response).to have_http_status(:ok)
         data = JSON.parse(response.body, symbolize_names: true)
         expect(data[:redirect_url]).to eq("/school/#{school.code}/class/#{school_class.code}")
-        expect(ClassTeacher.exists?(school_class:, teacher_id: student.id)).to be(true)
-        expect(ClassStudent.exists?(school_class:, student_id: student.id)).to be(false)
-        expect(Role.where(user_id: student.id, school:).pluck(:role)).to eq(['teacher'])
+        expect(ClassTeacher.exists?(school_class:, teacher_id: teacher.id)).to be(true)
+        expect(ClassStudent.exists?(school_class:, student_id: teacher.id)).to be(false)
+        expect(Role.where(user_id: teacher.id, school:).pluck(:role)).to eq(['teacher'])
       end
 
       it 'is idempotent when the user is already a teacher in the class' do
-        create(:teacher_role, school:, user_id: student.id)
-        ClassTeacher.create!(school_class:, teacher_id: student.id)
+        create(:teacher_role, school:, user_id: teacher.id)
+        ClassTeacher.create!(school_class:, teacher_id: teacher.id)
 
         expect do
           post "/api/join/#{school_class.join_code}", headers: headers
@@ -242,9 +272,13 @@
         data = JSON.parse(response.body, symbolize_names: true)
         expect(data[:redirect_url]).to eq("/school/#{school.code}/class/#{school_class.code}")
       end
+    end
+
+    context 'when the user is authenticated as an owner' do
+      before { authenticated_in_hydra_as(owner) }
 
       it 'redirects an owner into the class without adding them to it' do
-        create(:owner_role, school:, user_id: student.id)
+        create(:owner_role, school:, user_id: owner.id)
 
         expect do
           post "/api/join/#{school_class.join_code}", headers: headers
@@ -253,25 +287,9 @@
         expect(response).to have_http_status(:ok)
         data = JSON.parse(response.body, symbolize_names: true)
         expect(data[:redirect_url]).to eq("/school/#{school.code}/class/#{school_class.code}")
-        expect(ClassTeacher.exists?(school_class:, teacher_id: student.id)).to be(false)
-        expect(Role.where(user_id: student.id, school:).pluck(:role)).to eq(['owner'])
+        expect(ClassTeacher.exists?(school_class:, teacher_id: owner.id)).to be(false)
+        expect(Role.where(user_id: owner.id, school:).pluck(:role)).to eq(['owner'])
       end
-
-      it 'responds with 404 when the join code does not exist' do
-        post '/api/join/INVALID123', headers: headers
-        expect(response).to have_http_status(:not_found)
-      end
-
-      # rubocop:disable RSpec/AnyInstance
-      it 'responds with 500 when action_status returns an unexpected value' do
-        allow_any_instance_of(Api::JoinController).to receive(:action_status).and_return(:something_unexpected)
-
-        post "/api/join/#{school_class.join_code}", headers: headers
-
-        expect(response).to have_http_status(:internal_server_error)
-        expect(response.body).to include('Unexpected join action_status')
-      end
-      # rubocop:enable RSpec/AnyInstance
     end
   end
 end