[GitHub] zookeeper pull request #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user asfgit closed the pull request at: https://github.com/apache/zookeeper/pull/84 --- 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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user Randgalt commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/84#discussion_r88091830 --- Diff: src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java --- @@ -543,7 +546,7 @@ protected void pRequest2Txn(int type, long zxid, Request request, } nodeRecord = getRecordForPath(ZooDefs.CONFIG_NODE); -checkACL(zks, nodeRecord.acl, ZooDefs.Perms.WRITE, request.authInfo); +checkACL(zks, request.cnxn, nodeRecord.acl, ZooDefs.Perms.WRITE, request.authInfo, "/", null); --- End diff -- done --- 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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/84#discussion_r87957495 --- Diff: src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java --- @@ -543,7 +546,7 @@ protected void pRequest2Txn(int type, long zxid, Request request, } nodeRecord = getRecordForPath(ZooDefs.CONFIG_NODE); -checkACL(zks, nodeRecord.acl, ZooDefs.Perms.WRITE, request.authInfo); +checkACL(zks, request.cnxn, nodeRecord.acl, ZooDefs.Perms.WRITE, request.authInfo, "/", null); --- End diff -- `null` tells me that the method is not expecting a value from the caller in that position, which is better than an arbitrary value. I think it is fine to change it to `null`. --- 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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user fpj commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r87957002
--- Diff: src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml
---
@@ -1193,6 +1193,55 @@ authProvider.2=com.f.MyAuth2
only one will be used. Also all servers must have the same plugins
defined, otherwise clients using
the authentication schemes provided by the plugins will have problems
connecting to some servers.
+
+ Added in 3.6.0: An alternate
abstraction is available for pluggable
+authentication. It provides additional arguments.
+
+
+
+public abstract class ServerAuthenticationProvider implements
AuthenticationProvider {
--- End diff --
Ok.
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r87057041
--- Diff: src/java/test/org/apache/zookeeper/test/KeyAuthClientTest.java ---
@@ -0,0 +1,131 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.ACL;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.List;
+
+public class KeyAuthClientTest extends ClientBase {
+private static final Logger LOG =
LoggerFactory.getLogger(KeyAuthClientTest.class);
+static {
+
System.setProperty("zookeeper.authProvider.1","org.apache.zookeeper.server.auth.KeyAuthenticationProvider");
+}
+
+public void createNodePrintAcl(ZooKeeper zk, String path, String
testName) {
+ try {
+LOG.debug("KeyAuthenticationProvider Creating Test
Node:"+path+".\n");
+zk.create(path, null, Ids.CREATOR_ALL_ACL, CreateMode.PERSISTENT);
+List acls = zk.getACL(path, null);
+LOG.debug("Node: "+path+" Test:"+testName+" ACLs:");
+for (ACL acl : acls) {
+ LOG.debug(" "+acl.toString());
+}
+ } catch (Exception e) {
+ LOG.debug(" EXCEPTION THROWN", e);
+ }
+}
+
+public void testPreAuth() throws Exception {
+ZooKeeper zk = createClient();
+zk.addAuthInfo("key", "25".getBytes());
+try {
+createNodePrintAcl(zk, "/pre", "testPreAuth");
+zk.setACL("/", Ids.CREATOR_ALL_ACL, -1);
+zk.getChildren("/", false);
+zk.create("/abc", null, Ids.CREATOR_ALL_ACL,
CreateMode.PERSISTENT);
+zk.setData("/abc", "testData1".getBytes(), -1);
+zk.create("/key", null, Ids.CREATOR_ALL_ACL,
CreateMode.PERSISTENT);
+zk.setData("/key", "5".getBytes(), -1);
+Thread.sleep(1000);
--- End diff --
So, a question for a future ticket (IMHO). :cactus:
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r87054533
--- Diff: src/java/test/org/apache/zookeeper/test/KeyAuthClientTest.java ---
@@ -0,0 +1,131 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.ACL;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.List;
+
+public class KeyAuthClientTest extends ClientBase {
+private static final Logger LOG =
LoggerFactory.getLogger(KeyAuthClientTest.class);
+static {
+
System.setProperty("zookeeper.authProvider.1","org.apache.zookeeper.server.auth.KeyAuthenticationProvider");
+}
+
+public void createNodePrintAcl(ZooKeeper zk, String path, String
testName) {
+ try {
+LOG.debug("KeyAuthenticationProvider Creating Test
Node:"+path+".\n");
+zk.create(path, null, Ids.CREATOR_ALL_ACL, CreateMode.PERSISTENT);
+List acls = zk.getACL(path, null);
+LOG.debug("Node: "+path+" Test:"+testName+" ACLs:");
+for (ACL acl : acls) {
+ LOG.debug(" "+acl.toString());
+}
+ } catch (Exception e) {
+ LOG.debug(" EXCEPTION THROWN", e);
+ }
+}
+
+public void testPreAuth() throws Exception {
+ZooKeeper zk = createClient();
+zk.addAuthInfo("key", "25".getBytes());
+try {
+createNodePrintAcl(zk, "/pre", "testPreAuth");
+zk.setACL("/", Ids.CREATOR_ALL_ACL, -1);
+zk.getChildren("/", false);
+zk.create("/abc", null, Ids.CREATOR_ALL_ACL,
CreateMode.PERSISTENT);
+zk.setData("/abc", "testData1".getBytes(), -1);
+zk.create("/key", null, Ids.CREATOR_ALL_ACL,
CreateMode.PERSISTENT);
+zk.setData("/key", "5".getBytes(), -1);
+Thread.sleep(1000);
--- End diff --
These types of sleeps are all over the ZK tests (and Curator for that
matter).
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r87052291
--- Diff: src/java/test/org/apache/zookeeper/test/KeyAuthClientTest.java ---
@@ -0,0 +1,131 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.ACL;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.List;
+
+public class KeyAuthClientTest extends ClientBase {
+private static final Logger LOG =
LoggerFactory.getLogger(KeyAuthClientTest.class);
+static {
+
System.setProperty("zookeeper.authProvider.1","org.apache.zookeeper.server.auth.KeyAuthenticationProvider");
+}
+
+public void createNodePrintAcl(ZooKeeper zk, String path, String
testName) {
+ try {
+LOG.debug("KeyAuthenticationProvider Creating Test
Node:"+path+".\n");
+zk.create(path, null, Ids.CREATOR_ALL_ACL, CreateMode.PERSISTENT);
+List acls = zk.getACL(path, null);
+LOG.debug("Node: "+path+" Test:"+testName+" ACLs:");
+for (ACL acl : acls) {
+ LOG.debug(" "+acl.toString());
+}
+ } catch (Exception e) {
+ LOG.debug(" EXCEPTION THROWN", e);
+ }
+}
+
+public void testPreAuth() throws Exception {
+ZooKeeper zk = createClient();
+zk.addAuthInfo("key", "25".getBytes());
+try {
+createNodePrintAcl(zk, "/pre", "testPreAuth");
+zk.setACL("/", Ids.CREATOR_ALL_ACL, -1);
+zk.getChildren("/", false);
+zk.create("/abc", null, Ids.CREATOR_ALL_ACL,
CreateMode.PERSISTENT);
+zk.setData("/abc", "testData1".getBytes(), -1);
+zk.create("/key", null, Ids.CREATOR_ALL_ACL,
CreateMode.PERSISTENT);
+zk.setData("/key", "5".getBytes(), -1);
+Thread.sleep(1000);
--- End diff --
Only thing I can suppose (and **please** correct me if I am saying
something **utterly stupid**) is that this ``sleep`` was inserted in the hopes
of giving some time to ZK commit the changes. Naive but yet (shrug)...
Well, I guess we can either remove it or replace it with a
``zk.getData("/key");``, right?
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r86868035
--- Diff: src/java/test/org/apache/zookeeper/test/KeyAuthClientTest.java ---
@@ -0,0 +1,131 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.ACL;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.List;
+
+public class KeyAuthClientTest extends ClientBase {
+private static final Logger LOG =
LoggerFactory.getLogger(KeyAuthClientTest.class);
+static {
+
System.setProperty("zookeeper.authProvider.1","org.apache.zookeeper.server.auth.KeyAuthenticationProvider");
+}
+
+public void createNodePrintAcl(ZooKeeper zk, String path, String
testName) {
+ try {
+LOG.debug("KeyAuthenticationProvider Creating Test
Node:"+path+".\n");
+zk.create(path, null, Ids.CREATOR_ALL_ACL, CreateMode.PERSISTENT);
+List acls = zk.getACL(path, null);
+LOG.debug("Node: "+path+" Test:"+testName+" ACLs:");
+for (ACL acl : acls) {
+ LOG.debug(" "+acl.toString());
+}
+ } catch (Exception e) {
+ LOG.debug(" EXCEPTION THROWN", e);
+ }
+}
+
+public void testPreAuth() throws Exception {
+ZooKeeper zk = createClient();
+zk.addAuthInfo("key", "25".getBytes());
+try {
+createNodePrintAcl(zk, "/pre", "testPreAuth");
+zk.setACL("/", Ids.CREATOR_ALL_ACL, -1);
+zk.getChildren("/", false);
+zk.create("/abc", null, Ids.CREATOR_ALL_ACL,
CreateMode.PERSISTENT);
+zk.setData("/abc", "testData1".getBytes(), -1);
+zk.create("/key", null, Ids.CREATOR_ALL_ACL,
CreateMode.PERSISTENT);
+zk.setData("/key", "5".getBytes(), -1);
+Thread.sleep(1000);
--- End diff --
Me neither - I didn't write this code.
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user lvfangmin commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r86857366
--- Diff:
src/java/main/org/apache/zookeeper/server/auth/KeyAuthenticationProvider.java
---
@@ -0,0 +1,139 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.server.auth;
+
+import java.io.UnsupportedEncodingException;
+import java.util.List;
+
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.NoNodeException;
+import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.data.Id;
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperServer;
+import org.apache.zookeeper.server.ZKDatabase;
+import org.apache.zookeeper.data.Stat;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/*
+ * This class is a sample implementation of being passed the
ZooKeeperServer
+ * handle in the constructor, and reading data from zknodes to
authenticate.
+ * At a minimum, a real Auth provider would need to override validate() to
+ * e.g. perform certificate validation of auth based a public key.
+ *
+ * See the "Pluggable ZooKeeper authentication" section of the
+ * "Zookeeper Programmer's Guide" for general details of implementing an
+ * authentication plugin. e.g.
+ *
http://zookeeper.apache.org/doc/trunk/zookeeperProgrammers.html#sc_ZooKeeperPluggableAuthentication
+ *
+ * This class looks for a numeric "key" under the /key node.
+ * Authorizaton is granted if the user passes in as authorization a number
+ * which is a multiple of the key value, i.e.
+ * (auth % key) == 0
+ * In a real implementation, you might do something like storing a public
+ * key in /key, and using it to verify that auth tokens passed in were
signed
+ * by the corresponding private key.
+ *
+ * When the node /key does not exist, any auth token is accepted, so that
+ * bootstrapping may occur.
+ *
+ */
+public class KeyAuthenticationProvider extends
ServerAuthenticationProvider {
+private static final Logger LOG =
LoggerFactory.getLogger(KeyAuthenticationProvider.class);
+
+public String getScheme() {
+return "key";
+}
+
+public byte[] getKey(ZooKeeperServer zks) {
--- End diff --
Do we need this to be public? I only saw it's being called in
handleAuthentication.
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user lvfangmin commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/84#discussion_r86823972 --- Diff: src/java/main/org/apache/zookeeper/server/auth/AuthenticationProvider.java --- @@ -20,6 +20,9 @@ import org.apache.zookeeper.KeeperException; import org.apache.zookeeper.server.ServerCnxn; +import org.apache.zookeeper.server.ZooKeeperServer; + +import java.util.List; --- End diff -- These imports are not being used, get rid of 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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user lvfangmin commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r86824520
--- Diff:
src/java/main/org/apache/zookeeper/server/auth/KeyAuthenticationProvider.java
---
@@ -0,0 +1,139 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.server.auth;
+
+import java.io.UnsupportedEncodingException;
+import java.util.List;
+
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.NoNodeException;
+import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.data.Id;
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperServer;
+import org.apache.zookeeper.server.ZKDatabase;
+import org.apache.zookeeper.data.Stat;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/*
+ * This class is a sample implementation of being passed the
ZooKeeperServer
+ * handle in the constructor, and reading data from zknodes to
authenticate.
+ * At a minimum, a real Auth provider would need to override validate() to
+ * e.g. perform certificate validation of auth based a public key.
+ *
+ * See the "Pluggable ZooKeeper authentication" section of the
+ * "Zookeeper Programmer's Guide" for general details of implementing an
+ * authentication plugin. e.g.
+ *
http://zookeeper.apache.org/doc/trunk/zookeeperProgrammers.html#sc_ZooKeeperPluggableAuthentication
+ *
+ * This class looks for a numeric "key" under the /key node.
+ * Authorizaton is granted if the user passes in as authorization a number
+ * which is a multiple of the key value, i.e.
+ * (auth % key) == 0
+ * In a real implementation, you might do something like storing a public
+ * key in /key, and using it to verify that auth tokens passed in were
signed
+ * by the corresponding private key.
+ *
+ * When the node /key does not exist, any auth token is accepted, so that
+ * bootstrapping may occur.
+ *
+ */
+public class KeyAuthenticationProvider extends
ServerAuthenticationProvider {
+private static final Logger LOG =
LoggerFactory.getLogger(KeyAuthenticationProvider.class);
+
+public String getScheme() {
+return "key";
+}
+
+public byte[] getKey(ZooKeeperServer zks) {
+ZKDatabase db = zks.getZKDatabase();
+if (db != null) {
+try {
+Stat stat = new Stat();
+return db.getData("/key", stat, null);
+} catch (NoNodeException e) {
+// ignore
+}
+}
+return null;
+}
+
+public boolean validate(byte[] key, byte[] auth) {
+// perform arbitrary function (auth is a multiple of key)
+try {
+String keyStr = new String(key, "UTF-8");
+String authStr = new String(auth, "UTF-8");
+int keyVal = Integer.parseInt(keyStr);
+int authVal = Integer.parseInt(authStr);
+if (keyVal!=0 && ((authVal % keyVal) != 0)) {
+ return false;
+}
+} catch (NumberFormatException | UnsupportedEncodingException nfe)
{
+ return false;
--- End diff --
Maybe it's nice to log error here for debugging purpose.
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user lvfangmin commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r86824750
--- Diff:
src/java/main/org/apache/zookeeper/server/auth/KeyAuthenticationProvider.java
---
@@ -0,0 +1,139 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.server.auth;
+
+import java.io.UnsupportedEncodingException;
+import java.util.List;
+
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.NoNodeException;
+import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.data.Id;
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperServer;
+import org.apache.zookeeper.server.ZKDatabase;
+import org.apache.zookeeper.data.Stat;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/*
+ * This class is a sample implementation of being passed the
ZooKeeperServer
+ * handle in the constructor, and reading data from zknodes to
authenticate.
+ * At a minimum, a real Auth provider would need to override validate() to
+ * e.g. perform certificate validation of auth based a public key.
+ *
+ * See the "Pluggable ZooKeeper authentication" section of the
+ * "Zookeeper Programmer's Guide" for general details of implementing an
+ * authentication plugin. e.g.
+ *
http://zookeeper.apache.org/doc/trunk/zookeeperProgrammers.html#sc_ZooKeeperPluggableAuthentication
+ *
+ * This class looks for a numeric "key" under the /key node.
+ * Authorizaton is granted if the user passes in as authorization a number
+ * which is a multiple of the key value, i.e.
+ * (auth % key) == 0
+ * In a real implementation, you might do something like storing a public
+ * key in /key, and using it to verify that auth tokens passed in were
signed
+ * by the corresponding private key.
+ *
+ * When the node /key does not exist, any auth token is accepted, so that
+ * bootstrapping may occur.
+ *
+ */
+public class KeyAuthenticationProvider extends
ServerAuthenticationProvider {
+private static final Logger LOG =
LoggerFactory.getLogger(KeyAuthenticationProvider.class);
+
+public String getScheme() {
+return "key";
+}
+
+public byte[] getKey(ZooKeeperServer zks) {
+ZKDatabase db = zks.getZKDatabase();
+if (db != null) {
+try {
+Stat stat = new Stat();
+return db.getData("/key", stat, null);
+} catch (NoNodeException e) {
+// ignore
+}
+}
+return null;
+}
+
+public boolean validate(byte[] key, byte[] auth) {
+// perform arbitrary function (auth is a multiple of key)
+try {
+String keyStr = new String(key, "UTF-8");
+String authStr = new String(auth, "UTF-8");
+int keyVal = Integer.parseInt(keyStr);
+int authVal = Integer.parseInt(authStr);
+if (keyVal!=0 && ((authVal % keyVal) != 0)) {
+ return false;
+}
+} catch (NumberFormatException | UnsupportedEncodingException nfe)
{
+ return false;
+}
+return true;
+}
+
+@Override
+public KeeperException.Code handleAuthentication(ZooKeeperServer zks,
ServerCnxn cnxn, byte[] authData) {
+byte[] key = getKey(zks);
+String authStr = "";
+String keyStr = "";
+try {
+ authStr = new String(authData, "UTF-8");
+} catch (Exception e) {
+ // empty authData
--- End diff --
Same here, better to log the exception.
---
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
[GitHub] zookeeper pull request #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r85649050
--- Diff:
src/java/main/org/apache/zookeeper/server/auth/WrappedAuthenticationProvider.java
---
@@ -0,0 +1,74 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.server.auth;
+
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperServer;
+
+import java.util.List;
+
+/**
+ * Provides backwards compatibility between older {@link
AuthenticationProvider}
+ * implementations and the new {@link ServerAuthenticationProvider}
interface.
+ */
+class WrappedAuthenticationProvider extends ServerAuthenticationProvider {
+private final AuthenticationProvider implementation;
+
+static ServerAuthenticationProvider wrap(AuthenticationProvider
provider) {
+return (provider instanceof ServerAuthenticationProvider) ?
(ServerAuthenticationProvider)provider
+: new WrappedAuthenticationProvider(provider);
+}
+
+private WrappedAuthenticationProvider(AuthenticationProvider
implementation) {
+this.implementation = implementation;
+}
+
+@Override
+public KeeperException.Code handleAuthentication(ZooKeeperServer zks,
ServerCnxn cnxn, byte[] authData) {
+return implementation.handleAuthentication(cnxn, authData);
+}
+
+/**
+ * {@inheritDoc}
--- End diff --
Yes, you are correct.
>Do we have to do it for this method only or for others as well?
Well, it can be done anywhere. I only need it here.
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r85649028
--- Diff:
src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java ---
@@ -31,7 +31,15 @@
private static boolean initialized = false;
private static HashMap
authenticationProviders =
-new HashMap();
+new HashMap<>();
+
+//VisibleForTesting
+public static void reset() {
--- End diff --
I use it in the external code that I'm writing that uses this feature. I
can see it being useful to others as well. Without it it's very difficult to
write unit tests using custom auth plugins.
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r85649005
--- Diff: src/java/test/org/apache/zookeeper/test/KeyAuthClientTest.java ---
@@ -0,0 +1,131 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.ACL;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.List;
+
+public class KeyAuthClientTest extends ClientBase {
+private static final Logger LOG =
LoggerFactory.getLogger(KeyAuthClientTest.class);
+static {
+
System.setProperty("zookeeper.authProvider.1","org.apache.zookeeper.server.auth.KeyAuthenticationProvider");
+}
+
+public void createNodePrintAcl(ZooKeeper zk, String path, String
testName) {
+ try {
+LOG.debug("KeyAuthenticationProvider Creating Test
Node:"+path+".\n");
+zk.create(path, null, Ids.CREATOR_ALL_ACL, CreateMode.PERSISTENT);
+List acls = zk.getACL(path, null);
+LOG.debug("Node: "+path+" Test:"+testName+" ACLs:");
--- End diff --
I didn't write this code - only copied it. But, I'll reformat the file to
style.
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user Randgalt commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r85648967
--- Diff: src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml
---
@@ -1193,6 +1193,55 @@ authProvider.2=com.f.MyAuth2
only one will be used. Also all servers must have the same plugins
defined, otherwise clients using
the authentication schemes provided by the plugins will have problems
connecting to some servers.
+
+ Added in 3.6.0: An alternate
abstraction is available for pluggable
+authentication. It provides additional arguments.
+
+
+
+public abstract class ServerAuthenticationProvider implements
AuthenticationProvider {
--- End diff --
I'm copying the other parts of the zookeeperProgrammers.xml here. I built
the docs and it looks good.
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user fpj commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r85642051
--- Diff: src/java/test/org/apache/zookeeper/test/KeyAuthClientTest.java ---
@@ -0,0 +1,131 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.ACL;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.List;
+
+public class KeyAuthClientTest extends ClientBase {
+private static final Logger LOG =
LoggerFactory.getLogger(KeyAuthClientTest.class);
+static {
+
System.setProperty("zookeeper.authProvider.1","org.apache.zookeeper.server.auth.KeyAuthenticationProvider");
+}
+
+public void createNodePrintAcl(ZooKeeper zk, String path, String
testName) {
+ try {
+LOG.debug("KeyAuthenticationProvider Creating Test
Node:"+path+".\n");
+zk.create(path, null, Ids.CREATOR_ALL_ACL, CreateMode.PERSISTENT);
+List acls = zk.getACL(path, null);
+LOG.debug("Node: "+path+" Test:"+testName+" ACLs:");
+for (ACL acl : acls) {
+ LOG.debug(" "+acl.toString());
--- End diff --
Same here, space around symbols, please.
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user fpj commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r85642202
--- Diff: src/java/test/org/apache/zookeeper/test/KeyAuthClientTest.java ---
@@ -0,0 +1,131 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.ACL;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.List;
+
+public class KeyAuthClientTest extends ClientBase {
+private static final Logger LOG =
LoggerFactory.getLogger(KeyAuthClientTest.class);
+static {
+
System.setProperty("zookeeper.authProvider.1","org.apache.zookeeper.server.auth.KeyAuthenticationProvider");
+}
+
+public void createNodePrintAcl(ZooKeeper zk, String path, String
testName) {
+ try {
+LOG.debug("KeyAuthenticationProvider Creating Test
Node:"+path+".\n");
+zk.create(path, null, Ids.CREATOR_ALL_ACL, CreateMode.PERSISTENT);
+List acls = zk.getACL(path, null);
+LOG.debug("Node: "+path+" Test:"+testName+" ACLs:");
+for (ACL acl : acls) {
+ LOG.debug(" "+acl.toString());
+}
+ } catch (Exception e) {
+ LOG.debug(" EXCEPTION THROWN", e);
+ }
+}
+
+public void testPreAuth() throws Exception {
+ZooKeeper zk = createClient();
+zk.addAuthInfo("key", "25".getBytes());
+try {
+createNodePrintAcl(zk, "/pre", "testPreAuth");
+zk.setACL("/", Ids.CREATOR_ALL_ACL, -1);
+zk.getChildren("/", false);
+zk.create("/abc", null, Ids.CREATOR_ALL_ACL,
CreateMode.PERSISTENT);
+zk.setData("/abc", "testData1".getBytes(), -1);
+zk.create("/key", null, Ids.CREATOR_ALL_ACL,
CreateMode.PERSISTENT);
+zk.setData("/key", "5".getBytes(), -1);
+Thread.sleep(1000);
--- End diff --
I'm not sure why we need this `sleep` here.
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user fpj commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r85642282
--- Diff:
src/java/main/org/apache/zookeeper/server/auth/ServerAuthenticationProvider.java
---
@@ -0,0 +1,79 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.server.auth;
+
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperServer;
+
+import java.util.List;
+
+/**
+ * A variation on {@link AuthenticationProvider} that provides additional
+ * parameters for more detailed authentication
+ */
+public abstract class ServerAuthenticationProvider implements
AuthenticationProvider {
+/**
+ * This method is called when a client passes authentication data for
this
+ * scheme. The authData is directly from the authentication packet. The
+ * implementor may attach new ids to the authInfo field of cnxn or may
use
+ * cnxn to send packets back to the client.
+ *
+ * @param cnxn
+ *the cnxn that received the authentication
information.
+ * @param authData
+ *the authentication data received.
+ * @return indication of success or failure
+ */
+public abstract KeeperException.Code
handleAuthentication(ZooKeeperServer zks, ServerCnxn cnxn, byte authData[]);
+
+/**
+ * This method is called to see if the given id matches the given id
+ * expression in the ACL. This allows schemes to use application
specific
+ * wild cards.
+ *
+ * @param zks
+ *the ZooKeeper server instance
+ * @param cnxn
+ *the active server connection being authenticated
+ * @param path
+ *the path of the operation being authenticated
+ * @param id
+ *the id to check.
+ * @param aclExpr
+ *the expression to match ids against.
+ * @param perm
+ *the permission value being authenticated
+ * @param setAcls
+ *for set ACL operations, the list of ACLs being set.
Otherwise null.
+ * @return true if the arguments can be matched by the expression.
+ */
+public abstract boolean matches(ZooKeeperServer zks, ServerCnxn cnxn,
String path, String id, String aclExpr, int perm, List setAcls);
--- End diff --
Can we break this line, please?
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user fpj commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r85642270
--- Diff:
src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java ---
@@ -31,7 +31,15 @@
private static boolean initialized = false;
private static HashMap
authenticationProviders =
-new HashMap();
+new HashMap<>();
+
+//VisibleForTesting
+public static void reset() {
--- End diff --
Is this used anywhere?
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user fpj commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r85641947
--- Diff: src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml
---
@@ -1193,6 +1193,55 @@ authProvider.2=com.f.MyAuth2
only one will be used. Also all servers must have the same plugins
defined, otherwise clients using
the authentication schemes provided by the plugins will have problems
connecting to some servers.
+
+ Added in 3.6.0: An alternate
abstraction is available for pluggable
+authentication. It provides additional arguments.
+
+
+
+public abstract class ServerAuthenticationProvider implements
AuthenticationProvider {
--- End diff --
Indentation? Not sure if it matters within the listing block.
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user fpj commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r85642186
--- Diff: src/java/test/org/apache/zookeeper/test/KeyAuthClientTest.java ---
@@ -0,0 +1,131 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.ACL;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.List;
+
+public class KeyAuthClientTest extends ClientBase {
+private static final Logger LOG =
LoggerFactory.getLogger(KeyAuthClientTest.class);
+static {
+
System.setProperty("zookeeper.authProvider.1","org.apache.zookeeper.server.auth.KeyAuthenticationProvider");
+}
+
+public void createNodePrintAcl(ZooKeeper zk, String path, String
testName) {
+ try {
+LOG.debug("KeyAuthenticationProvider Creating Test
Node:"+path+".\n");
+zk.create(path, null, Ids.CREATOR_ALL_ACL, CreateMode.PERSISTENT);
+List acls = zk.getACL(path, null);
+LOG.debug("Node: "+path+" Test:"+testName+" ACLs:");
+for (ACL acl : acls) {
+ LOG.debug(" "+acl.toString());
+}
+ } catch (Exception e) {
+ LOG.debug(" EXCEPTION THROWN", e);
+ }
+}
+
+public void testPreAuth() throws Exception {
+ZooKeeper zk = createClient();
+zk.addAuthInfo("key", "25".getBytes());
+try {
+createNodePrintAcl(zk, "/pre", "testPreAuth");
+zk.setACL("/", Ids.CREATOR_ALL_ACL, -1);
+zk.getChildren("/", false);
+zk.create("/abc", null, Ids.CREATOR_ALL_ACL,
CreateMode.PERSISTENT);
+zk.setData("/abc", "testData1".getBytes(), -1);
+zk.create("/key", null, Ids.CREATOR_ALL_ACL,
CreateMode.PERSISTENT);
+zk.setData("/key", "5".getBytes(), -1);
+Thread.sleep(1000);
+} catch (KeeperException e) {
+Assert.fail("test failed :" + e);
+}
+finally {
+zk.close();
+}
+}
+
+public void testMissingAuth() throws Exception {
+ZooKeeper zk = createClient();
+try {
+zk.getData("/abc", false, null);
+Assert.fail("Should not be able to get data");
+} catch (KeeperException correct) {
+// correct
+}
+try {
+zk.setData("/abc", "testData2".getBytes(), -1);
+Assert.fail("Should not be able to set data");
+} catch (KeeperException correct) {
+// correct
+} finally {
+zk.close();
+}
+}
+
+public void testValidAuth() throws Exception {
+ZooKeeper zk = createClient();
+// any multiple of 5 will do...
+zk.addAuthInfo("key", "25".getBytes());
+try {
+createNodePrintAcl(zk, "/valid", "testValidAuth");
+zk.getData("/abc", false, null);
+zk.setData("/abc", "testData3".getBytes(), -1);
+} catch (KeeperException.AuthFailedException e) {
+Assert.fail("test failed :" + e);
+} finally {
+zk.close();
+}
+}
+
+public void testValidAuth2() throws Exception {
+ZooKeeper zk = createClient();
+// any multiple of 5 will do...
+zk.addAuthInfo("key", "125".getBytes());
+try {
+createNodePrintAcl(zk, "/valid2", "testValidAuth2");
+zk.getData("/abc", false, null);
+zk.setData("/abc", "testData3".getBytes(), -1);
+} cat
[GitHub] zookeeper pull request #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/84#discussion_r85641968 --- Diff: src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java --- @@ -543,7 +546,7 @@ protected void pRequest2Txn(int type, long zxid, Request request, } nodeRecord = getRecordForPath(ZooDefs.CONFIG_NODE); -checkACL(zks, nodeRecord.acl, ZooDefs.Perms.WRITE, request.authInfo); +checkACL(zks, request.cnxn, nodeRecord.acl, ZooDefs.Perms.WRITE, request.authInfo, "/", null); --- End diff -- Why is the path `"/"` here? --- 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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user fpj commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/84#discussion_r85641975 --- Diff: src/java/main/org/apache/zookeeper/server/ServerCnxn.java --- @@ -51,7 +47,7 @@ final public static Object me = new Object(); private static final Logger LOG = LoggerFactory.getLogger(ServerCnxn.class); -protected ArrayList authInfo = new ArrayList(); +private Set authInfo = Collections.newSetFromMap(new ConcurrentHashMap()); --- End diff -- Ok. --- 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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user fpj commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r85642389
--- Diff:
src/java/main/org/apache/zookeeper/server/auth/WrappedAuthenticationProvider.java
---
@@ -0,0 +1,74 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.server.auth;
+
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ZooKeeperServer;
+
+import java.util.List;
+
+/**
+ * Provides backwards compatibility between older {@link
AuthenticationProvider}
+ * implementations and the new {@link ServerAuthenticationProvider}
interface.
+ */
+class WrappedAuthenticationProvider extends ServerAuthenticationProvider {
+private final AuthenticationProvider implementation;
+
+static ServerAuthenticationProvider wrap(AuthenticationProvider
provider) {
+return (provider instanceof ServerAuthenticationProvider) ?
(ServerAuthenticationProvider)provider
+: new WrappedAuthenticationProvider(provider);
+}
+
+private WrappedAuthenticationProvider(AuthenticationProvider
implementation) {
+this.implementation = implementation;
+}
+
+@Override
+public KeeperException.Code handleAuthentication(ZooKeeperServer zks,
ServerCnxn cnxn, byte[] authData) {
+return implementation.handleAuthentication(cnxn, authData);
+}
+
+/**
+ * {@inheritDoc}
--- End diff --
I've never used this before and from the name it implies that it will copy
the doc blurb from the super class when running the javadoc tool. Is this
correct? Do we have to do it for this method only or for others as well?
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
Github user fpj commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/84#discussion_r85642046
--- Diff: src/java/test/org/apache/zookeeper/test/KeyAuthClientTest.java ---
@@ -0,0 +1,131 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.ACL;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.List;
+
+public class KeyAuthClientTest extends ClientBase {
+private static final Logger LOG =
LoggerFactory.getLogger(KeyAuthClientTest.class);
+static {
+
System.setProperty("zookeeper.authProvider.1","org.apache.zookeeper.server.auth.KeyAuthenticationProvider");
+}
+
+public void createNodePrintAcl(ZooKeeper zk, String path, String
testName) {
+ try {
+LOG.debug("KeyAuthenticationProvider Creating Test
Node:"+path+".\n");
+zk.create(path, null, Ids.CREATOR_ALL_ACL, CreateMode.PERSISTENT);
+List acls = zk.getACL(path, null);
+LOG.debug("Node: "+path+" Test:"+testName+" ACLs:");
--- End diff --
space around symbols, please?
---
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 #84: [ZOOKEEPER-1525] Plumb ZooKeeperServer object in...
GitHub user Randgalt opened a pull request: https://github.com/apache/zookeeper/pull/84 [ZOOKEEPER-1525] Plumb ZooKeeperServer object into auth plugins Based on patch work from https://issues.apache.org/jira/browse/ZOOKEEPER-1525 Created ServerAuthenticationProvider which has a method to accept the ZooKeeper server so that auth can be done using values in the ZK database. As this is a new interface, existing implementations aren't affected helping backward compatibility You can merge this pull request into a Git repository by running: $ git pull https://github.com/Randgalt/zookeeper ZOOKEEPER-1525 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/84.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 #84 commit 4de95483cb624217e2f3cd63872e23d60f28749b Author: randgalt Date: 2016-10-04T15:23:30Z Based on patch work from https://issues.apache.org/jira/browse/ZOOKEEPER-1525 Created ServerAuthenticationProvider which has a method to accept the ZooKeeper server so that auth can be done using values in the ZK database. As this is a new interface, existing implementations aren't affected helping backward compatibility --- 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. ---
