This repository was archived by the owner on Oct 19, 2018. It is now read-only.
Repository navigation
This repository was archived by the owner on Oct 19, 2018. It is now read-only.
Security hole - it might be possible to get a list of all id's for a model #45
Description
this
reads every id, but is not protected by the check_permission_with_acting_user method
it should read:
collection = cache_item.value.collect do |record|
record.check_permission_with_acting_user(@acting_user, :view_permitted?, :id)
record.id
end
cache_item.build_new_cache_item(collection, method, method)Similar to #43 .
Hopefully adding that line will not kill performance!
Metadata
Metadata
Assignees
Labels
No labels
Activity
catmando commentedon Dec 13, 2017
Be aware however that any record that has ANY attribute accessible to a channel will automatically be grant access of the id to that channel. This is to allow push synchronization. So for example if my User instance channel has access to the user's name (for example) then each user will also have access to the :id of each record that belongs to them.
I don't think this is an issue FYI... but if it is then we would have to implement an optional encryption of the id before transmission (and decryption after reception.) I would make optional so that if you are not worried about it you don't have to pay for the encrypt/decrypt overhead. Also would probably be handy during development not to encyrpt/decrypt the ids.
catmando commentedon Dec 13, 2017
FYI aggregations might have a similar problem, however i think we should just Make a caveat, as those things are so hard to deal with. I would rather we just had a config switch that turned them off.
janbiedermann commentedon Dec 14, 2017
Thanks for generously assigning this to me. I will immediately put it, in bold, to make sure of its importance, on my hyperloop todo list. To fix it ASAP, i put it at the end of the list, so it will be the first item after all the other items on my list.
catmando commentedon Dec 14, 2017
:-)
catmando commentedon Dec 14, 2017
I'll try to get done as well
catmando commentedon Dec 14, 2017
@janbiedermann sorry abt assigning problem. Fix should be just as shown, but I am concerned about perf impact. You r kind of master if that. If you check it out perf wise I will do test cases etc.
janbiedermann commentedon Dec 29, 2017
in progress. So far:
lib/active_record_base.rb:
def hyper_model_scopesdef server_methodslib/reactive_record/permissions.rb
Hyperloop::InternalPolicyto lib/hyperloop/internal_policy.rbReactiveRecord::AccessViolationto lib/reactive_record/access_violation.rb(hyper-mesh.rb modified accordingly)
lib/hyperloop/internal_policy.rb
added:
def self.guard_root_fetch_method!(model, method)=> true|ReactiveRecord::AccessViolation
def self.guard_record_fetch_method!(record, method)=> true|ReactiveRecord::AccessViolation
def self.guard_relation_fetch_method!(relation, method)=> true|ReactiveRecord::AccessViolation
def self.raise_access_violationadded in lib/reactive_record/proxy
class AttributeNodea = ReactiveRecord::Proxy::AttributeNode.new(record, attribute, value)a.value(Rails)param: must pass Rails object to ensure local call
=> value|ReactiveRecord::AccessViolation
r = ReactiveRecord::Proxy::RecordNode.new(record, acting_user)r = ReactiveRecord::Proxy::RelationNode.new(relation, acting_user)r = ReactiveRecord::Proxy::RootNode.new(model_name, acting_user)Proxy Interface may change slightly with further progress, maybe.
added lib/reactive_record/graph.rb
class ReactiveRecord::Graph < BasicObjectclass Node < BasicObjectinternal to Graphinterface still in flux
Ill update this to keep track.