Improved validation messages of controllers.

This commit is contained in:
Martin Edenhofer 2016-08-23 20:46:04 +02:00
parent 14e1b5a404
commit 2820639c42
12 changed files with 405 additions and 50 deletions

View file

@ -278,7 +278,7 @@ class ApplicationController < ActionController::Base
permission: auth_param[:permission],
inactive_user: true,
)
raise Exceptions::NotAuthorized, 'No permission (token)!' if !user
raise Exceptions::NotAuthorized, 'Not authorized (token)!' if !user
end
@_token_auth = token # remember for permission_check
return authentication_check_prerequesits(user, 'token_auth', auth_param) if user
@ -319,7 +319,7 @@ class ApplicationController < ActionController::Base
# check scopes / permission check
if auth_param[:permission] && !user.permissions?(auth_param[:permission])
raise Exceptions::NotAuthorized, 'No permission (user)!'
raise Exceptions::NotAuthorized, 'Not authorized (user)!'
end
current_user_set(user)
@ -364,11 +364,11 @@ class ApplicationController < ActionController::Base
permission: key,
)
return false if user
raise Exceptions::NotAuthorized, 'No permission (token)!'
raise Exceptions::NotAuthorized, 'Not authorized (token)!'
end
return false if current_user && current_user.permissions?(key)
raise Exceptions::NotAuthorized, 'No permission (user)!'
raise Exceptions::NotAuthorized, 'Not authorized (user)!'
end
def valid_session_with_user
@ -543,6 +543,14 @@ class ApplicationController < ActionController::Base
if error =~ /(already exists|duplicate key|duplicate entry)/i
data[:error_human] = 'Object already exists!'
end
if error =~ /null value in column "(.+?)" violates not-null constraint/i
data[:error_human] = "Attribute '#{$1}' required!"
end
if Rails.env.production? && !data[:error_human].empty?
data[:error] = data[:error_human]
data.delete('error_human')
end
data
end
@ -598,7 +606,11 @@ class ApplicationController < ActionController::Base
end
def unauthorized(e)
error = model_match_error(e.message)
message = e.message
if message == 'Exceptions::NotAuthorized'
message = 'Not authorized'
end
error = model_match_error(message)
if error && error[:error]
response.headers['X-Failure'] = error[:error_human] || error[:error]
end

View file

