From 55d0011bfc4c3d24f7ae89f8b630883953618b29 Mon Sep 17 00:00:00 2001 From: Arthur Chui Date: Wed, 26 Nov 2025 10:25:17 -0800 Subject: [PATCH] Upgrade omniauth-github to 2.1.4 --- Gemfile.lock | 21 ++++++++++++------- .../concerns/shipit/authentication.rb | 4 ++-- .../github_authentication_controller.rb | 2 ++ .../github_authentication/login.html.erb | 14 +++++++++++++ config/routes.rb | 3 ++- lib/shipit/engine.rb | 1 + shipit-engine.gemspec | 2 +- .../api_clients_controller_test.rb | 2 +- .../github_authentication_controller_test.rb | 6 ++++++ .../repositories_controller_test.rb | 2 +- test/controllers/stacks_controller_test.rb | 4 ++-- 11 files changed, 45 insertions(+), 16 deletions(-) create mode 100644 app/views/shipit/github_authentication/login.html.erb diff --git a/Gemfile.lock b/Gemfile.lock index 1217d3125..28e63becf 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -13,7 +13,7 @@ PATH jquery-rails (~> 4.4) lodash-rails (~> 4.17) octokit (~> 5.6.0) - omniauth-github (~> 1.4) + omniauth-github (~> 2.0) paquito pubsubstub (~> 0.2) rails (~> 8.0.1) @@ -246,15 +246,17 @@ GEM octokit (5.6.1) faraday (>= 1, < 3) sawyer (~> 0.9) - omniauth (1.9.2) + omniauth (2.1.4) hashie (>= 3.4.6) - rack (>= 1.6.2, < 3) - omniauth-github (1.4.0) - omniauth (~> 1.5) - omniauth-oauth2 (>= 1.4.0, < 2.0) - omniauth-oauth2 (1.7.3) + logger + rack (>= 2.2.3) + rack-protection + omniauth-github (2.0.1) + omniauth (~> 2.0) + omniauth-oauth2 (~> 1.8) + omniauth-oauth2 (1.8.0) oauth2 (>= 1.4, < 3) - omniauth (>= 1.9, < 3) + omniauth (~> 2.0) ostruct (0.6.2) paquito (0.10.0) msgpack (>= 1.5.2) @@ -276,6 +278,9 @@ GEM redis (~> 4.0) racc (1.8.1) rack (2.2.17) + rack-protection (3.2.0) + base64 (>= 0.1.0) + rack (~> 2.2, >= 2.2.4) rack-session (1.0.2) rack (< 3) rack-test (2.2.0) diff --git a/app/controllers/concerns/shipit/authentication.rb b/app/controllers/concerns/shipit/authentication.rb index e3bdf3469..747d7d610 100644 --- a/app/controllers/concerns/shipit/authentication.rb +++ b/app/controllers/concerns/shipit/authentication.rb @@ -21,7 +21,7 @@ def force_github_authentication if current_user.logged_in? && current_user.requires_fresh_login? Rails.logger.warn("User #{current_user.id} requires a fresh login, logging out...") reset_session - redirect_to(Shipit::Engine.routes.url_helpers.github_authentication_path(origin: request.original_url)) + redirect_to(Shipit::Engine.routes.url_helpers.github_authentication_login_path(origin: request.original_url)) elsif Shipit.authentication_disabled? || current_user.logged_in? unless current_user.authorized? team_handles = Shipit.github_teams.map(&:handle) @@ -29,7 +29,7 @@ def force_github_authentication render(plain: "You must be a member of #{team_list} to access this application.", status: :forbidden) end else - redirect_to(Shipit::Engine.routes.url_helpers.github_authentication_path(origin: request.original_url)) + redirect_to(Shipit::Engine.routes.url_helpers.github_authentication_login_path(origin: request.original_url)) end end diff --git a/app/controllers/shipit/github_authentication_controller.rb b/app/controllers/shipit/github_authentication_controller.rb index af2f9c2e6..0f2be9487 100644 --- a/app/controllers/shipit/github_authentication_controller.rb +++ b/app/controllers/shipit/github_authentication_controller.rb @@ -4,6 +4,8 @@ module Shipit class GithubAuthenticationController < ActionController::Base include Shipit::Engine.routes.url_helpers + layout 'shipit', only: 'login' + def callback return_url = request.env['omniauth.origin'] || root_path auth = request.env['omniauth.auth'] diff --git a/app/views/shipit/github_authentication/login.html.erb b/app/views/shipit/github_authentication/login.html.erb new file mode 100644 index 000000000..20d819ec9 --- /dev/null +++ b/app/views/shipit/github_authentication/login.html.erb @@ -0,0 +1,14 @@ +<% content_for :page_title do %> +

Login

