Skip to content

Commit

Permalink
Address static code analysis recommendations from IntelliJ (AthenZ#549)
Browse files Browse the repository at this point in the history
  • Loading branch information
@havetisyan
havetisyan authored Sep 16, 2018
1 parent 31ced22 commit 9158697
Show file tree
Hide file tree
Showing 17 changed files with 58 additions and 192 deletions.
2 changes: 1 addition & 1 deletion clients/java/zpe/src/main/java/com/yahoo/athenz/zpe/AuthZpeClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ public static AccessCheckStatus allowAccess(X509Certificate cert, String angReso
}
return AccessCheckStatus.DENY_CERT_MISSING_DOMAIN;
}
String roleName = subject.substring(idx + ROLE_SEARCH.length(), subject.length());
String roleName = subject.substring(idx + ROLE_SEARCH.length());
if (roleName.isEmpty()) {
if (LOG.isDebugEnabled()) {
LOG.debug("AUTHZPECLT:allowAccess: missing role name");
Expand Down
3 changes: 2 additions & 1 deletion libs/java/auth_core/src/main/java/com/yahoo/athenz/auth/impl/FilePrivateKeyStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.security.PrivateKey;

import org.slf4j.Logger;
Expand Down Expand Up @@ -76,7 +77,7 @@ private String retrieveKeyFromResource(String resourceName) {
try (InputStream is = getClass().getResourceAsStream(resourceName)) {
String resourceData = getString(is);
if (resourceData != null) {
key = Crypto.ybase64(resourceData.getBytes("UTF-8"));
key = Crypto.ybase64(resourceData.getBytes(StandardCharsets.UTF_8));
}
} catch (IOException e) {
if (LOG.isDebugEnabled()) {
Expand Down
4 changes: 2 additions & 2 deletions ...ava/auth_core/src/main/java/com/yahoo/athenz/auth/impl/SimpleServiceIdentityProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ public class SimpleServiceIdentityProvider implements ServiceIdentityProvider {

private String domain;
private String service;
private PrivateKey key = null;
private long tokenTimeout = 3600;
private PrivateKey key;
private long tokenTimeout;
private String keyId;
private String host = null;
private Authority authority;
Expand Down
3 changes: 2 additions & 1 deletion libs/java/auth_core/src/test/java/com/yahoo/athenz/auth/impl/FilePrivateKeyStoreTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.security.PrivateKey;

import org.testng.annotations.Test;
Expand Down Expand Up @@ -95,7 +96,7 @@ public void testGetString() throws IOException {

FilePrivateKeyStoreFactory factory = new FilePrivateKeyStoreFactory();
FilePrivateKeyStore store = (FilePrivateKeyStore) factory.create();
try (InputStream is = new ByteArrayInputStream(str.getBytes("UTF-8"))) {
try (InputStream is = new ByteArrayInputStream(str.getBytes(StandardCharsets.UTF_8))) {
String getStr = store.getString(is);
assertEquals(getStr, str);
}
Expand Down
4 changes: 2 additions & 2 deletions ...nce_provider/src/main/java/com/yahoo/athenz/instance/provider/InstanceProviderClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ private String responseText(final Response response) {
public InstanceConfirmation postInstanceConfirmation(InstanceConfirmation confirmation) {
WebTarget target = base.path("/instance");
Invocation.Builder invocationBuilder = target.request(MediaType.APPLICATION_JSON);
Response response = null;
Response response;
try {
response = invocationBuilder.post(Entity.entity(confirmation, MediaType.APPLICATION_JSON));
} catch (Exception ex) {
Expand All @@ -111,7 +111,7 @@ public InstanceConfirmation postInstanceConfirmation(InstanceConfirmation confir
public InstanceConfirmation postRefreshConfirmation(InstanceConfirmation confirmation) {
WebTarget target = base.path("/refresh");
Invocation.Builder invocationBuilder = target.request(MediaType.APPLICATION_JSON);
Response response = null;
Response response;
try {
response = invocationBuilder.post(Entity.entity(confirmation, MediaType.APPLICATION_JSON));
} catch (Exception ex) {
Expand Down
2 changes: 1 addition & 1 deletion servers/zms/src/main/java/com/yahoo/athenz/zms/DBService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2373,7 +2373,7 @@ Policy updateTemplatePolicy(Policy policy, String domainName, List<TemplateParam
ServiceIdentity updateTemplateServiceIdentity(ServiceIdentity serviceIdentity,
String domainName, List<TemplateParam> params) {

String templateServiceName = serviceIdentity.getName().replace(TEMPLATE_DOMAIN_NAME, domainName);;
String templateServiceName = serviceIdentity.getName().replace(TEMPLATE_DOMAIN_NAME, domainName);
if (params != null) {
for (TemplateParam param : params) {
final String paramKey = "_" + param.getName() + "_";
Expand Down
1 change: 1 addition & 0 deletions servers/zms/src/main/java/com/yahoo/athenz/zms/store/file/FileConnection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,7 @@ public List<String> listPrincipals(String domainName) {
return new ArrayList<>(principals);
}

@SuppressWarnings("SuspiciousListRemoveInLoop")
@Override
public boolean deletePrincipal(String principalName, boolean subDomains) {

Expand Down
5 changes: 3 additions & 2 deletions servers/zms/src/main/java/com/yahoo/athenz/zms/utils/ZMSUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,14 +194,15 @@ public static void validateRoleMembers(final Role role, final String caller,
}
}

@SuppressWarnings("SuspiciousListRemoveInLoop")
public static void removeMembers(List<RoleMember> originalRoleMembers,
List<RoleMember> removeRoleMembers) {
List<RoleMember> removeRoleMembers) {
if (removeRoleMembers == null || originalRoleMembers == null) {
return;
}
for (RoleMember removeMember : removeRoleMembers) {
String removeName = removeMember.getMemberName();
for (int j = 0; j < originalRoleMembers.size(); j ++) {
for (int j = 0; j < originalRoleMembers.size(); j++) {
if (removeName.equalsIgnoreCase(originalRoleMembers.get(j).getMemberName())) {
originalRoleMembers.remove(j);
}
Expand Down
22 changes: 6 additions & 16 deletions servers/zms/src/test/java/com/yahoo/athenz/zms/ZMSImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.io.File;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
Expand Down Expand Up @@ -6896,27 +6897,16 @@ public void testGetPublicKeyZMS() {

String publicKey = zms.getPublicKey("sys.auth", "zms", "0");
assertNotNull(publicKey);
try {
assertEquals(pubKey, Crypto.ybase64(publicKey.getBytes("UTF-8")));
} catch (UnsupportedEncodingException e) {
fail();
}
assertEquals(pubKey, Crypto.ybase64(publicKey.getBytes(StandardCharsets.UTF_8)));


publicKey = zms.getPublicKey("sys.auth", "zms", "1");
assertNotNull(publicKey);
try {
assertEquals(pubKeyK1, Crypto.ybase64(publicKey.getBytes("UTF-8")));
} catch (UnsupportedEncodingException e) {
fail();
}

assertEquals(pubKeyK1, Crypto.ybase64(publicKey.getBytes(StandardCharsets.UTF_8)));

publicKey = zms.getPublicKey("sys.auth", "zms", "2");
assertNotNull(publicKey);
try {
assertEquals(pubKeyK2, Crypto.ybase64(publicKey.getBytes("UTF-8")));
} catch (UnsupportedEncodingException e) {
fail();
}
assertEquals(pubKeyK2, Crypto.ybase64(publicKey.getBytes(StandardCharsets.UTF_8)));
}

@Test
Expand Down
61 changes: 27 additions & 34 deletions servers/zts/src/main/java/com/yahoo/athenz/zts/ZTSImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,10 @@ public class ZTSImpl implements KeyStore, ZTSHandler {

private static String ROOT_DIR;

protected DataStore dataStore = null;
protected CloudStore cloudStore = null;
protected InstanceCertManager instanceCertManager = null;
protected InstanceProviderManager instanceProviderManager = null;
protected DataStore dataStore;
protected CloudStore cloudStore;
protected InstanceCertManager instanceCertManager;
protected InstanceProviderManager instanceProviderManager;
protected Metric metric = null;
protected Schema schema = null;
protected PrivateKey privateKey = null;
Expand All @@ -92,7 +92,6 @@ public class ZTSImpl implements KeyStore, ZTSHandler {
protected int roleTokenDefaultTimeout;
protected int roleTokenMaxTimeout;
protected long x509CertRefreshResetTime;
protected boolean traceAccess = true;
protected long signedPolicyTimeout;
protected static String serverHostName = null;
protected String ostkHostSignerDomain = null;
Expand Down Expand Up @@ -1754,7 +1753,7 @@ X509CertRecord insertX509CertRecord(ResourceContext ctx, final String cn, final
}

return x509CertRecord;
};
}

@Override
public void postInstanceRegisterInformation(ResourceContext ctx, InstanceRegisterInformation info,
Expand Down Expand Up @@ -2198,19 +2197,8 @@ InstanceIdentity processProviderX509RefreshRequest(ResourceContext ctx, final Pr
x509CertRecord.setPrevSerial(x509CertRecord.getCurrentSerial());

} else if (!x509CertRecord.getPrevSerial().equals(serialNumber)) {

// we have a mismatch for both current and previous serial
// numbers so we're going to revoke it

LOGGER.error("Revoking certificate refresh for cn: {} instance id: {}," +
" current serial: {}, previous serial: {}, cert serial: {}",
principalName, x509CertRecord.getInstanceId(), x509CertRecord.getCurrentSerial(),
x509CertRecord.getPrevSerial(), serialNumber);

x509CertRecord.setPrevSerial("-1");
x509CertRecord.setCurrentSerial("-1");

instanceCertManager.updateX509CertRecord(x509CertRecord);

revokeCertificateRefresh(principalName, serialNumber, x509CertRecord);
throw forbiddenError("Certificate revoked", caller, domain);
}

Expand Down Expand Up @@ -2297,19 +2285,8 @@ InstanceIdentity processProviderSSHRefreshRequest(ResourceContext ctx, final Pri
String serialNumber = cert.getSerialNumber().toString();
if (!x509CertRecord.getCurrentSerial().equals(serialNumber) &&
!x509CertRecord.getPrevSerial().equals(serialNumber)) {

// we have a mismatch for both current and previous serial
// numbers so we're going to revoke it

LOGGER.error("Revoking certificate refresh for cn: {} instance id: {}," +
" current serial: {}, previous serial: {}, cert serial: {}",
principalName, x509CertRecord.getInstanceId(), x509CertRecord.getCurrentSerial(),
x509CertRecord.getPrevSerial(), serialNumber);

x509CertRecord.setPrevSerial("-1");
x509CertRecord.setCurrentSerial("-1");

instanceCertManager.updateX509CertRecord(x509CertRecord);

revokeCertificateRefresh(principalName, serialNumber, x509CertRecord);
throw forbiddenError("Certificate revoked", caller, domain);
}

Expand Down Expand Up @@ -2343,7 +2320,23 @@ InstanceIdentity processProviderSSHRefreshRequest(ResourceContext ctx, final Pri

return identity;
}


void revokeCertificateRefresh(final String principalName, final String serialNumber,
X509CertRecord x509CertRecord) {

// we have a mismatch for both current and previous serial
// numbers so we're going to revoke it

LOGGER.error("Revoking certificate refresh for cn: {} instance id: {}, current serial: {}, previous serial: {}, cert serial: {}",
principalName, x509CertRecord.getInstanceId(), x509CertRecord.getCurrentSerial(),
x509CertRecord.getPrevSerial(), serialNumber);

x509CertRecord.setPrevSerial("-1");
x509CertRecord.setCurrentSerial("-1");

instanceCertManager.updateX509CertRecord(x509CertRecord);
}

@Override
public void deleteInstanceIdentity(ResourceContext ctx, String provider,
String domain, String service, String instanceId) {
Expand Down Expand Up @@ -2618,7 +2611,7 @@ public SSHCertificates postSSHCertRequest(ResourceContext ctx, SSHCertRequest ce
// generate our certificate. the ssh signer interface throws
// rest ResourceExceptions so we'll catch and log those

SSHCertificates certs = null;
SSHCertificates certs;
try {
certs = instanceCertManager.getSSHCertificates(principal,
certRequest, instanceId);
Expand Down
3 changes: 1 addition & 2 deletions servers/zts/src/main/java/com/yahoo/athenz/zts/cert/impl/DynamoDBCertRecordStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ public DynamoDBCertRecordStore(AmazonDynamoDB client, final String tableName) {
@Override
public CertRecordStoreConnection getConnection() {
try {
DynamoDBCertRecordStoreConnection dynamoConn = new DynamoDBCertRecordStoreConnection(dynamoDB, tableName);
return dynamoConn;
return new DynamoDBCertRecordStoreConnection(dynamoDB, tableName);
} catch (Exception ex) {
LOG.error("getConnection: {}", ex.getMessage());
throw new ResourceException(ResourceException.SERVICE_UNAVAILABLE, ex.getMessage());
Expand Down
2 changes: 1 addition & 1 deletion servers/zts/src/main/java/com/yahoo/athenz/zts/utils/ZTSUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public static SslContextFactory createSSLContextObject(final String[] clientProt
ZTS_DEFAULT_EXCLUDED_CIPHER_SUITES);
String excludedProtocols = System.getProperty(ZTSConsts.ZTS_PROP_EXCLUDED_PROTOCOLS,
ZTS_DEFAULT_EXCLUDED_PROTOCOLS);
Boolean wantClientAuth = Boolean.parseBoolean(System.getProperty(ZTSConsts.ZTS_PROP_WANT_CLIENT_CERT, "false"));
boolean wantClientAuth = Boolean.parseBoolean(System.getProperty(ZTSConsts.ZTS_PROP_WANT_CLIENT_CERT, "false"));

SslContextFactory sslContextFactory = new SslContextFactory();
if (keyStorePath != null) {
Expand Down
5 changes: 2 additions & 3 deletions servers/zts/src/test/java/com/yahoo/athenz/zts/InstanceProviderManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,8 @@ public void testGetHttpsProviderUnknownProvider() throws NoSuchAlgorithmExceptio
store.processDomain(signedDomain, false);

InstanceProviderManager provider = new InstanceProviderManager(store, SSLContext.getDefault(), null);
InstanceProvider client = provider.getProvider("coretech.weather");
assertNotNull(client);
client.close();
InstanceProvider client = provider.getProvider("coretech.weather2");
assertNull(client);
}

@Test
Expand Down
6 changes: 3 additions & 3 deletions servers/zts/src/test/java/com/yahoo/athenz/zts/ZTSImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2478,8 +2478,8 @@ public void testGetAWSTemporaryCredentials() {

Principal principal = SimplePrincipal.create("user_domain", "user101",
"v=U1;d=user_domain;n=user101;s=signature", 0, null);
CloudStore cloudStore = new MockCloudStore();
((MockCloudStore) cloudStore).setMockFields("1234", "aws_role_name", "user_domain.user101");
MockCloudStore cloudStore = new MockCloudStore();
cloudStore.setMockFields("1234", "aws_role_name", "user_domain.user101");
store.setCloudStore(cloudStore);
zts.cloudStore = cloudStore;

Expand All @@ -2493,7 +2493,7 @@ public void testGetAWSTemporaryCredentials() {
// now try a failure case

try {
((MockCloudStore) cloudStore).setMockFields("1234", "aws_role2_name", "user_domain.user101");
cloudStore.setMockFields("1234", "aws_role2_name", "user_domain.user101");
zts.getAWSTemporaryCredentials(createResourceContext(principal), "athenz.product", "aws_role_name", null, null);
fail();
} catch (ResourceException ex) {
Expand Down
Loading

0 comments on commit 9158697

Please sign in to comment.