@ -74,6 +74,14 @@ class TicketsController < ApplicationController
clean_params = Ticket.param_association_lookup(params)
clean_params = Ticket.param_cleanup(clean_params, true)
# overwrite params
if !current_user.permissions?('ticket.agent')
[:owner, :owner_id, :customer, :customer_id, :organization, :organization_id, :preferences].each { |key|
clean_params.delete(key)
}
clean_params[:customer_id] = current_user.id
end
# try to create customer if needed
if clean_params[:customer_id] && clean_params[:customer_id] =~ /^guess:(.+?)$/
email = $1
@ -105,10 +113,7 @@ class TicketsController < ApplicationController
end
# create ticket
if !ticket.save
render json: ticket.errors, status: :unprocessable_entity
return
end
ticket.save!
# create tags if given
if params[:tags] && !params[:tags].empty?
@ -128,12 +133,6 @@ class TicketsController < ApplicationController
article_create(ticket, params[:article])
end
if params[:expand]
result = ticket.attributes_with_relation_names
render json: result, status: :created
return
end
# create links (e. g. in case of ticket split)
# links: {
# Ticket: {
@ -161,6 +160,12 @@ class TicketsController < ApplicationController
}
end
if params[:expand]
result = ticket.attributes_with_relation_names
render json: result, status: :created
return
end
render json: ticket, status: :created
end
@ -174,7 +179,14 @@ class TicketsController < ApplicationController
clean_params = Ticket.param_association_lookup(params)
clean_params = Ticket.param_cleanup(clean_params, true)
if ticket.update_attributes(clean_params)
# overwrite params
if !current_user.permissions?('ticket.agent')
[:owner, :owner_id, :customer, :customer_id, :organization, :organization_id, :preferences].each { |key|
clean_params.delete(key)
}
end
ticket.update_attributes!(clean_params)
if params[:article]
article_create(ticket, params[:article])
@ -187,9 +199,6 @@ class TicketsController < ApplicationController
end
render json: ticket, status: :ok
else
render json: ticket.errors, status: :unprocessable_entity
end
end
# DELETE /api/v1/tickets/1
@ -199,7 +208,9 @@ class TicketsController < ApplicationController
ticket = Ticket.find(params[:id])
ticket_permission(ticket)
ticket.destroy
raise Exceptions::NotAuthorized, 'Not authorized (admin permission required)!' if !current_user.permissions?('admin')
ticket.destroy!
head :ok
end
@ -612,8 +623,36 @@ class TicketsController < ApplicationController
form_id = params[:form_id]
params.delete(:form_id)
# check min. params
raise 'Need at least article: { body: "some text" }' if !params[:body]
# fill default values
if params[:type_id].empty?
params[:type_id] = Ticket::Article::Type.lookup(name: 'note').id
end
if params[:sender_id].empty?
sender = 'Customer'
if current_user.permissions?('ticket.agent')
sender = 'Agent'
end
params[:sender_id] = Ticket::Article::Sender.lookup(name: sender).id
end
clean_params = Ticket::Article.param_association_lookup(params)
clean_params = Ticket::Article.param_cleanup(clean_params, true)
# overwrite params
if !current_user.permissions?('ticket.agent')
clean_params[:sender_id] = Ticket::Article::Sender.lookup(name: 'Customer').id
clean_params.delete(:sender)
type = Ticket::Article::Type.lookup(id: clean_params[:type_id])
if type !~ /^(note|web)$/
clean_params[:type_id] = Ticket::Article::Type.lookup(name: 'note').id
end
clean_params.delete(:type)
clean_params[:internal] = false
end
article = Ticket::Article.new(clean_params)
article.ticket_id = ticket.id
@ -646,10 +685,7 @@ class TicketsController < ApplicationController
o_id: form_id,
)
end
if !article.save
render json: article.errors, status: :unprocessable_entity
return
end
article.save!
# remove attachments from upload cache
return if !form_id

View file

@ -13,11 +13,13 @@ class Observer::Ticket::Article::CommunicateEmail < ActiveRecord::Observer
return if ApplicationHandleInfo.current.split('.')[1] == 'postmaster'
# if sender is customer, do not communicate
return if !record.sender_id
sender = Ticket::Article::Sender.lookup(id: record.sender_id)
return 1 if sender.nil?
return 1 if sender['name'] == 'Customer'
# only apply on emails
return if !record.type_id
type = Ticket::Article::Type.lookup(id: record.type_id)
return if type['name'] != 'email'

View file

@ -15,11 +15,13 @@ class Observer::Ticket::Article::CommunicateFacebook < ActiveRecord::Observer
return if ApplicationHandleInfo.current.split('.')[1] == 'postmaster'
# if sender is customer, do not communicate
return if !record.sender_id
sender = Ticket::Article::Sender.lookup(id: record.sender_id)
return 1 if sender.nil?
return 1 if sender['name'] == 'Customer'
# only apply for facebook
return if !record.type_id
type = Ticket::Article::Type.lookup(id: record.type_id)
return if type['name'] !~ /\Afacebook/

View file

@ -13,11 +13,13 @@ class Observer::Ticket::Article::CommunicateTwitter < ActiveRecord::Observer
return if ApplicationHandleInfo.current.split('.')[1] == 'postmaster'
# if sender is customer, do not communicate
return if !record.sender_id
sender = Ticket::Article::Sender.lookup(id: record.sender_id)
return if sender.nil?
return if sender['name'] == 'Customer'
# only apply on tweets
return if !record.type_id
type = Ticket::Article::Type.lookup(id: record.type_id)
return if type['name'] !~ /\Atwitter/i

View file

@ -13,11 +13,13 @@ class Observer::Ticket::Article::FillupFromEmail < ActiveRecord::Observer
return if ApplicationHandleInfo.current.split('.')[1] == 'postmaster'
# if sender is customer, do not change anything
return if !record.sender_id
sender = Ticket::Article::Sender.lookup(id: record.sender_id)
return if sender.nil?
return if sender['name'] == 'Customer'
# set email attributes
return if !record.type_id
type = Ticket::Article::Type.lookup(id: record.type_id)
return if type['name'] != 'email'

