diff --git a/Gemfile.lock b/Gemfile.lock index 98d163e..c040098 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -101,7 +101,7 @@ GEM bootstrap-sass (3.4.1) autoprefixer-rails (>= 5.2.1) sassc (>= 2.0.0) - brakeman (8.0.2) + brakeman (8.0.4) racc builder (3.3.0) capybara (3.40.0) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 7410138..2611473 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,23 +1,16 @@ class ApplicationController < ActionController::Base # Only allow modern browsers supporting webp images, web push, badges, import maps, CSS nesting, and CSS :has. allow_browser versions: :modern - before_action :check_session_expiry - before_action :check_user_token + before_action :authenticate! private - def check_user_token - unless session[:user_token] - render "puzzles/login" - end - end - - def check_session_expiry + def authenticate! if session[:expires_at].present? && Time.current > session[:expires_at] reset_session - render "puzzles/login" - else - session[:expires_at] = 1.hour.from_now + render "puzzles/login" and return end + session[:expires_at] = 1.hour.from_now + render "puzzles/login" unless session[:user_token] end end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 6b15682..d3626b3 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,5 +1,5 @@ class SessionsController < ApplicationController - skip_before_action :check_user_token + skip_before_action :authenticate! def create auth = request.env["omniauth.auth"] diff --git a/app/controllers/slack/application_controller.rb b/app/controllers/slack/application_controller.rb index e587022..7a88a07 100644 --- a/app/controllers/slack/application_controller.rb +++ b/app/controllers/slack/application_controller.rb @@ -1,13 +1,13 @@ class Slack::ApplicationController < ApplicationController skip_before_action :verify_authenticity_token - skip_before_action :check_user_token + skip_before_action :authenticate! - before_action :valid_slack_request? + before_action :verify_slack_request! private - def valid_slack_request? - @verified ||= verify_slack_signature + def verify_slack_request! + head :unauthorized unless verify_slack_signature end def verify_slack_signature @@ -43,7 +43,8 @@ def open_view(view, trigger_id:) def send_message(message, channel_id:) SlackClient::Client.instance.chat_postMessage(channel: channel_id, blocks: message) - rescue Slack::Web::Api::Errors::SlackError - head :unprocessable_entity + rescue Slack::Web::Api::Errors::SlackError => e + Rails.logger.error "Failed to send Slack message: #{e.message}" + Sentry.capture_exception(e) end end diff --git a/test/controllers/puzzles_controller_test.rb b/test/controllers/puzzles_controller_test.rb index 6f64d3a..8ff1b84 100644 --- a/test/controllers/puzzles_controller_test.rb +++ b/test/controllers/puzzles_controller_test.rb @@ -7,6 +7,22 @@ class PuzzlesControllerTest < ActionDispatch::IntegrationTest assert_response :success end + test "unauthenticated request renders login" do + get puzzles_path + assert_response :success + assert_dom "p", "Log in to access the Ruby or Rails admin panel." + end + + test "expired session renders login and requires re-authentication" do + sign_in + get puzzles_path # authenticate! sets session[:expires_at] + travel_to 2.hours.from_now do + get puzzles_path + assert_response :success + assert_dom "p", "Log in to access the Ruby or Rails admin panel." + end + end + test "should show error message when editing puzzle with invalid data" do puzzle = puzzles(:one) diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index e5da4b3..12da3f4 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -1,7 +1,12 @@ require "test_helper" class SessionsControllerTest < ActionDispatch::IntegrationTest - # test "the truth" do - # assert true - # end + test "completes OAuth even when session has expired" do + sign_in + + travel_to 2.hours.from_now do + sign_in + assert_redirected_to root_path + end + end end diff --git a/test/controllers/slack/puzzles_controller_test.rb b/test/controllers/slack/puzzles_controller_test.rb new file mode 100644 index 0000000..fd18875 --- /dev/null +++ b/test/controllers/slack/puzzles_controller_test.rb @@ -0,0 +1,68 @@ +require "test_helper" + +class Slack::PuzzlesControllerTest < ActionDispatch::IntegrationTest + setup do + ENV["SLACK_SIGNING_SECRET"] = "test_signing_secret" + end + + test "rejects request with invalid Slack signature" do + post slack_puzzle_path, params: { payload: puzzle_payload }, + headers: slack_headers(secret: "wrong_secret") + + assert_response :unauthorized + end + + test "rejects request with expired Slack timestamp" do + post slack_puzzle_path, params: { payload: puzzle_payload }, + headers: slack_headers(timestamp: Time.now.to_i - 400) + + assert_response :unauthorized + end + + test "renders ok when puzzle fails validation" do + params = { payload: puzzle_payload(question: "") } + + post slack_puzzle_path, params: params, + headers: slack_headers(body: params.to_query) + + assert_response :ok + end + + test "renders ok even when Slack notification fails after puzzle is saved" do + original = Slack::ApplicationController.instance_method(:send_message) + Slack::ApplicationController.define_method(:send_message) { |*| nil } + + params = { payload: puzzle_payload(question: "What is unique about Ruby's blocks?") } + + post slack_puzzle_path, params: params, + headers: slack_headers(body: params.to_query) + + assert_response :ok + ensure + Slack::ApplicationController.define_method(:send_message, original) + end + + private + + def puzzle_payload(question: "What is Ruby?") + { + user: { id: "U123" }, + view: { + state: { + values: { + question: { question: { value: question } }, + answer: { answer: { selected_option: { value: "ruby" } } }, + explanation: { explanation: { value: "It is a programming language." } }, + link: { link: { value: nil } } + } + } + } + }.to_json + end + + def slack_headers(secret: ENV["SLACK_SIGNING_SECRET"], timestamp: Time.now.to_i, body: "") + ts = timestamp.to_s + sig = "v0=" + OpenSSL::HMAC.hexdigest("SHA256", secret, "v0:#{ts}:#{body}") + { "X-Slack-Request-Timestamp" => ts, "X-Slack-Signature" => sig } + end +end