do not pass user into authorization callback

pull/1636/head
Markus Kahl 10 years ago
parent 3303126657
commit fd1f8315cf
  1. 3
      app/controllers/concerns/omniauth_login.rb
  2. 21
      lib/open_project/omni_auth/authorization.rb
  3. 10
      spec/controllers/concerns/omniauth_login_spec.rb

@ -10,8 +10,7 @@ module Concerns::OmniauthLogin
params[:back_url] = request.env['omniauth.origin']
user = User.find_or_initialize_by_identity_url identity_url_from_omniauth(auth_hash)
dec = OpenProject::OmniAuth::Authorization.authorized? user_with_info(user, auth_hash),
auth_hash
dec = OpenProject::OmniAuth::Authorization.authorized? auth_hash
if dec.approve?
authorization_successful user, auth_hash
else

@ -6,11 +6,11 @@ module OpenProject
##
# Checks whether the given user is authorized to login by calling
# all registered callbacks. If all callbacks approve the user is authorized and may log in.
def self.authorized?(user, auth_hash)
def self.authorized?(auth_hash)
rejection = nil
callbacks.each do |callback|
d = callback.authorize user, auth_hash
d = callback.authorize auth_hash
if d.is_a? Decision
if d.reject?
@ -51,9 +51,9 @@ module OpenProject
end
def self.authorize_user_for_provider(provider, &block)
callback = BlockCallback.new do |dec, user, auth_hash|
callback = BlockCallback.new do |dec, auth_hash|
if auth_hash.provider.to_sym == provider.to_sym
block.call dec, user, auth_hash
block.call dec, auth_hash
else
Decision.approve
end
@ -74,15 +74,14 @@ module OpenProject
# Performs user authorization.
class Callback
##
# Given a user and an OmniAuth auth hash this decides if a user is authorized or not.
# Given an OmniAuth auth hash this decides if a user is authorized or not.
#
# @param [User] user The OpenProject user to be logged in.
# @param [AuthHash] OmniAuth authentication information including user info
# @param [AuthHash] auth_hash OmniAuth authentication information including user info
# and credentials.
#
# @return [Decision] A decision indicating whether the user is authorized or not.
def authorize(user, auth_hash)
fail "subclass responsibility: authorize(#{user}, #{auth_hash})"
def authorize(auth_hash)
fail "subclass responsibility: authorize(#{auth_hash})"
end
end
@ -95,8 +94,8 @@ module OpenProject
@block = block
end
def authorize(user, auth_hash)
block.call Decision, user, auth_hash
def authorize(auth_hash)
block.call Decision, auth_hash
end
end

@ -226,7 +226,7 @@ describe AccountController do
# Let's set up a couple of authorization callbacks to see if the mechanism
# works as intended.
OpenProject::OmniAuth::Authorization.authorize_user provider: :google do |dec, _, auth|
OpenProject::OmniAuth::Authorization.authorize_user provider: :google do |dec, auth|
if auth.info.name == config.google_name
dec.approve
else
@ -234,8 +234,8 @@ describe AccountController do
end
end
OpenProject::OmniAuth::Authorization.authorize_user do |dec, user, _|
if user.mail == config.global_email
OpenProject::OmniAuth::Authorization.authorize_user do |dec, auth|
if auth.info.email == config.global_email
dec.approve
else
dec.reject "I only want to see #{config[:global_email]} here."
@ -243,12 +243,12 @@ describe AccountController do
end
# ineffective callback
OpenProject::OmniAuth::Authorization.authorize_user provider: :foobar do |dec, _, _|
OpenProject::OmniAuth::Authorization.authorize_user provider: :foobar do |dec, _|
dec.reject 'Though shalt not pass!'
end
# free for all callback
OpenProject::OmniAuth::Authorization.authorize_user do |dec, _, _|
OpenProject::OmniAuth::Authorization.authorize_user do |dec, _|
dec.approve
end
end

Loading…
Cancel
Save