View file

@ -13,6 +13,7 @@ class Observer::Ticket::Article::FillupFromGeneral < ActiveRecord::Observer
return if ApplicationHandleInfo.current.split('.')[1] == 'postmaster'
# if sender is customer, do not change anything
return if !record.sender_id
sender = Ticket::Article::Sender.lookup(id: record.sender_id)
return if sender.nil?
return if sender['name'] == 'Customer'
@ -20,6 +21,7 @@ class Observer::Ticket::Article::FillupFromGeneral < ActiveRecord::Observer
# set from if not given
return if record.from
return if !record.created_by_id
user = User.find(record.created_by_id)
record.from = "#{user.firstname} #{user.lastname}"
end

View file

@ -22,6 +22,7 @@ class Observer::Ticket::CloseTime < ActiveRecord::Observer
return true if record.close_time
# check if ticket is closed now
return if !record.state_id
state = Ticket::State.lookup(id: record.state_id)
state_type = Ticket::StateType.lookup(id: state.state_type_id)
return true if state_type.name != 'closed'

View file

@ -140,7 +140,7 @@ class ApiAuthControllerTest < ActionDispatch::IntegrationTest
assert_response(401)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal('No permission (token)!', result['error'])
assert_equal('Not authorized (token)!', result['error'])
admin_token.preferences[:permission] = []
admin_token.save!
@ -149,7 +149,7 @@ class ApiAuthControllerTest < ActionDispatch::IntegrationTest
assert_response(401)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal('No permission (token)!', result['error'])
assert_equal('Not authorized (token)!', result['error'])
@admin.active = false
@admin.save!
@ -182,7 +182,7 @@ class ApiAuthControllerTest < ActionDispatch::IntegrationTest
assert_response(401)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal('No permission (token)!', result['error'])
assert_equal('Not authorized (token)!', result['error'])
admin_token.preferences[:permission] = ['admin.session_not_existing', 'admin.role']
admin_token.save!

View file

@ -111,7 +111,7 @@ class PackagesControllerTest < ActionDispatch::IntegrationTest
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_not(result['packages'])
assert_equal('No permission (user)!', result['error'])
assert_equal('Not authorized (user)!', result['error'])
end
test '06 packages index with customer' do
@ -125,7 +125,7 @@ class PackagesControllerTest < ActionDispatch::IntegrationTest
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_not(result['packages'])
assert_equal('No permission (user)!', result['error'])
assert_equal('Not authorized (user)!', result['error'])
end
end

View file

@ -82,7 +82,7 @@ class SettingsControllerTest < ActionDispatch::IntegrationTest
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_not(result['settings'])
assert_equal('No permission (user)!', result['error'])
assert_equal('Not authorized (user)!', result['error'])
end
test 'settings index with customer' do
@ -95,7 +95,7 @@ class SettingsControllerTest < ActionDispatch::IntegrationTest
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_not(result['settings'])
assert_equal('No permission (user)!', result['error'])
assert_equal('Not authorized (user)!', result['error'])
end
end

View file

