Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/183#discussion_r141267616
--- Diff:
extensions/guacamole-auth-cas/src/main/java/org/apache/guacamole/auth/cas/AuthenticationProviderService.java
---
@@ -105,4 +146,59 @@ public AuthenticatedUser authenticateUser(Credentials
credentials)
}
+ /**
+ * Takes an encrypted string representing a password provided by
+ * the CAS ClearPass service and decrypts it using the private
+ * key configured for this extension. Returns null if it is
+ * unable to decrypt the password.
+ *
+ * @param encryptedPassword
+ * A string with the encrypted password provided by the
+ * CAS service.
+ *
+ * @return
+ * The decrypted password, or null if it is unable to
+ * decrypt the password.
+ *
+ * @throws GuacamoleException
+ * If unable to get Guacamole configuration data
+ */
+ private final String decryptPassword(String encryptedPassword)
+ throws GuacamoleException {
+
+ // If we get nothing, we return nothing.
+ if (encryptedPassword == null || encryptedPassword.isEmpty()) {
+ logger.warn("No or empty encrypted password, no password will
be available.");
+ return null;
+ }
+
+ final PrivateKey clearpassKey = confService.getClearpassKey();
+ if (clearpassKey == null) {
+ logger.warn("No private key available to decrypt password.");
+ return null;
+ }
+
+ try {
+
+ final Cipher cipher =
Cipher.getInstance(clearpassKey.getAlgorithm());
+
+ if (cipher == null)
+ throw new GuacamoleServerException("Failed to initialize
cipher object with private key.");
+
+ // Initialize the Cipher in decrypt mode.
+ cipher.init(Cipher.DECRYPT_MODE, clearpassKey);
+
+ // Decode and decrypt, and return a new string.
+ final byte[] pass64 =
DatatypeConverter.parseBase64Binary(encryptedPassword);
+ final byte[] cipherData = cipher.doFinal(pass64);
+ return new String(cipherData);
+
+ }
+ catch (Throwable t) {
--- End diff --
Yeah, catching `Throwable` is generally bad. There are cases where it's
necessary, but they're not common. Best practice is normally to explicitly
catch the exceptions which apply, catching superclasses of those exceptions
where it makes sense to do so.
It doesn't have to be pointless - if the context of each relevant exception
is known, you can include a meaningful message in the
`GuacamoleServerException` which you use to wrap the caught exception, rather
than simply rethrowing things.
---
