Fixes #3113 - Prefer HTTP standard header "From" over custom "X-On-Behalf-Of" for impersonation.
This commit is contained in:
parent
884b5c6455
commit
55ed75cfd7
4 changed files with 45 additions and 35 deletions
|
@ -272,7 +272,7 @@ RSpec/ExampleLength:
|
||||||
- 'spec/models/trigger_spec.rb'
|
- 'spec/models/trigger_spec.rb'
|
||||||
- 'spec/models/user_spec.rb'
|
- 'spec/models/user_spec.rb'
|
||||||
- 'spec/requests/admin/knowledge_base/public_menu_spec.rb'
|
- 'spec/requests/admin/knowledge_base/public_menu_spec.rb'
|
||||||
- 'spec/requests/api_auth_on_behalf_of_spec.rb'
|
- 'spec/requests/api_auth_from_spec.rb'
|
||||||
- 'spec/requests/api_auth_spec.rb'
|
- 'spec/requests/api_auth_spec.rb'
|
||||||
- 'spec/requests/calendar_spec.rb'
|
- 'spec/requests/calendar_spec.rb'
|
||||||
- 'spec/requests/external_credentials_spec.rb'
|
- 'spec/requests/external_credentials_spec.rb'
|
||||||
|
@ -346,7 +346,7 @@ RSpec/InstanceVariable:
|
||||||
- 'spec/lib/notification_factory/renderer_spec.rb'
|
- 'spec/lib/notification_factory/renderer_spec.rb'
|
||||||
- 'spec/models/import_job_spec.rb'
|
- 'spec/models/import_job_spec.rb'
|
||||||
- 'spec/models/scheduler_spec.rb'
|
- 'spec/models/scheduler_spec.rb'
|
||||||
- 'spec/requests/api_auth_on_behalf_of_spec.rb'
|
- 'spec/requests/api_auth_from_spec.rb'
|
||||||
- 'spec/requests/integration/monitoring_spec.rb'
|
- 'spec/requests/integration/monitoring_spec.rb'
|
||||||
- 'spec/requests/integration/sipgate_spec.rb'
|
- 'spec/requests/integration/sipgate_spec.rb'
|
||||||
- 'spec/requests/organization_spec.rb'
|
- 'spec/requests/organization_spec.rb'
|
||||||
|
@ -558,7 +558,7 @@ RSpec/MultipleExpectations:
|
||||||
- 'spec/models/trigger_spec.rb'
|
- 'spec/models/trigger_spec.rb'
|
||||||
- 'spec/models/type_lookup_spec.rb'
|
- 'spec/models/type_lookup_spec.rb'
|
||||||
- 'spec/models/user_spec.rb'
|
- 'spec/models/user_spec.rb'
|
||||||
- 'spec/requests/api_auth_on_behalf_of_spec.rb'
|
- 'spec/requests/api_auth_from_spec.rb'
|
||||||
- 'spec/requests/api_auth_spec.rb'
|
- 'spec/requests/api_auth_spec.rb'
|
||||||
- 'spec/requests/calendar_spec.rb'
|
- 'spec/requests/calendar_spec.rb'
|
||||||
- 'spec/requests/error_spec.rb'
|
- 'spec/requests/error_spec.rb'
|
||||||
|
|
|
@ -21,24 +21,34 @@ module ApplicationController::HasUser
|
||||||
@_current_user ||= User.lookup(id: session[:user_id]) # rubocop:disable Naming/MemoizedInstanceVariableName
|
@_current_user ||= User.lookup(id: session[:user_id]) # rubocop:disable Naming/MemoizedInstanceVariableName
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def request_header_from
|
||||||
|
@request_header_from ||= begin
|
||||||
|
if request.headers['X-On-Behalf-Of'].present?
|
||||||
|
ActiveSupport::Deprecation.warn("Header 'X-On-Behalf-Of' is deprecated. Please use header 'From' instead.")
|
||||||
|
end
|
||||||
|
|
||||||
|
request.headers['From'] || request.headers['X-On-Behalf-Of']
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
# Finds the user based on the id, login or email which is given
|
# Finds the user based on the id, login or email which is given
|
||||||
# in the headers. If it is found then all api activities are done
|
# in the headers. If it is found then all api activities are done
|
||||||
# with the behalf of user. With this functionality it is possible
|
# with the behalf of user. With this functionality it is possible
|
||||||
# to do changes with a user which is different from the admin user.
|
# to do changes with a user which is different from the admin user.
|
||||||
# E.g. create a ticket as a customer user based on a user with admin rights.
|
# E.g. create a ticket as a customer user based on a user with admin rights.
|
||||||
def current_user_on_behalf
|
def current_user_on_behalf
|
||||||
return if request.headers['X-On-Behalf-Of'].blank? # require header
|
return if request_header_from.blank? # require header
|
||||||
return @_user_on_behalf if @_user_on_behalf # return memoized user
|
return @_user_on_behalf if @_user_on_behalf # return memoized user
|
||||||
return if !current_user_real # require session user
|
return if !current_user_real # require session user
|
||||||
if !SessionsPolicy.new(current_user_real, Sessions).impersonate?
|
if !SessionsPolicy.new(current_user_real, Sessions).impersonate?
|
||||||
raise Exceptions::Forbidden, "Current user has no permission to use 'X-On-Behalf-Of'!"
|
raise Exceptions::Forbidden, "Current user has no permission to use 'From'/'X-On-Behalf-Of'!"
|
||||||
end
|
end
|
||||||
|
|
||||||
@_user_on_behalf = find_on_behalf_user request.headers['X-On-Behalf-Of'].to_s.downcase.strip
|
@_user_on_behalf = find_on_behalf_user request_header_from.to_s.downcase.strip
|
||||||
|
|
||||||
# no behalf of user found
|
# no behalf of user found
|
||||||
if !@_user_on_behalf
|
if !@_user_on_behalf
|
||||||
raise Exceptions::Forbidden, "No such user '#{request.headers['X-On-Behalf-Of']}'"
|
raise Exceptions::Forbidden, "No such user '#{request_header_from}'"
|
||||||
end
|
end
|
||||||
|
|
||||||
@_user_on_behalf
|
@_user_on_behalf
|
||||||
|
|
|
@ -2,7 +2,7 @@
|
||||||
|
|
||||||
require 'rails_helper'
|
require 'rails_helper'
|
||||||
|
|
||||||
RSpec.describe 'Api Auth On Behalf Of', type: :request do
|
RSpec.describe 'Api Auth From', type: :request do
|
||||||
|
|
||||||
let(:admin) do
|
let(:admin) do
|
||||||
create(:admin, groups: Group.all)
|
create(:admin, groups: Group.all)
|
||||||
|
@ -11,12 +11,12 @@ RSpec.describe 'Api Auth On Behalf Of', type: :request do
|
||||||
create(:agent)
|
create(:agent)
|
||||||
end
|
end
|
||||||
let(:customer) do
|
let(:customer) do
|
||||||
create(:customer, firstname: 'Behalf of')
|
create(:customer, firstname: 'From')
|
||||||
end
|
end
|
||||||
|
|
||||||
describe 'request handling' do
|
describe 'request handling' do
|
||||||
|
|
||||||
it 'does X-On-Behalf-Of auth - ticket create admin for customer by id' do
|
it 'does From auth - ticket create admin for customer by id' do
|
||||||
params = {
|
params = {
|
||||||
title: 'a new ticket #3',
|
title: 'a new ticket #3',
|
||||||
group: 'Users',
|
group: 'Users',
|
||||||
|
@ -27,14 +27,14 @@ RSpec.describe 'Api Auth On Behalf Of', type: :request do
|
||||||
body: 'some test 123',
|
body: 'some test 123',
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
authenticated_as(admin, on_behalf_of: customer.id)
|
authenticated_as(admin, from: customer.id)
|
||||||
post '/api/v1/tickets', params: params, as: :json
|
post '/api/v1/tickets', params: params, as: :json
|
||||||
expect(response).to have_http_status(:created)
|
expect(response).to have_http_status(:created)
|
||||||
expect(json_response).to be_a_kind_of(Hash)
|
expect(json_response).to be_a_kind_of(Hash)
|
||||||
expect(customer.id).to eq(json_response['created_by_id'])
|
expect(customer.id).to eq(json_response['created_by_id'])
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'does X-On-Behalf-Of auth - ticket create admin for customer by login (upcase)' do
|
it 'does From auth - ticket create admin for customer by login (upcase)' do
|
||||||
params = {
|
params = {
|
||||||
title: 'a new ticket #3',
|
title: 'a new ticket #3',
|
||||||
group: 'Users',
|
group: 'Users',
|
||||||
|
@ -45,14 +45,14 @@ RSpec.describe 'Api Auth On Behalf Of', type: :request do
|
||||||
body: 'some test 123',
|
body: 'some test 123',
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
authenticated_as(admin, on_behalf_of: customer.login.upcase)
|
authenticated_as(admin, from: customer.login.upcase)
|
||||||
post '/api/v1/tickets', params: params, as: :json
|
post '/api/v1/tickets', params: params, as: :json
|
||||||
expect(response).to have_http_status(:created)
|
expect(response).to have_http_status(:created)
|
||||||
expect(json_response).to be_a_kind_of(Hash)
|
expect(json_response).to be_a_kind_of(Hash)
|
||||||
expect(customer.id).to eq(json_response['created_by_id'])
|
expect(customer.id).to eq(json_response['created_by_id'])
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'does X-On-Behalf-Of auth - ticket create admin for customer by login' do
|
it 'does From auth - ticket create admin for customer by login' do
|
||||||
ActivityStream.cleanup(1.year)
|
ActivityStream.cleanup(1.year)
|
||||||
|
|
||||||
params = {
|
params = {
|
||||||
|
@ -65,7 +65,7 @@ RSpec.describe 'Api Auth On Behalf Of', type: :request do
|
||||||
body: 'some test 123',
|
body: 'some test 123',
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
authenticated_as(admin, on_behalf_of: customer.login)
|
authenticated_as(admin, from: customer.login)
|
||||||
post '/api/v1/tickets', params: params, as: :json
|
post '/api/v1/tickets', params: params, as: :json
|
||||||
expect(response).to have_http_status(:created)
|
expect(response).to have_http_status(:created)
|
||||||
json_response_ticket = json_response
|
json_response_ticket = json_response
|
||||||
|
@ -108,7 +108,7 @@ RSpec.describe 'Api Auth On Behalf Of', type: :request do
|
||||||
expect(customer.id).to eq(ticket_created.created_by_id)
|
expect(customer.id).to eq(ticket_created.created_by_id)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'does X-On-Behalf-Of auth - ticket create admin for customer by email' do
|
it 'does From auth - ticket create admin for customer by email' do
|
||||||
params = {
|
params = {
|
||||||
title: 'a new ticket #3',
|
title: 'a new ticket #3',
|
||||||
group: 'Users',
|
group: 'Users',
|
||||||
|
@ -119,14 +119,14 @@ RSpec.describe 'Api Auth On Behalf Of', type: :request do
|
||||||
body: 'some test 123',
|
body: 'some test 123',
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
authenticated_as(admin, on_behalf_of: customer.email)
|
authenticated_as(admin, from: customer.email)
|
||||||
post '/api/v1/tickets', params: params, as: :json
|
post '/api/v1/tickets', params: params, as: :json
|
||||||
expect(response).to have_http_status(:created)
|
expect(response).to have_http_status(:created)
|
||||||
expect(json_response).to be_a_kind_of(Hash)
|
expect(json_response).to be_a_kind_of(Hash)
|
||||||
expect(customer.id).to eq(json_response['created_by_id'])
|
expect(customer.id).to eq(json_response['created_by_id'])
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'does X-On-Behalf-Of auth - ticket create admin for unknown' do
|
it 'does From auth - ticket create admin for unknown' do
|
||||||
params = {
|
params = {
|
||||||
title: 'a new ticket #3',
|
title: 'a new ticket #3',
|
||||||
group: 'Users',
|
group: 'Users',
|
||||||
|
@ -137,7 +137,7 @@ RSpec.describe 'Api Auth On Behalf Of', type: :request do
|
||||||
body: 'some test 123',
|
body: 'some test 123',
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
authenticated_as(admin, on_behalf_of: 99_449_494_949)
|
authenticated_as(admin, from: 99_449_494_949)
|
||||||
post '/api/v1/tickets', params: params, as: :json
|
post '/api/v1/tickets', params: params, as: :json
|
||||||
expect(response).to have_http_status(:forbidden)
|
expect(response).to have_http_status(:forbidden)
|
||||||
expect(@response.header).not_to be_key('Access-Control-Allow-Origin')
|
expect(@response.header).not_to be_key('Access-Control-Allow-Origin')
|
||||||
|
@ -145,7 +145,7 @@ RSpec.describe 'Api Auth On Behalf Of', type: :request do
|
||||||
expect(json_response['error']).to eq("No such user '99449494949'")
|
expect(json_response['error']).to eq("No such user '99449494949'")
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'does X-On-Behalf-Of auth - ticket create customer for admin' do
|
it 'does From auth - ticket create customer for admin' do
|
||||||
params = {
|
params = {
|
||||||
title: 'a new ticket #3',
|
title: 'a new ticket #3',
|
||||||
group: 'Users',
|
group: 'Users',
|
||||||
|
@ -156,15 +156,15 @@ RSpec.describe 'Api Auth On Behalf Of', type: :request do
|
||||||
body: 'some test 123',
|
body: 'some test 123',
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
authenticated_as(customer, on_behalf_of: admin.email)
|
authenticated_as(customer, from: admin.email)
|
||||||
post '/api/v1/tickets', params: params, as: :json
|
post '/api/v1/tickets', params: params, as: :json
|
||||||
expect(response).to have_http_status(:forbidden)
|
expect(response).to have_http_status(:forbidden)
|
||||||
expect(@response.header).not_to be_key('Access-Control-Allow-Origin')
|
expect(@response.header).not_to be_key('Access-Control-Allow-Origin')
|
||||||
expect(json_response).to be_a_kind_of(Hash)
|
expect(json_response).to be_a_kind_of(Hash)
|
||||||
expect(json_response['error']).to eq("Current user has no permission to use 'X-On-Behalf-Of'!")
|
expect(json_response['error']).to eq("Current user has no permission to use 'From'/'X-On-Behalf-Of'!")
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'does X-On-Behalf-Of auth - ticket create admin for customer by email but no permitted action' do
|
it 'does From auth - ticket create admin for customer by email but no permitted action' do
|
||||||
params = {
|
params = {
|
||||||
title: 'a new ticket #3',
|
title: 'a new ticket #3',
|
||||||
group: 'secret1234',
|
group: 'secret1234',
|
||||||
|
@ -175,7 +175,7 @@ RSpec.describe 'Api Auth On Behalf Of', type: :request do
|
||||||
body: 'some test 123',
|
body: 'some test 123',
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
authenticated_as(admin, on_behalf_of: customer.email)
|
authenticated_as(admin, from: customer.email)
|
||||||
post '/api/v1/tickets', params: params, as: :json
|
post '/api/v1/tickets', params: params, as: :json
|
||||||
expect(response).to have_http_status(:unprocessable_entity)
|
expect(response).to have_http_status(:unprocessable_entity)
|
||||||
expect(@response.header).not_to be_key('Access-Control-Allow-Origin')
|
expect(@response.header).not_to be_key('Access-Control-Allow-Origin')
|
||||||
|
@ -204,7 +204,7 @@ RSpec.describe 'Api Auth On Behalf Of', type: :request do
|
||||||
body: 'some test 123',
|
body: 'some test 123',
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
authenticated_as(admin, on_behalf_of: customer.email, token: token)
|
authenticated_as(admin, from: customer.email, token: token)
|
||||||
post '/api/v1/tickets', params: params, as: :json
|
post '/api/v1/tickets', params: params, as: :json
|
||||||
expect(response).to have_http_status(:created)
|
expect(response).to have_http_status(:created)
|
||||||
expect(json_response).to be_a_kind_of(Hash)
|
expect(json_response).to be_a_kind_of(Hash)
|
||||||
|
@ -232,7 +232,7 @@ RSpec.describe 'Api Auth On Behalf Of', type: :request do
|
||||||
body: 'some test 123',
|
body: 'some test 123',
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
authenticated_as(admin, on_behalf_of: customer.email)
|
authenticated_as(admin, from: customer.email)
|
||||||
post '/api/v1/tickets', params: params, as: :json
|
post '/api/v1/tickets', params: params, as: :json
|
||||||
expect(response).to have_http_status(:created)
|
expect(response).to have_http_status(:created)
|
||||||
expect(customer.id).to eq(json_response['created_by_id'])
|
expect(customer.id).to eq(json_response['created_by_id'])
|
||||||
|
@ -243,29 +243,29 @@ RSpec.describe 'Api Auth On Behalf Of', type: :request do
|
||||||
end
|
end
|
||||||
|
|
||||||
describe 'user lookup' do
|
describe 'user lookup' do
|
||||||
it 'does X-On-Behalf-Of auth - user lookup by ID' do
|
it 'does From auth - user lookup by ID' do
|
||||||
authenticated_as(admin, on_behalf_of: customer.id)
|
authenticated_as(admin, from: customer.id)
|
||||||
get '/api/v1/users/me', as: :json
|
get '/api/v1/users/me', as: :json
|
||||||
expect(json_response.fetch('id')).to be customer.id
|
expect(json_response.fetch('id')).to be customer.id
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'does X-On-Behalf-Of auth - user lookup by login' do
|
it 'does From auth - user lookup by login' do
|
||||||
authenticated_as(admin, on_behalf_of: customer.login)
|
authenticated_as(admin, from: customer.login)
|
||||||
get '/api/v1/users/me', as: :json
|
get '/api/v1/users/me', as: :json
|
||||||
expect(json_response.fetch('id')).to be customer.id
|
expect(json_response.fetch('id')).to be customer.id
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'does X-On-Behalf-Of auth - user lookup by email' do
|
it 'does From auth - user lookup by email' do
|
||||||
authenticated_as(admin, on_behalf_of: customer.email)
|
authenticated_as(admin, from: customer.email)
|
||||||
get '/api/v1/users/me', as: :json
|
get '/api/v1/users/me', as: :json
|
||||||
expect(json_response.fetch('id')).to be customer.id
|
expect(json_response.fetch('id')).to be customer.id
|
||||||
end
|
end
|
||||||
|
|
||||||
# https://github.com/zammad/zammad/issues/2851
|
# https://github.com/zammad/zammad/issues/2851
|
||||||
it 'does X-On-Behalf-Of auth - user lookup by email even if email starts with a digit' do
|
it 'does From auth - user lookup by email even if email starts with a digit' do
|
||||||
customer.update! email: "#{agent.id}#{customer.email}"
|
customer.update! email: "#{agent.id}#{customer.email}"
|
||||||
|
|
||||||
authenticated_as(admin, on_behalf_of: customer.email)
|
authenticated_as(admin, from: customer.email)
|
||||||
get '/api/v1/users/me', as: :json
|
get '/api/v1/users/me', as: :json
|
||||||
expect(json_response.fetch('id')).to be customer.id
|
expect(json_response.fetch('id')).to be customer.id
|
||||||
end
|
end
|
|
@ -80,7 +80,7 @@ module ZammadSpecSupportRequest
|
||||||
when :api_client
|
when :api_client
|
||||||
# ensure that always the correct header value is set
|
# ensure that always the correct header value is set
|
||||||
# otherwise previous header configurations will be re-used
|
# otherwise previous header configurations will be re-used
|
||||||
add_headers('X-On-Behalf-Of' => options[:on_behalf_of])
|
add_headers('From' => options[:from])
|
||||||
|
|
||||||
# if we want to authenticate by token
|
# if we want to authenticate by token
|
||||||
credentials = if options[:token].present?
|
credentials = if options[:token].present?
|
||||||
|
|
Loading…
Reference in a new issue