Skip to content

Commit bf3bb55

Browse files
authored
Clean up AccessController with Exception disambiguation (#5815)
Signed-off-by: Craig Perkins <cwperx@amazon.com>
1 parent 190b7c7 commit bf3bb55

File tree

10 files changed

+38
-157
lines changed

10 files changed

+38
-157
lines changed

src/main/java/org/opensearch/security/auth/http/saml/SamlFilesystemMetadataResolver.java

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,7 @@ public class SamlFilesystemMetadataResolver extends FilesystemMetadataResolver {
2929

3030
@Override
3131
protected byte[] fetchMetadata() throws ResolverException {
32-
try {
33-
return AccessController.doPrivilegedChecked(SamlFilesystemMetadataResolver.super::fetchMetadata);
34-
} catch (Exception e) {
35-
36-
if (e instanceof ResolverException) {
37-
throw (ResolverException) e;
38-
} else {
39-
throw new RuntimeException(e);
40-
}
41-
}
32+
return AccessController.doPrivilegedChecked(SamlFilesystemMetadataResolver.super::fetchMetadata);
4233
}
4334

4435
private static File getMetadataFile(String filePath, Settings settings, Path configPath) {

src/main/java/org/opensearch/security/auth/http/saml/SamlHTTPMetadataResolver.java

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,7 @@ public class SamlHTTPMetadataResolver extends HTTPMetadataResolver {
3737

3838
@Override
3939
protected byte[] fetchMetadata() throws ResolverException {
40-
try {
41-
return AccessController.doPrivilegedChecked(SamlHTTPMetadataResolver.super::fetchMetadata);
42-
} catch (Exception e) {
43-
if (e instanceof ResolverException) {
44-
throw (ResolverException) e;
45-
} else {
46-
throw new RuntimeException(e);
47-
}
48-
}
40+
return AccessController.doPrivilegedChecked(SamlHTTPMetadataResolver.super::fetchMetadata);
4941
}
5042

5143
private static SettingsBasedSSLConfiguratorV4.SSLConfig getSSLConfig(Settings settings, Path configPath) throws Exception {

src/main/java/org/opensearch/security/auth/ldap/util/LdapHelper.java

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,8 @@ public static List<LdapEntry> search(
6666

6767
return entries;
6868
});
69-
} catch (Exception e) {
70-
if (e instanceof LdapException) {
71-
throw (LdapException) e;
72-
} else if (e instanceof RuntimeException) {
73-
throw (RuntimeException) e;
74-
} else {
75-
throw new RuntimeException(e);
76-
}
69+
} catch (InvalidNameException e) {
70+
throw new RuntimeException(e);
7771
}
7872
}
7973

src/main/java/org/opensearch/security/auth/ldap2/LDAPAuthenticationBackend2.java

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -87,17 +87,7 @@ public LDAPAuthenticationBackend2(final Settings settings, final Path configPath
8787

8888
@Override
8989
public User authenticate(AuthenticationContext context) throws OpenSearchSecurityException {
90-
try {
91-
return AccessController.doPrivilegedChecked(() -> authenticate0(context));
92-
} catch (Exception e) {
93-
if (e instanceof OpenSearchSecurityException) {
94-
throw (OpenSearchSecurityException) e;
95-
} else if (e instanceof RuntimeException) {
96-
throw (RuntimeException) e;
97-
} else {
98-
throw new RuntimeException(e);
99-
}
100-
}
90+
return AccessController.doPrivilegedChecked(() -> authenticate0(context));
10191
}
10292

10393
private User authenticate0(AuthenticationContext context) throws OpenSearchSecurityException {
@@ -217,19 +207,7 @@ public Optional<User> impersonate(User user) {
217207
}
218208

219209
private void authenticateByLdapServer(final Connection connection, final String dn, byte[] password) throws LdapException {
220-
try {
221-
AccessController.doPrivilegedChecked(
222-
() -> connection.getProviderConnection().bind(new BindRequest(dn, new Credential(password)))
223-
);
224-
} catch (Exception e) {
225-
if (e instanceof LdapException) {
226-
throw (LdapException) e;
227-
} else if (e instanceof RuntimeException) {
228-
throw (RuntimeException) e;
229-
} else {
230-
throw new RuntimeException(e);
231-
}
232-
}
210+
AccessController.doPrivilegedChecked(() -> connection.getProviderConnection().bind(new BindRequest(dn, new Credential(password))));
233211
}
234212

235213
private void authenticateByLdapServerWithSeparateConnection(final String dn, byte[] password) throws LdapException {

src/main/java/org/opensearch/security/auth/ldap2/LDAPAuthorizationBackend2.java

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -120,17 +120,7 @@ private static List<Map.Entry<String, Settings>> convertOldStyleSettingsToNewSty
120120

121121
@Override
122122
public User addRoles(final User user, AuthenticationContext context) throws OpenSearchSecurityException {
123-
try {
124-
return AccessController.doPrivilegedChecked(() -> addRoles0(user, context));
125-
} catch (Exception e) {
126-
if (e instanceof OpenSearchSecurityException) {
127-
throw (OpenSearchSecurityException) e;
128-
} else if (e instanceof RuntimeException) {
129-
throw (RuntimeException) e;
130-
} else {
131-
throw new RuntimeException(e);
132-
}
133-
}
123+
return AccessController.doPrivilegedChecked(() -> addRoles0(user, context));
134124
}
135125

136126
private User addRoles0(final User user, AuthenticationContext context) throws OpenSearchSecurityException {

src/main/java/org/opensearch/security/auth/ldap2/MakeJava9Happy.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,7 @@ static ClassLoader getClassLoader() {
2727
}
2828

2929
if (classLoader == null) {
30-
31-
try {
32-
return AccessController.doPrivilegedChecked(() -> new Java9CL());
33-
} catch (Exception e) {
34-
throw new RuntimeException(e);
35-
}
30+
return AccessController.doPrivilegedChecked(() -> new Java9CL());
3631
}
3732

3833
return classLoader;

src/main/java/org/opensearch/security/auth/ldap2/PrivilegedProvider.java

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -78,17 +78,7 @@ public JndiProviderConfig getProviderConfig() {
7878

7979
@Override
8080
public ProviderConnection create() throws LdapException {
81-
try {
82-
return AccessController.doPrivilegedChecked(() -> new PrivilegedProviderConnection(delegate.create(), getProviderConfig()));
83-
} catch (Exception e) {
84-
if (e instanceof LdapException) {
85-
throw (LdapException) e;
86-
} else if (e instanceof RuntimeException) {
87-
throw (RuntimeException) e;
88-
} else {
89-
throw new RuntimeException(e);
90-
}
91-
}
81+
return AccessController.doPrivilegedChecked(() -> new PrivilegedProviderConnection(delegate.create(), getProviderConfig()));
9282
}
9383

9484
}
@@ -103,30 +93,20 @@ public PrivilegedProviderConnection(ProviderConnection delegate, JndiProviderCon
10393
}
10494

10595
public Response<Void> bind(BindRequest request) throws LdapException {
106-
try {
107-
return AccessController.doPrivilegedChecked(() -> {
108-
if (jndiProviderConfig.getClassLoader() != null) {
109-
ClassLoader originalClassLoader = Thread.currentThread().getContextClassLoader();
110-
111-
try {
112-
Thread.currentThread().setContextClassLoader(jndiProviderConfig.getClassLoader());
113-
return delegate.bind(request);
114-
} finally {
115-
Thread.currentThread().setContextClassLoader(originalClassLoader);
116-
}
117-
} else {
96+
return AccessController.doPrivilegedChecked(() -> {
97+
if (jndiProviderConfig.getClassLoader() != null) {
98+
ClassLoader originalClassLoader = Thread.currentThread().getContextClassLoader();
99+
100+
try {
101+
Thread.currentThread().setContextClassLoader(jndiProviderConfig.getClassLoader());
118102
return delegate.bind(request);
103+
} finally {
104+
Thread.currentThread().setContextClassLoader(originalClassLoader);
119105
}
120-
});
121-
} catch (Exception e) {
122-
if (e instanceof LdapException) {
123-
throw (LdapException) e;
124-
} else if (e instanceof RuntimeException) {
125-
throw (RuntimeException) e;
126106
} else {
127-
throw new RuntimeException(e);
107+
return delegate.bind(request);
128108
}
129-
}
109+
});
130110
}
131111

132112
public Response<Void> add(AddRequest request) throws LdapException {

src/main/java/org/opensearch/security/dlic/rest/support/Utils.java

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020

2121
import com.google.common.collect.ImmutableList;
2222
import com.google.common.collect.ImmutableSet;
23-
import com.fasterxml.jackson.core.JsonProcessingException;
2423
import com.fasterxml.jackson.core.type.TypeReference;
2524
import com.fasterxml.jackson.databind.JsonNode;
2625
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -149,34 +148,12 @@ public static JsonNode convertJsonToJackson(ToXContent jsonContent, boolean omit
149148
}
150149

151150
public static byte[] jsonMapToByteArray(Map<String, Object> jsonAsMap) throws IOException {
152-
153-
try {
154-
return AccessController.doPrivilegedChecked(() -> internalMapper.writeValueAsBytes(jsonAsMap));
155-
} catch (final Exception e) {
156-
if (e instanceof JsonProcessingException) {
157-
throw (JsonProcessingException) e;
158-
} else if (e instanceof RuntimeException) {
159-
throw (RuntimeException) e;
160-
} else {
161-
throw new RuntimeException(e);
162-
}
163-
}
151+
return AccessController.doPrivilegedChecked(() -> internalMapper.writeValueAsBytes(jsonAsMap));
164152
}
165153

166154
public static Map<String, Object> byteArrayToMutableJsonMap(byte[] jsonBytes) throws IOException {
167-
168-
try {
169-
return AccessController.doPrivilegedChecked(() -> internalMapper.readValue(jsonBytes, new TypeReference<Map<String, Object>>() {
170-
}));
171-
} catch (final Exception e) {
172-
if (e instanceof IOException) {
173-
throw (IOException) e;
174-
} else if (e instanceof RuntimeException) {
175-
throw (RuntimeException) e;
176-
} else {
177-
throw new RuntimeException(e);
178-
}
179-
}
155+
return AccessController.doPrivilegedChecked(() -> internalMapper.readValue(jsonBytes, new TypeReference<Map<String, Object>>() {
156+
}));
180157
}
181158

182159
/**

src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java

Lines changed: 14 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -870,24 +870,15 @@ private SslContext buildSSLServerContext(
870870
final SslProvider sslProvider,
871871
final ClientAuth authMode
872872
) throws SSLException {
873+
final SslContextBuilder _sslContextBuilder = AccessController.doPrivilegedChecked(
874+
() -> configureSSLServerContextBuilder(SslContextBuilder.forServer(_key, _cert), sslProvider, ciphers, authMode)
875+
);
873876

874-
try {
875-
final SslContextBuilder _sslContextBuilder = AccessController.doPrivilegedChecked(
876-
() -> configureSSLServerContextBuilder(SslContextBuilder.forServer(_key, _cert), sslProvider, ciphers, authMode)
877-
);
878-
879-
if (_trustedCerts != null && _trustedCerts.length > 0) {
880-
_sslContextBuilder.trustManager(_trustedCerts);
881-
}
882-
883-
return buildSSLContext0(_sslContextBuilder);
884-
} catch (final Exception e) {
885-
if (e.getCause() instanceof SSLException) {
886-
throw (SSLException) e.getCause();
887-
} else {
888-
throw new RuntimeException(e);
889-
}
877+
if (_trustedCerts != null && _trustedCerts.length > 0) {
878+
_sslContextBuilder.trustManager(_trustedCerts);
890879
}
880+
881+
return buildSSLContext0(_sslContextBuilder);
891882
}
892883

893884
private SslContext buildSSLServerContext(
@@ -899,23 +890,15 @@ private SslContext buildSSLServerContext(
899890
final SslProvider sslProvider,
900891
final ClientAuth authMode
901892
) throws SSLException {
902-
try {
903-
final SslContextBuilder _sslContextBuilder = AccessController.doPrivilegedChecked(
904-
() -> configureSSLServerContextBuilder(SslContextBuilder.forServer(_cert, _key, pwd), sslProvider, ciphers, authMode)
905-
);
906-
907-
if (_trustedCerts != null) {
908-
_sslContextBuilder.trustManager(_trustedCerts);
909-
}
893+
final SslContextBuilder _sslContextBuilder = AccessController.doPrivilegedChecked(
894+
() -> configureSSLServerContextBuilder(SslContextBuilder.forServer(_cert, _key, pwd), sslProvider, ciphers, authMode)
895+
);
910896

911-
return buildSSLContext0(_sslContextBuilder);
912-
} catch (final Exception e) {
913-
if (e.getCause() instanceof SSLException) {
914-
throw (SSLException) e.getCause();
915-
} else {
916-
throw new RuntimeException(e);
917-
}
897+
if (_trustedCerts != null) {
898+
_sslContextBuilder.trustManager(_trustedCerts);
918899
}
900+
901+
return buildSSLContext0(_sslContextBuilder);
919902
}
920903

921904
private SslContextBuilder configureSSLServerContextBuilder(

src/main/java/org/opensearch/security/ssl/SslConfiguration.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.stream.Stream;
2020
import java.util.stream.StreamSupport;
2121
import javax.net.ssl.KeyManagerFactory;
22+
import javax.net.ssl.SSLException;
2223
import javax.net.ssl.TrustManagerFactory;
2324
import javax.security.auth.x500.X500Principal;
2425

@@ -117,7 +118,7 @@ SslContext buildServerSslContext(final boolean validateCertificates) {
117118
.trustManager(trustStoreConfiguration.createTrustManagerFactory(validateCertificates, issuerDns))
118119
.build();
119120
});
120-
} catch (Exception e) {
121+
} catch (SSLException e) {
121122
throw new OpenSearchException("Failed to build server SSL context", e);
122123
}
123124
}

0 commit comments

Comments
 (0)