diff --git a/Gemfile.lock b/Gemfile.lock index 59b6734..fcb1672 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -3,6 +3,7 @@ PATH specs: coplan-engine (0.4.0) commonmarker + diff-lcs diffy importmap-rails jbuilder diff --git a/engine/app/channels/coplan/plan_presence_channel.rb b/engine/app/channels/coplan/plan_presence_channel.rb index 5c140c2..e6ee4a8 100644 --- a/engine/app/channels/coplan/plan_presence_channel.rb +++ b/engine/app/channels/coplan/plan_presence_channel.rb @@ -1,9 +1,14 @@ module CoPlan class PlanPresenceChannel < ActionCable::Channel::Base def subscribed + unless current_user + reject + return + end + @plan = Plan.find_by(id: params[:plan_id]) policy = @plan && PlanPolicy.new(current_user, @plan) - unless @plan && policy.show? + unless @plan && policy&.show? reject return end @@ -29,7 +34,13 @@ def ping private def current_user - connection.current_user + @current_user ||= resolve_current_user + end + + def resolve_current_user + return connection.current_user if connection.respond_to?(:current_user) && connection.current_user + + CoPlan::Authentication.user_from_request(connection.request) end def broadcast_viewers diff --git a/engine/app/controllers/coplan/application_controller.rb b/engine/app/controllers/coplan/application_controller.rb index f9abf2a..b2419f9 100644 --- a/engine/app/controllers/coplan/application_controller.rb +++ b/engine/app/controllers/coplan/application_controller.rb @@ -38,13 +38,8 @@ def signed_in? end def authenticate_coplan_user! - callback = CoPlan.configuration.authenticate - unless callback - raise "CoPlan.configure { |c| c.authenticate = ->(request) { ... } } is required" - end - - attrs = callback.call(request) - unless attrs && attrs[:external_id].present? + @current_coplan_user = CoPlan::Authentication.user_from_request(request) + unless @current_coplan_user if agent_request? render plain: agent_redirect_instructions, content_type: "text/markdown", status: :unauthorized elsif CoPlan.configuration.sign_in_path @@ -52,26 +47,6 @@ def authenticate_coplan_user! else head :unauthorized end - return - end - - external_id = attrs[:external_id].to_s - @current_coplan_user = CoPlan::User.find_or_initialize_by(external_id: external_id) - sync_user_attrs(@current_coplan_user, attrs) - if @current_coplan_user.new_record? || @current_coplan_user.changed? - @current_coplan_user.save! - end - rescue ActiveRecord::RecordNotUnique - @current_coplan_user = CoPlan::User.find_by!(external_id: external_id) - sync_user_attrs(@current_coplan_user, attrs) - @current_coplan_user.save! if @current_coplan_user.changed? - end - - def sync_user_attrs(user, attrs) - safe_attrs = attrs.slice(:name, :username, :admin, :avatar_url, :title, :team).compact - user.assign_attributes(safe_attrs) - if attrs.key?(:metadata) && attrs[:metadata].is_a?(Hash) - user.metadata = (user.metadata || {}).merge(attrs[:metadata]) end end diff --git a/engine/app/services/coplan/authentication.rb b/engine/app/services/coplan/authentication.rb new file mode 100644 index 0000000..f55bc1f --- /dev/null +++ b/engine/app/services/coplan/authentication.rb @@ -0,0 +1,37 @@ +module CoPlan + module Authentication + MISSING_AUTHENTICATE_MESSAGE = "CoPlan.configure { |c| c.authenticate = ->(request) { ... } } is required" + + module_function + + def user_from_request(request, callback: CoPlan.configuration.authenticate) + raise MISSING_AUTHENTICATE_MESSAGE unless callback + + attrs = callback.call(request) + return nil unless attrs && attrs[:external_id].present? + + provision_user!(attrs) + end + + def provision_user!(attrs) + external_id = attrs[:external_id].to_s + user = CoPlan::User.find_or_initialize_by(external_id: external_id) + sync_user_attrs(user, attrs) + user.save! if user.new_record? || user.changed? + user + rescue ActiveRecord::RecordNotUnique + user = CoPlan::User.find_by!(external_id: external_id) + sync_user_attrs(user, attrs) + user.save! if user.changed? + user + end + + def sync_user_attrs(user, attrs) + safe_attrs = attrs.slice(:name, :username, :admin, :avatar_url, :title, :team).compact + user.assign_attributes(safe_attrs) + if attrs.key?(:metadata) && attrs[:metadata].is_a?(Hash) + user.metadata = (user.metadata || {}).merge(attrs[:metadata]) + end + end + end +end diff --git a/engine/coplan.gemspec b/engine/coplan.gemspec index 49160ae..670bbb5 100644 --- a/engine/coplan.gemspec +++ b/engine/coplan.gemspec @@ -3,7 +3,7 @@ require_relative "lib/coplan/version" Gem::Specification.new do |spec| spec.name = "coplan-engine" spec.version = CoPlan::VERSION - spec.authors = ["Block"] + spec.authors = [ "Block" ] spec.summary = "CoPlan — AI-assisted engineering design doc review" spec.description = "A Rails Engine for collaborative plan review with AI-powered feedback." spec.license = "Apache-2.0" @@ -14,6 +14,7 @@ Gem::Specification.new do |spec| spec.add_dependency "rails", ">= 8.0" spec.add_dependency "commonmarker" + spec.add_dependency "diff-lcs" spec.add_dependency "diffy" spec.add_dependency "ruby-openai" spec.add_dependency "propshaft" diff --git a/spec/channels/coplan/plan_presence_channel_spec.rb b/spec/channels/coplan/plan_presence_channel_spec.rb index cf88988..9e7da87 100644 --- a/spec/channels/coplan/plan_presence_channel_spec.rb +++ b/spec/channels/coplan/plan_presence_channel_spec.rb @@ -16,6 +16,20 @@ expect(CoPlan::PlanViewer.where(plan: plan, user: user)).to exist end + it "authenticates through the engine callback when the connection has no current_user" do + stub_connection(request: ActionDispatch::Request.empty) + allow(CoPlan.configuration).to receive(:authenticate).and_return( + ->(_request) { { external_id: "websocket-user", name: "Websocket User" } } + ) + plan = create(:plan, created_by_user: user) + + subscribe(plan_id: plan.id) + + websocket_user = CoPlan::User.find_by!(external_id: "websocket-user") + expect(subscription).to be_confirmed + expect(CoPlan::PlanViewer.where(plan: plan, user: websocket_user)).to exist + end + it "rejects when plan does not exist" do subscribe(plan_id: "nonexistent") expect(subscription).to be_rejected