@ -50,10 +50,83 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
end
test '01 ticket create with agent' do
test '01.01 ticket create with agent - missing group' do
credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
params = {
title: 'a new ticket #1',
article: {
content_type: 'text/plain', # or text/html
body: 'some body',
sender: 'Customer',
type: 'note',
},
}
post '/api/v1/tickets', params.to_json, @headers.merge('Authorization' => credentials)
assert_response(500)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal('Attribute \'group_id\' required!', result['error_human'])
end
test '01.02 ticket create with agent - wrong group' do
credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
params = {
title: 'a new ticket #2',
group: 'not_existing',
article: {
content_type: 'text/plain', # or text/html
body: 'some body',
sender: 'Customer',
type: 'note',
},
}
post '/api/v1/tickets', params.to_json, @headers.merge('Authorization' => credentials)
assert_response(500)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal('No lookup value found for \'group\': "not_existing"', result['error'])
end
test '01.03 ticket create with agent - missing article.body' do
credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
params = {
title: 'a new ticket #3',
group: 'Users',
priority: '2 normal',
state: 'new',
customer_id: @customer_without_org.id,
article: {},
}
post '/api/v1/tickets', params.to_json, @headers.merge('Authorization' => credentials)
assert_response(500)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal('Need at least article: { body: "some text" }', result['error'])
end
test '01.03 ticket create with agent - minimal article' do
credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
params = {
title: 'a new ticket #3',
group: 'Users',
priority: '2 normal',
state: 'new',
customer_id: @customer_without_org.id,
article: {
body: 'some test 123',
},
}
post '/api/v1/tickets', params.to_json, @headers.merge('Authorization' => credentials)
assert_response(201)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
assert_equal('a new ticket #3', result['title'])
assert_equal(@customer_without_org.id, result['customer_id'])
end
test '02.02 ticket create with agent' do
credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
params = {
title: 'a new ticket #1',
state: 'new',
@ -63,8 +136,6 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
article: {
content_type: 'text/plain', # or text/html
body: 'some body',
sender: 'Customer',
type: 'note',
},
links: {
Ticket: {
@ -72,20 +143,245 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
}
}
}
post '/api/v1/tickets', params.to_json, @headers.merge('Authorization' => credentials)
assert_response(201)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
assert_equal('a new ticket #1', result['title'])
links = Link.list(
link_object: 'Ticket',
link_object_value: result['id'],
)
p links.inspect
assert_equal('child', links[0]['link_type'])
assert_equal('Ticket', links[0]['link_object'])
assert_equal(1, links[0]['link_object_value'])
end
test '02.03 ticket with wrong ticket id' do
group = Group.create_or_update(
name: "GroupWithoutPermission-#{rand(9_999_999_999)}",
active: true,
updated_by_id: 1,
created_by_id: 1,
)
ticket = Ticket.create!(
title: 'ticket with wrong ticket id',
group_id: group.id,
customer_id: @customer_without_org.id,
state: Ticket::State.lookup(name: 'new'),
priority: Ticket::Priority.lookup(name: '2 normal'),
updated_by_id: 1,
created_by_id: 1,
)
credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
get "/api/v1/tickets/#{ticket.id}", {}, @headers.merge('Authorization' => credentials)
assert_response(401)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal('Not authorized', result['error'])
params = {
title: 'ticket with wrong ticket id - 2',
}
put "/api/v1/tickets/#{ticket.id}", params.to_json, @headers.merge('Authorization' => credentials)
assert_response(401)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal('Not authorized', result['error'])
delete "/api/v1/tickets/#{ticket.id}", {}.to_json, @headers.merge('Authorization' => credentials)
assert_response(401)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal('Not authorized', result['error'])
end
test '02.04 ticket with correct ticket id' do
ticket = Ticket.create!(
title: 'ticket with corret ticket id',
group: Group.lookup(name: 'Users'),
customer_id: @customer_without_org.id,
state: Ticket::State.lookup(name: 'new'),
priority: Ticket::Priority.lookup(name: '2 normal'),
updated_by_id: 1,
created_by_id: 1,
)
credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
get "/api/v1/tickets/#{ticket.id}", {}, @headers.merge('Authorization' => credentials)
assert_response(200)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal(ticket.id, result['id'])
assert_equal('ticket with corret ticket id', result['title'])
assert_equal(ticket.customer_id, result['customer_id'])
params = {
title: 'ticket with corret ticket id - 2',
customer_id: @agent.id,
}
put "/api/v1/tickets/#{ticket.id}", params.to_json, @headers.merge('Authorization' => credentials)
assert_response(200)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal(ticket.id, result['id'])
assert_equal('ticket with corret ticket id - 2', result['title'])
assert_equal(@agent.id, result['customer_id'])
delete "/api/v1/tickets/#{ticket.id}", {}.to_json, @headers.merge('Authorization' => credentials)
assert_response(401)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal('Not authorized (admin permission required)!', result['error'])
end
test '02.05 ticket with correct ticket id' do
ticket = Ticket.create!(
title: 'ticket with corret ticket id',
group: Group.lookup(name: 'Users'),
customer_id: @customer_without_org.id,
state: Ticket::State.lookup(name: 'new'),
priority: Ticket::Priority.lookup(name: '2 normal'),
updated_by_id: 1,
created_by_id: 1,
)
credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-admin', 'adminpw')
get "/api/v1/tickets/#{ticket.id}", {}, @headers.merge('Authorization' => credentials)
assert_response(200)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal(ticket.id, result['id'])
assert_equal('ticket with corret ticket id', result['title'])
assert_equal(ticket.customer_id, result['customer_id'])
params = {
title: 'ticket with corret ticket id - 2',
customer_id: @agent.id,
}
put "/api/v1/tickets/#{ticket.id}", params.to_json, @headers.merge('Authorization' => credentials)
assert_response(200)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal(ticket.id, result['id'])
assert_equal('ticket with corret ticket id - 2', result['title'])
assert_equal(@agent.id, result['customer_id'])
delete "/api/v1/tickets/#{ticket.id}", {}.to_json, @headers.merge('Authorization' => credentials)
assert_response(200)
end
test '03.01 ticket create with customer minimal' do
credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-customer1@example.com', 'customer1pw')
params = {
title: 'a new ticket #c1',
state: 'new',
priority: '2 normal',
group: 'Users',
article: {
body: 'some body',
},
}
post '/api/v1/tickets', params.to_json, @headers.merge('Authorization' => credentials)
assert_response(201)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
assert_equal('a new ticket #c1', result['title'])
assert_equal(@customer_without_org.id, result['customer_id'])
end
test '03.02 ticket create with customer with wrong customer' do
credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-customer1@example.com', 'customer1pw')
params = {
title: 'a new ticket #c2',
state: 'new',
priority: '2 normal',
group: 'Users',
customer_id: @agent.id,
article: {
content_type: 'text/plain', # or text/html
body: 'some body',
sender: 'System',
},
}
post '/api/v1/tickets', params.to_json, @headers.merge('Authorization' => credentials)
assert_response(201)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
assert_equal('a new ticket #c2', result['title'])
assert_equal(@customer_without_org.id, result['customer_id'])
end
test '03.03 ticket with wrong ticket id' do
ticket = Ticket.create!(
title: 'ticket with wrong ticket id',
group: Group.lookup(name: 'Users'),
customer_id: @agent.id,
state: Ticket::State.lookup(name: 'new'),
priority: Ticket::Priority.lookup(name: '2 normal'),
updated_by_id: 1,
created_by_id: 1,
)
credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-customer1@example.com', 'customer1pw')
get "/api/v1/tickets/#{ticket.id}", {}, @headers.merge('Authorization' => credentials)
assert_response(401)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal('Not authorized', result['error'])
params = {
title: 'ticket with wrong ticket id - 2',
}
put "/api/v1/tickets/#{ticket.id}", params.to_json, @headers.merge('Authorization' => credentials)
assert_response(401)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal('Not authorized', result['error'])
delete "/api/v1/tickets/#{ticket.id}", {}.to_json, @headers.merge('Authorization' => credentials)
assert_response(401)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal('Not authorized', result['error'])
end
test '03.04 ticket with correct ticket id' do
ticket = Ticket.create!(
title: 'ticket with corret ticket id',
group: Group.lookup(name: 'Users'),
customer_id: @customer_without_org.id,
state: Ticket::State.lookup(name: 'new'),
priority: Ticket::Priority.lookup(name: '2 normal'),
updated_by_id: 1,
created_by_id: 1,
)
credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-customer1@example.com', 'customer1pw')
get "/api/v1/tickets/#{ticket.id}", {}, @headers.merge('Authorization' => credentials)
assert_response(200)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal(ticket.id, result['id'])
assert_equal('ticket with corret ticket id', result['title'])
assert_equal(ticket.customer_id, result['customer_id'])
params = {
title: 'ticket with corret ticket id - 2',
customer_id: @agent.id,
}
put "/api/v1/tickets/#{ticket.id}", params.to_json, @headers.merge('Authorization' => credentials)
assert_response(200)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal(ticket.id, result['id'])
assert_equal('ticket with corret ticket id - 2', result['title'])
assert_equal(ticket.customer_id, result['customer_id'])
delete "/api/v1/tickets/#{ticket.id}", {}.to_json, @headers.merge('Authorization' => credentials)
assert_response(401)
result = JSON.parse(@response.body)
assert_equal(Hash, result.class)
assert_equal('Not authorized (admin permission required)!', result['error'])
end
end