[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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. ---