ACCUMULO-2733 adds thrift deserialization to Credentials
Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/6742627f Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/6742627f Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/6742627f Branch: refs/heads/master Commit: 6742627f20ad58e8e0737baddde952d9d7edba87 Parents: 90377b6 Author: Sean Busbey <bus...@cloudera.com> Authored: Thu Apr 24 23:44:49 2014 -0500 Committer: Sean Busbey <bus...@cloudera.com> Committed: Fri Apr 25 10:32:13 2014 -0500 ---------------------------------------------------------------------- .../accumulo/core/security/Credentials.java | 9 +++++++ .../accumulo/core/security/CredentialsTest.java | 8 ++++++ .../server/client/ClientServiceHandler.java | 8 +++--- .../server/security/SecurityOperation.java | 28 ++++++-------------- 4 files changed, 28 insertions(+), 25 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/6742627f/core/src/main/java/org/apache/accumulo/core/security/Credentials.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/security/Credentials.java b/core/src/main/java/org/apache/accumulo/core/security/Credentials.java index 5afc6e8..9f8b1be 100644 --- a/core/src/main/java/org/apache/accumulo/core/security/Credentials.java +++ b/core/src/main/java/org/apache/accumulo/core/security/Credentials.java @@ -91,6 +91,15 @@ public class Credentials { } /** + * Converts a given thrift object to our internal Credentials representation. + * @param serialized a Thrift encoded set of credentials + * @return a new Credentials instance; destroy the token when you're done. + */ + public static Credentials fromThrift(TCredentials serialized) { + return new Credentials(serialized.getPrincipal(), AuthenticationTokenSerializer.deserialize(serialized.getTokenClassName(), serialized.getToken())); + } + + /** * Converts the current object to a serialized form. The object returned from this contains a non-destroyable version of the {@link AuthenticationToken}, so * references to it should be tightly controlled. * http://git-wip-us.apache.org/repos/asf/accumulo/blob/6742627f/core/src/test/java/org/apache/accumulo/core/security/CredentialsTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/accumulo/core/security/CredentialsTest.java b/core/src/test/java/org/apache/accumulo/core/security/CredentialsTest.java index 4f8079e..b925e4a 100644 --- a/core/src/test/java/org/apache/accumulo/core/security/CredentialsTest.java +++ b/core/src/test/java/org/apache/accumulo/core/security/CredentialsTest.java @@ -62,6 +62,14 @@ public class CredentialsTest { assertTrue(AccumuloSecurityException.class.cast(e.getCause()).getSecurityErrorCode().equals(SecurityErrorCode.TOKEN_EXPIRED)); } } + + @Test + public void roundtripThrift() throws DestroyFailedException { + Credentials creds = new Credentials("test", new PasswordToken("testing")); + TCredentials tCreds = creds.toThrift(new MockInstance()); + Credentials roundtrip = Credentials.fromThrift(tCreds); + assertEquals("Roundtrip through thirft changed credentials equality", creds, roundtrip); + } @Test public void testMockConnector() throws AccumuloException, DestroyFailedException, AccumuloSecurityException { http://git-wip-us.apache.org/repos/asf/accumulo/blob/6742627f/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java ---------------------------------------------------------------------- diff --git a/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java b/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java index 3571d7f..e6739f8 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java +++ b/server/base/src/main/java/org/apache/accumulo/server/client/ClientServiceHandler.java @@ -46,7 +46,6 @@ import org.apache.accumulo.core.client.impl.thrift.TableOperationExceptionType; import org.apache.accumulo.core.client.impl.thrift.ThriftSecurityException; import org.apache.accumulo.core.client.impl.thrift.ThriftTableOperationException; import org.apache.accumulo.core.client.security.tokens.AuthenticationToken; -import org.apache.accumulo.core.client.security.tokens.AuthenticationToken.AuthenticationTokenSerializer; import org.apache.accumulo.core.client.security.tokens.PasswordToken; import org.apache.accumulo.core.conf.AccumuloConfiguration; import org.apache.accumulo.core.conf.Property; @@ -282,8 +281,7 @@ public class ClientServiceHandler implements ClientService.Iface { return transactionWatcher.run(Constants.BULK_ARBITRATOR_TYPE, tid, new Callable<List<String>>() { @Override public List<String> call() throws Exception { - return BulkImporter.bulkLoad(new ServerConfiguration(instance).getConfiguration(), instance, new Credentials(credentials.getPrincipal(), - AuthenticationTokenSerializer.deserialize(credentials.getTokenClassName(), credentials.getToken())), tid, tableId, files, errorDir, setTime); + return BulkImporter.bulkLoad(new ServerConfiguration(instance).getConfiguration(), instance, Credentials.fromThrift(credentials), tid, tableId, files, errorDir, setTime); } }); } catch (AccumuloSecurityException e) { @@ -396,8 +394,8 @@ public class ClientServiceHandler implements ClientService.Iface { @Override public List<TDiskUsage> getDiskUsage(Set<String> tables, TCredentials credentials) throws ThriftTableOperationException, ThriftSecurityException, TException { try { - AuthenticationToken token = AuthenticationTokenSerializer.deserialize(credentials.getTokenClassName(), credentials.getToken()); - Connector conn = instance.getConnector(credentials.getPrincipal(), token); + final Credentials creds = Credentials.fromThrift(credentials); + Connector conn = instance.getConnector(creds.getPrincipal(), creds.getToken()); HashSet<String> tableIds = new HashSet<String>(); http://git-wip-us.apache.org/repos/asf/accumulo/blob/6742627f/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java ---------------------------------------------------------------------- diff --git a/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java b/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java index c2a7001..2f9e4d3 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java +++ b/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java @@ -30,7 +30,6 @@ import org.apache.accumulo.core.client.impl.Namespaces; import org.apache.accumulo.core.client.impl.thrift.SecurityErrorCode; import org.apache.accumulo.core.client.impl.thrift.ThriftSecurityException; import org.apache.accumulo.core.client.security.tokens.AuthenticationToken; -import org.apache.accumulo.core.client.security.tokens.AuthenticationToken.AuthenticationTokenSerializer; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.data.thrift.IterInfo; import org.apache.accumulo.core.data.thrift.TColumn; @@ -154,13 +153,15 @@ public class SecurityOperation { if (!credentials.getInstanceId().equals(HdfsZooInstance.getInstance().getInstanceID())) throw new ThriftSecurityException(credentials.getPrincipal(), SecurityErrorCode.INVALID_INSTANCEID); - AuthenticationToken token = AuthenticationTokenSerializer.deserialize(credentials.getTokenClassName(), credentials.getToken()); + Credentials creds = Credentials.fromThrift(credentials); if (isSystemUser(credentials)) { - authenticateSystemUserToken(credentials, token); + if (!(SystemCredentials.get().equals(creds))) { + throw new ThriftSecurityException(creds.getPrincipal(), SecurityErrorCode.BAD_CREDENTIALS); + } } else { try { - if (!authenticator.authenticateUser(credentials.getPrincipal(), token)) { - throw new ThriftSecurityException(credentials.getPrincipal(), SecurityErrorCode.BAD_CREDENTIALS); + if (!authenticator.authenticateUser(creds.getPrincipal(), creds.getToken())) { + throw new ThriftSecurityException(creds.getPrincipal(), SecurityErrorCode.BAD_CREDENTIALS); } } catch (AccumuloSecurityException e) { log.debug(e); @@ -169,11 +170,6 @@ public class SecurityOperation { } } - private void authenticateSystemUserToken(TCredentials credentials, AuthenticationToken token) throws ThriftSecurityException { - if (!SystemCredentials.get().getToken().equals(token)) - throw new ThriftSecurityException(credentials.getPrincipal(), SecurityErrorCode.BAD_CREDENTIALS); - } - public boolean canAskAboutUser(TCredentials credentials, String user) throws ThriftSecurityException { // Authentication done in canPerformSystemActions if (!(canPerformSystemActions(credentials) || credentials.getPrincipal().equals(user))) @@ -187,21 +183,13 @@ public class SecurityOperation { if (credentials.equals(toAuth)) return true; try { - AuthenticationToken token = reassembleToken(toAuth); - return authenticator.authenticateUser(toAuth.getPrincipal(), token); + Credentials toCreds = Credentials.fromThrift(toAuth); + return authenticator.authenticateUser(toCreds.getPrincipal(), toCreds.getToken()); } catch (AccumuloSecurityException e) { throw e.asThriftException(); } } - private AuthenticationToken reassembleToken(TCredentials toAuth) throws AccumuloSecurityException { - String tokenClass = toAuth.getTokenClassName(); - if (authenticator.validTokenClass(tokenClass)) { - return AuthenticationTokenSerializer.deserialize(toAuth.getTokenClassName(), toAuth.getToken()); - } - throw new AccumuloSecurityException(toAuth.getPrincipal(), SecurityErrorCode.INVALID_TOKEN); - } - public Authorizations getUserAuthorizations(TCredentials credentials, String user) throws ThriftSecurityException { authenticate(credentials);