You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by md...@apache.org on 2013/08/28 03:50:40 UTC

git commit: ACCUMULO-1478 Fixing login tokens.

Updated Branches:
  refs/heads/master 21f93e34b -> 12c676ae7


ACCUMULO-1478 Fixing login tokens.

Initialize the token properties to an empty map by default.
Added unit tests for using login tokens.


Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo
Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/12c676ae
Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/12c676ae
Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/12c676ae

Branch: refs/heads/master
Commit: 12c676ae7fdbfde4458fae9b9bf28475ab38f42e
Parents: 21f93e3
Author: Mike Drob <md...@mdrob.com>
Authored: Tue Aug 27 21:41:12 2013 -0400
Committer: Mike Drob <md...@mdrob.com>
Committed: Tue Aug 27 21:47:33 2013 -0400

----------------------------------------------------------------------
 .../apache/accumulo/core/util/shell/Shell.java  | 40 ++++-----
 .../core/util/shell/ShellOptionsJC.java         |  5 +-
 .../core/util/shell/ShellConfigTest.java        | 88 ++++++++++++++++++++
 3 files changed, 109 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/12c676ae/core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java b/core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java
index 2b8d96e..175d67e 100644
--- a/core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java
+++ b/core/src/main/java/org/apache/accumulo/core/util/shell/Shell.java
@@ -265,18 +265,23 @@ public class Shell extends ShellOptions {
     // process default parameters if unspecified
     try {
       boolean hasToken = (token != null);
-      boolean hasTokenOptions = loginOptions != null && !loginOptions.isEmpty();
-      
-      // Need either both a token and options, or neither, but not just one.
-      if (hasToken != hasTokenOptions) {
-        throw new ParameterException("Must supply either both or neither of '--tokenClass' and '--tokenProperty'");
-      }
+      boolean hasTokenOptions = !loginOptions.isEmpty();
       
       if (hasToken && password != null) {
         throw new ParameterException("Can not supply '--pass' option with '--tokenClass' option");
       }
       
-      if (hasToken && hasTokenOptions) {
+      Runtime.getRuntime().addShutdownHook(new Thread() {
+        @Override
+        public void run() {
+          reader.getTerminal().setEchoEnabled(true);
+        }
+      });
+
+      // Need either both a token and options, or neither, but not just one.
+      if (hasToken != hasTokenOptions) {
+        throw new ParameterException("Must supply either both or neither of '--tokenClass' and '--tokenProperty'");
+      } else if (hasToken) { // implied hasTokenOptions
         // Fully qualified name so we don't shadow java.util.Properties
         org.apache.accumulo.core.client.security.tokens.AuthenticationToken.Properties props;
         // and line wrap it because the package name is so long
@@ -284,21 +289,7 @@ public class Shell extends ShellOptions {
         
         props.putAllStrings(loginOptions);
         token.init(props);
-      }
-      
-      if (!options.isFake()) {
-        ZooReader zr = new ZooReader(instance.getZooKeepers(), instance.getZooKeepersSessionTimeOut());
-        DistributedTrace.enable(instance, zr, "shell", InetAddress.getLocalHost().getHostName());
-      }
-      
-      Runtime.getRuntime().addShutdownHook(new Thread() {
-        @Override
-        public void run() {
-          reader.getTerminal().setEchoEnabled(true);
-        }
-      });
-      
-      if (!hasToken) {
+      } else {
         if (password == null) {
           password = reader.readLine("Password: ", '*');
         }
@@ -311,6 +302,11 @@ public class Shell extends ShellOptions {
         }
       }
       
+      if (!options.isFake()) {
+        ZooReader zr = new ZooReader(instance.getZooKeepers(), instance.getZooKeepersSessionTimeOut());
+        DistributedTrace.enable(instance, zr, "shell", InetAddress.getLocalHost().getHostName());
+      }
+      
       this.setTableName("");
       this.principal = user;
       connector = instance.getConnector(this.principal, token);

http://git-wip-us.apache.org/repos/asf/accumulo/blob/12c676ae/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java b/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java
index 33b3eac..29dc4a7 100644
--- a/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java
+++ b/core/src/main/java/org/apache/accumulo/core/util/shell/ShellOptionsJC.java
@@ -20,6 +20,7 @@ import java.io.File;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.TreeMap;
 
 import org.apache.accumulo.core.client.security.tokens.AuthenticationToken;
 
@@ -37,7 +38,7 @@ public class ShellOptionsJC {
   @Parameter(names = {"-p", "--password"}, description = "password (prompt for password if this option is missing)")
   private String password;
   
-  private static class TokenConverter implements IStringConverter<AuthenticationToken> {
+  public static class TokenConverter implements IStringConverter<AuthenticationToken> {
     @Override
     public AuthenticationToken convert(String value) {
       try {
@@ -53,7 +54,7 @@ public class ShellOptionsJC {
   private AuthenticationToken authenticationToken;
   
   @DynamicParameter(names = {"-l", "--tokenProperty"}, description = "login properties in the format key=value. Reuse -l for each property")
-  private Map<String,String> tokenProperties;
+  private Map<String,String> tokenProperties = new TreeMap<String,String>();
   
   @Parameter(names = "--disable-tab-completion", description = "disables tab completion (for less overhead when scripting)")
   private boolean tabCompletionDisabled;

http://git-wip-us.apache.org/repos/asf/accumulo/blob/12c676ae/core/src/test/java/org/apache/accumulo/core/util/shell/ShellConfigTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/accumulo/core/util/shell/ShellConfigTest.java b/core/src/test/java/org/apache/accumulo/core/util/shell/ShellConfigTest.java
new file mode 100644
index 0000000..465e9c9
--- /dev/null
+++ b/core/src/test/java/org/apache/accumulo/core/util/shell/ShellConfigTest.java
@@ -0,0 +1,88 @@
+/*
+ * 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.accumulo.core.util.shell;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.FileDescriptor;
+import java.io.FileInputStream;
+import java.io.PrintStream;
+
+import jline.console.ConsoleReader;
+
+import org.apache.accumulo.core.client.security.tokens.PasswordToken;
+import org.apache.accumulo.core.util.shell.ShellTest.TestOutputStream;
+import org.apache.log4j.Level;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.beust.jcommander.ParameterException;
+
+public class ShellConfigTest {
+  TestOutputStream output;
+  Shell shell;
+  PrintStream out;
+  
+  @Before
+  public void setUp() throws Exception {
+    Shell.log.setLevel(Level.ERROR);
+    
+    out = System.out;
+    output = new TestOutputStream();
+    System.setOut(new PrintStream(output));
+    
+    shell = new Shell(new ConsoleReader(new FileInputStream(FileDescriptor.in), output));
+    shell.setLogErrorsToConsole();
+  }
+  
+  @After
+  public void teardown() throws Exception {
+    output.clear();
+    System.setOut(out);
+  }
+  
+  @Test
+  public void testHelp() {
+    assertTrue(shell.config("--help"));
+    assertTrue("Did not print usage", output.get().startsWith("Usage"));
+  }
+  
+  @Test
+  public void testBadArg() {
+    assertTrue(shell.config("--bogus"));
+    assertTrue("Did not print usage", output.get().startsWith("Usage"));
+  }
+  
+  @Test
+  public void testToken() {
+    assertTrue(shell.config("--fake", "-tc", PasswordToken.class.getCanonicalName()));
+    assertTrue(output.get().contains(ParameterException.class.getCanonicalName()));
+  }
+  
+  @Test
+  public void testTokenAndOption() {
+    assertFalse(shell.config("--fake", "-tc", PasswordToken.class.getCanonicalName(), "-l", "password=foo"));
+  }
+  
+  @Test
+  public void testTokenAndOptionAndPassword() {
+    assertTrue(shell.config("--fake", "-tc", PasswordToken.class.getCanonicalName(), "-l", "password=foo", "-p", "bar"));
+    assertTrue(output.get().contains(ParameterException.class.getCanonicalName()));
+  }
+}