Skip to content

Commit

Permalink
Enhance plugin compatibility with Redmine 5.1.2
Browse files Browse the repository at this point in the history
  • Loading branch information
nanego committed Apr 15, 2024
1 parent 7e7dc07 commit c02d59e
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 40 deletions.
34 changes: 19 additions & 15 deletions lib/omniauth/dynamic_full_host.rb
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
# configures public url for our application
OmniAuth.config.full_host = Proc.new do |env|
url = env["rack.session"]["omniauth.origin"] || env["omniauth.origin"]

#unescapes url on-the-fly because it might be double-escaped in some environments
#(it happens for me at work with 2 reverse-proxies in front of the app...)
url = CGI.unescape(url) if url
module Omniauth::DynamicFullHost
def full_host_url(url = nil)
# unescapes url on-the-fly because it might be double-escaped in some environments
#(it happens for me at work with 2 reverse-proxies in front of the app...)
url = CGI.unescape(url) if url

#if no url found, fall back to config/app_config.yml addresses
if url.blank?
url = Setting["host_name"]
#else, parse it and remove both request_uri and query_string
else
uri = URI.parse(URI.encode(url)) # Encode to ensure we only have ASCII charaters in url
url = "#{uri.scheme}://#{uri.host}"
url << ":#{uri.port}" unless uri.default_port == uri.port
# if no url found, fall back to config/app_config.yml addresses
if url.blank?
url = Setting["host_name"]
# else, parse it and remove both request_uri and query_string
else
uri = URI.parse(URI.encode(url)) # Encode to ensure we only have ASCII charaters in url
url = "#{uri.scheme}://#{uri.host}"
url << ":#{uri.port}" unless uri.default_port == uri.port
end
url
end
url
end

OmniAuth.config.full_host = Proc.new do |env|
full_host_url(env["rack.session"]["omniauth.origin"] || env["omniauth.origin"])
end
20 changes: 13 additions & 7 deletions lib/omniauth/patches.rb
Original file line number Diff line number Diff line change
@@ -1,35 +1,41 @@
require 'omniauth/cas'

module Omniauth::Patches
# patch to disable return_url to avoid polluting the service URL
def return_url
{}
end
end

module OmniAuth
module Strategies
class CAS
prepend Omniauth::Patches

# patch to accept path (subdir) in cas_host
option :path, nil

# patch to accept a different host for service_validate_url
def service_validate_url_with_different_host(service_url, ticket)
service_url = Addressable::URI.parse( service_url )
service_url = Addressable::URI.parse(service_url)
service_url.query_values = service_url.query_values.tap { |qs| qs.delete('ticket') }

validate_url = Addressable::URI.parse( @options.service_validate_url )
validate_url = Addressable::URI.parse(@options.service_validate_url)

if service_url.host.nil? || validate_url.host.nil?
cas_url + append_params(@options.service_validate_url, { :service => service_url.to_s, :ticket => ticket })
else
append_params(@options.service_validate_url, { :service => service_url.to_s, :ticket => ticket })
end
end

# alias_method_chain is deprecated in Rails 5: replaced with two alias_method
# as a quick workaround. Using the 'prepend' method can generate an
# 'stack level too deep' error in conjunction with other (non ported) plugins.
#alias_method_chain :service_validate_url, :different_host
# alias_method_chain :service_validate_url, :different_host
alias_method :service_validate_url_without_different_host, :service_validate_url
alias_method :service_validate_url, :service_validate_url_with_different_host

# patch to disable return_url to avoid polluting the service URL
def return_url
{}
end
end
end
end
5 changes: 4 additions & 1 deletion lib/redmine_omniauth_cas/account_helper_patch.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
require_dependency 'account_helper'

module AccountHelper
module RedmineOmniauthCas::AccountHelperPatch
def label_for_cas_login
RedmineOmniauthCas.label_login_with_cas.presence || l(:label_login_with_cas)
end
end

AccountHelper.prepend RedmineOmniauthCas::AccountHelperPatch
ActionView::Base.prepend AccountHelper
11 changes: 6 additions & 5 deletions lib/redmine_omniauth_cas/application_controller_patch.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
require_dependency 'application_controller'

module PluginOmniauthCas
module ApplicationController
module RedmineOmniauthCas
module ApplicationControllerPatch
extend ActiveSupport::Concern

def require_login
if !User.current.logged?
Expand Down Expand Up @@ -34,8 +35,8 @@ def require_login
head(:forbidden)
end
}
format.js {head :unauthorized, 'WWW-Authenticate' => 'Basic realm="Redmine API"'}
format.any {head :unauthorized}
format.js { head :unauthorized, 'WWW-Authenticate' => 'Basic realm="Redmine API"' }
format.any { head :unauthorized }
end
return false
end
Expand Down Expand Up @@ -91,4 +92,4 @@ def validate_back_url(back_url)
end
end

ApplicationController.prepend PluginOmniauthCas::ApplicationController
ApplicationController.prepend RedmineOmniauthCas::ApplicationControllerPatch
1 change: 0 additions & 1 deletion lib/redmine_omniauth_cas/hooks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ def after_plugins_loaded(_context = {})
require_relative 'account_controller_patch'
require_relative 'account_helper_patch'
require_relative 'application_controller_patch'
require_relative 'user_patch'
end
end
end
6 changes: 0 additions & 6 deletions lib/redmine_omniauth_cas/user_patch.rb

This file was deleted.

14 changes: 9 additions & 5 deletions spec/integration/account_patch_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
{ :controller => 'account', :action => 'login_with_cas_redirect', :provider => 'blah' }
)
end
#TODO: some real test?
# TODO: some real test?
end
context "GET /auth/:provider/callback" do
it "should route things correctly" do
Expand All @@ -27,15 +27,17 @@
end

it "should authorize login if user exists with this login" do
OmniAuth.config.mock_auth[:cas] = { 'uid' => 'admin' }
OmniAuth.config.mock_auth[:cas] = OmniAuth::AuthHash.new({ 'uid' => 'admin' })
Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[:cas]
get '/auth/cas/callback'
expect(response).to redirect_to('/my/page')
get '/my/page'
expect(response.body).to match /Logged in as.*admin/im
end

it "should authorize login if user exists with this email" do
OmniAuth.config.mock_auth[:cas] = { 'uid' => '[email protected]' }
OmniAuth.config.mock_auth[:cas] = OmniAuth::AuthHash.new({ 'uid' => '[email protected]' })
Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[:cas]
get '/auth/cas/callback'
expect(response).to redirect_to('/my/page')
get '/my/page'
Expand All @@ -45,15 +47,17 @@
it "should update last_login_on field" do
user = User.find(1)
user.update_attribute(:last_login_on, Time.now - 6.hours)
OmniAuth.config.mock_auth[:cas] = { 'uid' => 'admin' }
OmniAuth.config.mock_auth[:cas] = OmniAuth::AuthHash.new({ 'uid' => 'admin' })
Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[:cas]
get '/auth/cas/callback'
expect(response).to redirect_to('/my/page')
user.reload
assert Time.now - user.last_login_on < 30.seconds
end

it "should refuse login if user doesn't exist" do
OmniAuth.config.mock_auth[:cas] = { 'uid' => 'johndoe' }
OmniAuth.config.mock_auth[:cas] = OmniAuth::AuthHash.new({ 'uid' => 'johndoe' })
Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[:cas]
get '/auth/cas/callback'
expect(response).to redirect_to('/login')
follow_redirect!
Expand Down

0 comments on commit c02d59e

Please sign in to comment.