+<% end %> + +
+
+ <%= form_with url: github_authentication_path, method: :post do |form| %> + <%= form.hidden_field :origin, value: params[:origin] %> +
+ <%= form.submit "Login with GitHub", class: 'btn primary' %> +
+ <% end %> +
+
diff --git a/config/routes.rb b/config/routes.rb index 97a3d6b82..0d5b25f73 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -68,7 +68,8 @@ end scope '/github/auth/github', as: :github_authentication, controller: :github_authentication do - get '/', action: :request + get :login + post '/', action: :request post :callback get :callback get :logout diff --git a/lib/shipit/engine.rb b/lib/shipit/engine.rb index 990a4bc2e..6a919cc6a 100644 --- a/lib/shipit/engine.rb +++ b/lib/shipit/engine.rb @@ -45,6 +45,7 @@ class Engine < ::Rails::Engine ActiveModel::Serializer.include(Engine.routes.url_helpers) if Shipit.github.oauth? + OmniAuth::AuthenticityTokenProtection.default_options(key: "_csrf_token", authenticity_param: "authenticity_token") OmniAuth::Strategies::GitHub.configure(path_prefix: '/github/auth') app.middleware.use(OmniAuth::Builder) do provider(:github, *Shipit.github.oauth_config) diff --git a/shipit-engine.gemspec b/shipit-engine.gemspec index dab8a8883..98b9c7d38 100644 --- a/shipit-engine.gemspec +++ b/shipit-engine.gemspec @@ -31,7 +31,7 @@ Gem::Specification.new do |s| s.add_dependency('jquery-rails', '~> 4.4') s.add_dependency('lodash-rails', '~> 4.17') s.add_dependency('octokit', '~> 5.6.0') - s.add_dependency('omniauth-github', '~> 1.4') + s.add_dependency('omniauth-github', '~> 2.0') s.add_dependency('paquito') s.add_dependency('pubsubstub', '~> 0.2') s.add_dependency('rails', '~> 8.0.1') diff --git a/test/controllers/api_clients_controller_test.rb b/test/controllers/api_clients_controller_test.rb index 3ad42e43f..7708d6a75 100644 --- a/test/controllers/api_clients_controller_test.rb +++ b/test/controllers/api_clients_controller_test.rb @@ -13,7 +13,7 @@ class ApiClientsControllerTest < ActionController::TestCase test "GitHub authentication is mandatory" do session[:user_id] = nil get :index - assert_redirected_to '/github/auth/github?origin=http%3A%2F%2Ftest.host%2Fapi_clients' + assert_redirected_to '/github/auth/github/login?origin=http%3A%2F%2Ftest.host%2Fapi_clients' end test "current_user must be a member of at least a Shipit.github_teams" do diff --git a/test/controllers/github_authentication_controller_test.rb b/test/controllers/github_authentication_controller_test.rb index 4b3ff3271..2ea396e16 100644 --- a/test/controllers/github_authentication_controller_test.rb +++ b/test/controllers/github_authentication_controller_test.rb @@ -4,6 +4,12 @@ module Shipit class GithubAuthenticationControllerTest < ActionController::TestCase + test ":login can render a page to start the OAuth Flow" do + get :login + + assert_response :ok + end + test ":callback can sign in to github" do auth = OmniAuth::AuthHash.new( credentials: OmniAuth::AuthHash.new( diff --git a/test/controllers/repositories_controller_test.rb b/test/controllers/repositories_controller_test.rb index b93dca8ed..0574e4afe 100644 --- a/test/controllers/repositories_controller_test.rb +++ b/test/controllers/repositories_controller_test.rb @@ -13,7 +13,7 @@ class RepositoriesControllerTest < ActionController::TestCase test "GitHub authentication is mandatory" do session[:user_id] = nil get :index - assert_redirected_to '/github/auth/github?origin=http%3A%2F%2Ftest.host%2Frepositories' + assert_redirected_to '/github/auth/github/login?origin=http%3A%2F%2Ftest.host%2Frepositories' end test "current_user must be a member of at least a Shipit.github_teams" do diff --git a/test/controllers/stacks_controller_test.rb b/test/controllers/stacks_controller_test.rb index 699144762..eb54c2bce 100644 --- a/test/controllers/stacks_controller_test.rb +++ b/test/controllers/stacks_controller_test.rb @@ -34,7 +34,7 @@ class StacksControllerTest < ActionController::TestCase test "GitHub authentication is mandatory" do session[:user_id] = nil get :index - assert_redirected_to '/github/auth/github?origin=http%3A%2F%2Ftest.host%2F' + assert_redirected_to '/github/auth/github/login?origin=http%3A%2F%2Ftest.host%2F' end test "users which require a fresh login are redirected" do @@ -44,7 +44,7 @@ class StacksControllerTest < ActionController::TestCase get :index - assert_redirected_to '/github/auth/github?origin=http%3A%2F%2Ftest.host%2F' + assert_redirected_to '/github/auth/github/login?origin=http%3A%2F%2Ftest.host%2F' assert_nil session[:user_id] end