[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...

2017-03-03 Thread joshelser
GitHub user joshelser opened a pull request:

https://github.com/apache/zookeeper/pull/182

ZOOKEEPER-2709 Clarify documentation around the "auth" ACL scheme

Not sure if I should include the modified files from the result of `ant 
docs`. Happy to do so if expected :)

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/joshelser/zookeeper ZOOKEEPER-2709

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/zookeeper/pull/182.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #182


commit 207146e8df26d3a22199725e2f36c04d473f7e37
Author: Josh Elser 
Date:   2017-03-03T21:28:50Z

ZOOKEEPER-2709 Clarify documentation around the "auth" ACL scheme




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...

2017-03-06 Thread hanm
Github user hanm commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/182#discussion_r104595517
  
--- Diff: src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 
---
@@ -899,9 +899,12 @@
 single id, anyone, that represents
 anyone.
 
-auth doesn't
-use any id, represents any authenticated
-user.
+auth is a 
convenience
+scheme which defaults to the currently-authenticated user and 
scheme.
+Any ID which is provided using this scheme is ignored by ZooKeeper.
--- End diff --

Thanks for the patch, it is a good improvement on doc.

I think the ID here refers to the id of the scheme:id pair of the ID object 
in the ACL, correct? Mentioning this because the auth scheme is also referenced 
in command line where people can do 'setAcl /node auth:username:password:crdwa' 
in which case the username (sometimes overloaded as id) is required. 



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...

2017-03-07 Thread joshelser
Github user joshelser commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/182#discussion_r104744229
  
--- Diff: src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 
---
@@ -899,9 +899,12 @@
 single id, anyone, that represents
 anyone.
 
-auth doesn't
-use any id, represents any authenticated
-user.
+auth is a 
convenience
+scheme which defaults to the currently-authenticated user and 
scheme.
+Any ID which is provided using this scheme is ignored by ZooKeeper.
--- End diff --

Thanks for taking a look, @hanm!

> I think the ID here refers to the id of the scheme:id pair of the ID 
object in the ACL, correct?

Yup, that's what I was intending. Perhaps I should try to clarify that 
better :)

> the auth scheme is also referenced in command line where people can do 
'setAcl /node auth:username:password:crdwa' in which case the username 
(sometimes overloaded as id) is required.

OK, that's a good point which I didn't realize. I would have expected that 
`auth:username:password:crdwa` would have resulted in ignoring 
`username:password`. Let me play with that to better understand it..


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...

2017-03-07 Thread afine
Github user afine commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/182#discussion_r104765953
  
--- Diff: src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 
---
@@ -841,9 +841,9 @@
 itself, ZooKeeper associates all the ids that correspond to a
 client with the clients connection. These ids are checked against
 the ACLs of znodes when a clients tries to access a node. ACLs are
-made up of pairs of (scheme:expression,
+made up of pairs of (scheme:id,
--- End diff --

I'm not sure if this is the best way to clarify here. As demonstrated below 
with the ip address example, the second field can be an "expression" that 
matches against ids. Although in the code we occasionally refer to the second 
term as an "id" (`ap.matches(authId.getId(), id.getId())` in 
`PrepRequestProcessor`) we do also refer to it as an "expression" in other 
places (`boolean matches(String id, String aclExpr)` in 
`AuthenticationProvider`).

I think continuing to refer to the second term as an "expression" and 
explaining exactly what an "expression" is may be clearer.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...

2017-03-07 Thread afine
Github user afine commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/182#discussion_r104766400
  
--- Diff: src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 
---
@@ -899,9 +899,16 @@
 single id, anyone, that represents
 anyone.
 
-auth doesn't
-use any id, represents any authenticated
-user.
+auth is a special
+scheme which ignores any provided ID and instead uses the current 
user,
+credentials, and scheme. Any ID (whether, 'user' like with SASL
--- End diff --

not sure the comma after whether is needed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...

2017-03-07 Thread afine
Github user afine commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/182#discussion_r104770508
  
--- Diff: src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 
---
@@ -899,9 +899,16 @@
 single id, anyone, that represents
 anyone.
 
-auth doesn't
-use any id, represents any authenticated
-user.
+auth is a special
+scheme which ignores any provided ID and instead uses the current 
user,
+credentials, and scheme. Any ID (whether, 'user' like with SASL
+authentication or 'user:password' like with DIGEST authentication) 
provided is ignored
+by the ZooKeeper server when persisting the ACL. However, the ID 
must be
+provided in the ACL because the ACL must match the form 
'scheme:id:perms'.
+This scheme is provided as a convenience as it is a common 
use-case for
+a client to create a znode and then restrict access to that znode 
to only that client.
--- End diff --

perhaps "only that user" would be clearer?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...

2017-03-08 Thread joshelser
Github user joshelser commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/182#discussion_r104986907
  
--- Diff: src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 
---
@@ -899,9 +899,16 @@
 single id, anyone, that represents
 anyone.
 
-auth doesn't
-use any id, represents any authenticated
-user.
+auth is a special
+scheme which ignores any provided ID and instead uses the current 
user,
+credentials, and scheme. Any ID (whether, 'user' like with SASL
--- End diff --

Agreed :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...

2017-03-08 Thread joshelser
Github user joshelser commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/182#discussion_r104986881
  
--- Diff: src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 
---
@@ -841,9 +841,9 @@
 itself, ZooKeeper associates all the ids that correspond to a
 client with the clients connection. These ids are checked against
 the ACLs of znodes when a clients tries to access a node. ACLs are
-made up of pairs of (scheme:expression,
+made up of pairs of (scheme:id,
--- End diff --

> I think continuing to refer to the second term as an "expression" and 
explaining exactly what an "expression" is may be clearer.

Ok, I can do that instead. I was trying to consolidate the use of "id" and 
"expression" in these different contexts to prevent more confusion (as you 
realized). Will try to come up with something better :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...

2017-03-08 Thread joshelser
Github user joshelser commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/182#discussion_r104988617
  
--- Diff: src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 
---
@@ -841,9 +841,9 @@
 itself, ZooKeeper associates all the ids that correspond to a
 client with the clients connection. These ids are checked against
 the ACLs of znodes when a clients tries to access a node. ACLs are
-made up of pairs of (scheme:expression,
+made up of pairs of (scheme:id,
--- End diff --

Oh, I see. ID is meant to be the complete `scheme:expression`. Need more 
coffee...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...

2017-03-08 Thread joshelser
Github user joshelser commented on a diff in the pull request:

https://github.com/apache/zookeeper/pull/182#discussion_r104987220
  
--- Diff: src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml 
---
@@ -899,9 +899,16 @@
 single id, anyone, that represents
 anyone.
 
-auth doesn't
-use any id, represents any authenticated
-user.
+auth is a special
+scheme which ignores any provided ID and instead uses the current 
user,
+credentials, and scheme. Any ID (whether, 'user' like with SASL
+authentication or 'user:password' like with DIGEST authentication) 
provided is ignored
+by the ZooKeeper server when persisting the ACL. However, the ID 
must be
+provided in the ACL because the ACL must match the form 
'scheme:id:perms'.
+This scheme is provided as a convenience as it is a common 
use-case for
+a client to create a znode and then restrict access to that znode 
to only that client.
--- End diff --

Avoiding the word "user" was intentional as a nod to some of the other auth 
schemes (e.g. the IP address one), but maybe that just creates more confusion 
than it's worth.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...

2017-03-08 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/zookeeper/pull/182


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---