hobo_update resets @current_user with @this even if update was failed
Reported by Tomoaki Hayasaka | November 17th, 2010 @ 07:18 PM | in Hobo 1.0X
hobo-1.0.2 and hobo-1.3.0.pre17
hobo_update resets @current_user with
@this even if update was failed, and it could be a
security hole.
Fix for 1.3.0.pre17 (same issue is there in 1.0.2 in lib/hobo/user_controller.rb):
diff --git a/hobo/lib/hobo/controller/model.rb b/hobo/lib/hobo/controller/model.rb
index c791b85..6c19a8a 100644
--- a/hobo/lib/hobo/controller/model.rb
+++ b/hobo/lib/hobo/controller/model.rb
@@ -593,7 +593,7 @@ module Hobo
this.user_update_attributes(current_user, changes)
# Ensure current_user isn't out of date
- @current_user = @this if @this == current_user
+ @current_user = @this if @this == current_user && valid?
in_place_edit_field = changes.keys.first if changes.size == 1 && params[:render]
update_response(in_place_edit_field, options, &b)
Comments and changes to this ticket
-
Bryan Larsen November 19th, 2010 @ 09:49 PM
- State changed from “new” to “open”
- Milestone set to “Hobo 1.0X”
- Milestone order changed from “197921” to “0”
Good point. However, valid? isn't necessarily a cheap operation, and perhaps more importantly, causes validations to be rerun. I know I've had situations in the past where rerunning validations actually lost error messages. I can't think of a good alternative to your patch off the top of my head, though.
-
Betelgeuse November 20th, 2010 @ 09:12 AM
There's is an alternative that doesn't require calling valid? update_attributes that user_updated_attributes eventually calls returns false if the record is invalid. Assuming the user_update_attributes call return value is the update_attributes return value then we can use its return value to decide if @current_user should be updated.
-
Bryan Larsen November 27th, 2010 @ 04:07 AM
(from [6506fabd36c39c2e02c49f2dd5b7fd319ee27f10]) [#857] don't update @current_user if update fails. Credits: Tomoaki Hayasaki and Betelgeuse https://github.com/tablatom/hobo/commit/6506fabd36c39c2e02c49f2dd5b...
-
Bryan Larsen November 27th, 2010 @ 04:07 AM
(from [e433c144d5c2e0ed6f113d63803d9a1f9a33c033]) [#857] don't update @current_user if update fails. Credits: Tomoaki Hayasaki and Betelgeuse
https://github.com/tablatom/hobo/commit/e433c144d5c2e0ed6f113d63803... -
Domizio Demichelis November 27th, 2010 @ 02:46 PM
- State changed from “open” to “resolved”
(from [072cda62da91c62f8bd9255c4c2af6c9eb8f9adf]) don't update @current_user if update fails. Credits: Tomoaki Hayasaki and Betelgeuse [#857 state:resolved] https://github.com/tablatom/hobo/commit/072cda62da91c62f8bd9255c4c2...
Please Sign in or create a free account to add a new ticket.
With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.
Create new ticket
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป
People watching this ticket
Bryan Larsen
Domizio Demichelis
Tags
Referenced by
-
857
hobo_update resets @current_user with @this even if update was failed
(from [6506fabd36c39c2e02c49f2dd5b7fd319ee27f10])
[#857]
...
-
857
hobo_update resets @current_user with @this even if update was failed
(from [e433c144d5c2e0ed6f113d63803d9a1f9a33c033])
[#857]
...
-
857
hobo_update resets @current_user with @this even if update was failed
(from [072cda62da91c62f8bd9255c4c2af6c9eb8f9adf])
don't u...