You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by joshelser <gi...@git.apache.org> on 2017/03/03 21:30:45 UTC
[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 <jo...@gmail.com>
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...
Posted by joshelser <gi...@git.apache.org>.
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, <emphasis>anyone</emphasis>, that represents
anyone.</para></listitem>
- <listitem><para><emphasis role="bold">auth</emphasis> doesn't
- use any id, represents any authenticated
- user.</para></listitem>
+ <listitem><para><emphasis role="bold">auth</emphasis> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...
Posted by joshelser <gi...@git.apache.org>.
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, <emphasis>anyone</emphasis>, that represents
anyone.</para></listitem>
- <listitem><para><emphasis role="bold">auth</emphasis> doesn't
- use any id, represents any authenticated
- user.</para></listitem>
+ <listitem><para><emphasis role="bold">auth</emphasis> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...
Posted by joshelser <gi...@git.apache.org>.
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, <emphasis>anyone</emphasis>, that represents
anyone.</para></listitem>
- <listitem><para><emphasis role="bold">auth</emphasis> doesn't
- use any id, represents any authenticated
- user.</para></listitem>
+ <listitem><para><emphasis role="bold">auth</emphasis> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper issue #182: ZOOKEEPER-2709 Clarify documentation around the "auth"...
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/zookeeper/pull/182
LMK what you think of the re-wording in 057cb18, @hanm. Tried to clarify things.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...
Posted by joshelser <gi...@git.apache.org>.
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 <emphasis>(scheme:expression,
+ made up of pairs of <emphasis>(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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper issue #182: ZOOKEEPER-2709 Clarify documentation around the "auth"...
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/zookeeper/pull/182
@hanm @afine let's try out 4d7b712 (didn't squash so it's easier to see what was changed -- sans the line-wrapping, sorry in advance).
I tried to consistently (and correctly) use the terms "id" and "expression" throughout. I hope it's more clear now :D
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...
Posted by afine <gi...@git.apache.org>.
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 <emphasis>(scheme:expression,
+ made up of pairs of <emphasis>(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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...
Posted by afine <gi...@git.apache.org>.
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, <emphasis>anyone</emphasis>, that represents
anyone.</para></listitem>
- <listitem><para><emphasis role="bold">auth</emphasis> doesn't
- use any id, represents any authenticated
- user.</para></listitem>
+ <listitem><para><emphasis role="bold">auth</emphasis> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper issue #182: ZOOKEEPER-2709 Clarify documentation around the "auth"...
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/182
Thanks for the quick update @joshelser ! The latest doc looks perfect minors what Abe pointed out. Do you mind to address Abe's comment regarding the id / expression clarification?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...
Posted by afine <gi...@git.apache.org>.
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, <emphasis>anyone</emphasis>, that represents
anyone.</para></listitem>
- <listitem><para><emphasis role="bold">auth</emphasis> doesn't
- use any id, represents any authenticated
- user.</para></listitem>
+ <listitem><para><emphasis role="bold">auth</emphasis> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...
Posted by joshelser <gi...@git.apache.org>.
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 <emphasis>(scheme:expression,
+ made up of pairs of <emphasis>(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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...
Posted by asfgit <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper issue #182: ZOOKEEPER-2709 Clarify documentation around the "auth"...
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/182
Thanks @joshelser! Latest update looks great.
Patch committed to master and branch-3.5.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #182: ZOOKEEPER-2709 Clarify documentation around the...
Posted by hanm <gi...@git.apache.org>.
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, <emphasis>anyone</emphasis>, that represents
anyone.</para></listitem>
- <listitem><para><emphasis role="bold">auth</emphasis> doesn't
- use any id, represents any authenticated
- user.</para></listitem>
+ <listitem><para><emphasis role="bold">auth</emphasis> 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---