petalmd / armor

Apache License 2.0
15 stars 8 forks source link

Issue in TokenEvaluator #14

Open ersushantsood opened 8 years ago

ersushantsood commented 8 years ago

Hi @jmaitrehenry , i was trying to validate the logic written in TokenEvaluator to check if ROLE A has access to Index A only . This seems not working as After authenticating and getting the roles for the user , the ACL for Indices is not getting applied , can you please verify once .

jmaitrehenry commented 8 years ago

Hi @ersushantsood , I will check it soon but can you write a test that validate this issue? This will help me to fixe the issue and for not having it in futur version.

ersushantsood commented 8 years ago

Hi jmaitrehenry , apologies for not able to provide a test as i am hardly able to get some free time . I made few changes in TokenEvaluator locally to make index level ACL working , but before i validate i would like to know the design decision behind this class . Does design always want to execute the default ACL bypass filter even if any user or role specific ACL is also defined in acl? Current code is taking superset of Default and role specific ACL and executing the bypass out of them.

ersushantsood commented 8 years ago

Hi @jmaitrehenry , If you create below entries in armor and try to restrict access using restrictOnly action filter then also TokenEvaluator always bypass the filters by giving precedence to Default value. Is it a desired design ? I modified the design as per my needs where i will consider Default ACL only if none of the ACL matches as per Role/User/IndexRequest. Please comment on the design of TokenEvaluator:

curl -u kibana -XPUT 'http://localhost:9200/armor/ac/ac?pretty' -d ' {"acl": [ { "Comment": "Default is to execute all filters", "filters_bypass": ["*"], "filters_execute": [] }, { "Comment": "This role is for LDAP User and role value is taken from LDAP memberOf attribute", "users": ["admin"], "roles": ["support","user"], "indices": ["logstash-support-2016.01.01"], "filters_bypass": [], "filters_execute": ["actionrequestfilter.readwrite"] }, { "Comment": "This role is for LDAP User and role value is taken from LDAP memberOf attribute", "users": ["admin"], "roles": ["logstash","user"], "indices": ["logstash-user-2016.01.01","logstash-logstash-2016.01.01"], "filters_bypass": [], "filters_execute": ["actionrequestfilter.restrictonly"] }

]}'

ersushantsood commented 8 years ago

Hi I am running armor successfully after making certain fixes with Elasticsearch 2.1 .Can I also become a co admin to armor to maintain this plugin as I will add additional features in armor in future and I don't want to work in silo.

jmaitrehenry commented 8 years ago

Hi @ersushantsood ! I add you as a collaborator. Thanks for your help!

ersushantsood commented 8 years ago

I have fixed this bug , i will move this change soon but want to test few more scenarios.

jmaitrehenry commented 8 years ago

No problem, thanks!