fix(auth): prevent cross-org account takeover via Okta SSO

app/models/integrations/okta_integration.rb

@@ -4,6 +4,7 @@ module Integrations
   class OktaIntegration < BaseIntegration
     validates :client_secret, :client_id, :domain, :organization_name, presence: true
     validate :domain_uniqueness
+    validate :host_format
 
     settings_accessors :client_id, :domain, :organization_name, :host
     secrets_accessors :client_secret
@@ -25,6 +26,14 @@ def domain_uniqueness
 
       errors.add(:domain, "domain_not_unique") if okta_integration
     end
+
+    def host_format
+      configured_host = get_from_settings("host")
+      return if configured_host.blank?
+
+      url_segment = /\A[a-zA-Z0-9.-]+\z/
+      errors.add(:host, "host_invalid") unless configured_host.match?(url_segment)
+    end
   end
 end
 

app/services/auth/okta/base_service.rb

@@ -60,7 +60,7 @@ def check_userinfo(email)
         userinfo_headers = {"Authorization" => "Bearer #{result.okta_access_token}"}
         response = userinfo_client.get(headers: userinfo_headers)
 
-        raise ValidationError, "okta_userinfo_error" if response["email"] != email
+        raise ValidationError, "okta_userinfo_error" unless response["email"]&.casecmp?(email)
 
         result.userinfo = response
       end

app/services/auth/okta/login_service.rb

@@ -20,6 +20,10 @@ def call
         query_okta_access_token
         check_userinfo(result.email)
 
+        if existing_user_outside_organization?
+          return result.single_validation_failure!(error_code: "user_does_not_belong_to_organization")
+        end
+
         find_or_create_user
         find_or_create_membership
 
@@ -41,6 +45,13 @@ def call
 
       attr_reader :code, :state
 
+      def existing_user_outside_organization?
+        user = User.find_by(email: result.email)
+        return false if user.nil?
+
+        !user.memberships.active.exists?(organization_id: result.okta_integration.organization_id)
+      end
+
       def generate_token
         result.token = Utils::AuthToken.encode(user: result.user, login_method: Organizations::AuthenticationMethods::OKTA)
         result

spec/models/integrations/okta_integration_spec.rb

@@ -46,5 +46,22 @@
         expect(okta_integration.errors).to include(:domain)
       end
     end
+
+    context "when host contains unsafe URL characters" do
+      before { subject.host = "evil.com/path" }
+
+      it "is invalid" do
+        expect(subject).not_to be_valid
+        expect(subject.errors).to include(:host)
+      end
+    end
+
+    context "when host is a custom domain" do
+      before { subject.host = "login.acme.com" }
+
+      it "is valid" do
+        expect(subject).to be_valid
+      end
+    end
   end
 end

spec/services/auth/okta/login_service_spec.rb

@@ -104,21 +104,58 @@
       end
     end
 
-    context "when user already exists" do
+    context "when okta userinfo email only differs in casing" do
+      let(:okta_userinfo_response) { {"email" => "FOO@BAR.COM"} }
+
+      it "authenticates the user" do
+        result = service.call
+
+        expect(result).to be_success
+        expect(result.token).to be_present
+      end
+    end
+
+    context "when the user exists but does not belong to the organization" do
+      before { create(:user, email: "foo@bar.com") }
+
+      it "does not authenticate the user" do
+        result = service.call
+
+        expect(result).not_to be_success
+        expect(result.error.messages.values.flatten).to include("user_does_not_belong_to_organization")
+        expect(result.token).to be_nil
+      end
+
+      it "does not create a membership" do
+        expect { service.call }.not_to change(Membership, :count)
+      end
+    end
+
+    context "when the user exists with a revoked membership in the organization" do
       let(:user) { create(:user, email: "foo@bar.com") }
 
-      before { user }
+      before { create(:membership, :revoked, user:, organization: okta_integration.organization) }
+
+      it "does not authenticate the user" do
+        result = service.call
 
-      it "does not create a new user" do
-        expect { service.call }.not_to change(User, :count)
+        expect(result).not_to be_success
+        expect(result.error.messages.values.flatten).to include("user_does_not_belong_to_organization")
       end
     end
 
-    context "when membership already exists" do
+    context "when the user already belongs to the organization" do
       let(:user) { create(:user, email: "foo@bar.com") }
-      let(:membership) { create(:membership, user:, organization: okta_integration.organization) }
 
-      before { membership }
+      before { create(:membership, user:, organization: okta_integration.organization) }
+
+      it "authenticates without creating a new user" do
+        result = nil
+
+        expect { result = service.call }.not_to change(User, :count)
+        expect(result).to be_success
+        expect(result.token).to be_present
+      end
 
       it "does not create a new membership" do
         expect { service.call }.not_to change(Membership, :count)