This is an automated email from the ASF dual-hosted git repository.
jbonofre pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/activemq.git
The following commit(s) were added to refs/heads/master by this push:
new 042cad9 AMQ-7137 - Implement abort() properly in the LoginModules.
Also fix a bug in LdapLoginModule relating to Logout
new ad6df3c Merge pull request #338 from coheigea/loginmodules
042cad9 is described below
commit 042cad9e931378609bf83ac9ecb50f3a224930c8
Author: Colm O hEigeartaigh <cohei...@apache.org>
AuthorDate: Wed Jan 16 14:10:02 2019 +0000
AMQ-7137 - Implement abort() properly in the LoginModules. Also fix a bug
in LdapLoginModule relating to Logout
---
.../activemq/jaas/CertificateLoginModule.java | 43 +++++++++++----
.../org/apache/activemq/jaas/GuestLoginModule.java | 38 ++++++++++----
.../org/apache/activemq/jaas/LDAPLoginModule.java | 42 +++++++++++++--
.../activemq/jaas/PropertiesLoginModule.java | 61 ++++++++++++++--------
4 files changed, 136 insertions(+), 48 deletions(-)
diff --git
a/activemq-jaas/src/main/java/org/apache/activemq/jaas/CertificateLoginModule.java
b/activemq-jaas/src/main/java/org/apache/activemq/jaas/CertificateLoginModule.java
index f2a6528..dd84018 100644
---
a/activemq-jaas/src/main/java/org/apache/activemq/jaas/CertificateLoginModule.java
+++
b/activemq-jaas/src/main/java/org/apache/activemq/jaas/CertificateLoginModule.java
@@ -49,10 +49,13 @@ public abstract class CertificateLoginModule extends
PropertiesLoader implements
private CallbackHandler callbackHandler;
private Subject subject;
- private X509Certificate certificates[];
private String username;
private Set<Principal> principals = new HashSet<Principal>();
+ /** the authentication status*/
+ private boolean succeeded = false;
+ private boolean commitSucceeded = false;
+
/**
* Overriding to allow for proper initialization. Standard JAAS.
*/
@@ -78,7 +81,7 @@ public abstract class CertificateLoginModule extends
PropertiesLoader implements
} catch (UnsupportedCallbackException uce) {
throw new LoginException(uce.getMessage() + " Unable to obtain
client certificates.");
}
- certificates = ((CertificateCallback)callbacks[0]).getCertificates();
+ X509Certificate[] certificates =
((CertificateCallback)callbacks[0]).getCertificates();
username = getUserNameForCertificates(certificates);
if (username == null) {
@@ -88,6 +91,7 @@ public abstract class CertificateLoginModule extends
PropertiesLoader implements
if (debug) {
LOG.debug("Certificate for user: " + username);
}
+ succeeded = true;
return true;
}
@@ -96,6 +100,15 @@ public abstract class CertificateLoginModule extends
PropertiesLoader implements
*/
@Override
public boolean commit() throws LoginException {
+ if (debug) {
+ LOG.debug("commit");
+ }
+
+ if (!succeeded) {
+ clear();
+ return false;
+ }
+
principals.add(new UserPrincipal(username));
for (String group : getUserGroups(username)) {
@@ -104,11 +117,8 @@ public abstract class CertificateLoginModule extends
PropertiesLoader implements
subject.getPrincipals().addAll(principals);
- clear();
-
- if (debug) {
- LOG.debug("commit");
- }
+ username = null;
+ commitSucceeded = true;
return true;
}
@@ -117,11 +127,19 @@ public abstract class CertificateLoginModule extends
PropertiesLoader implements
*/
@Override
public boolean abort() throws LoginException {
- clear();
-
if (debug) {
LOG.debug("abort");
}
+ if (!succeeded) {
+ return false;
+ } else if (succeeded && commitSucceeded) {
+ // we succeeded, but another required module failed
+ logout();
+ } else {
+ // our commit failed
+ clear();
+ succeeded = false;
+ }
return true;
}
@@ -131,11 +149,14 @@ public abstract class CertificateLoginModule extends
PropertiesLoader implements
@Override
public boolean logout() {
subject.getPrincipals().removeAll(principals);
- principals.clear();
+ clear();
if (debug) {
LOG.debug("logout");
}
+
+ succeeded = false;
+ commitSucceeded = false;
return true;
}
@@ -143,8 +164,8 @@ public abstract class CertificateLoginModule extends
PropertiesLoader implements
* Helper method.
*/
private void clear() {
- certificates = null;
username = null;
+ principals.clear();
}
/**
diff --git
a/activemq-jaas/src/main/java/org/apache/activemq/jaas/GuestLoginModule.java
b/activemq-jaas/src/main/java/org/apache/activemq/jaas/GuestLoginModule.java
index 621083d..724e158 100644
--- a/activemq-jaas/src/main/java/org/apache/activemq/jaas/GuestLoginModule.java
+++ b/activemq-jaas/src/main/java/org/apache/activemq/jaas/GuestLoginModule.java
@@ -54,7 +54,10 @@ public class GuestLoginModule implements LoginModule {
private boolean credentialsInvalidate;
private Set<Principal> principals = new HashSet<Principal>();
private CallbackHandler callbackHandler;
- private boolean loginSucceeded;
+
+ /** the authentication status*/
+ private boolean succeeded = false;
+ private boolean commitSucceeded = false;
@Override
public void initialize(Subject subject, CallbackHandler callbackHandler,
Map sharedState, Map options) {
@@ -79,7 +82,7 @@ public class GuestLoginModule implements LoginModule {
@Override
public boolean login() throws LoginException {
- loginSucceeded = true;
+ succeeded = true;
if (credentialsInvalidate) {
PasswordCallback passwordCallback = new
PasswordCallback("Password: ", false);
try {
@@ -88,7 +91,7 @@ public class GuestLoginModule implements LoginModule {
if (debug) {
LOG.debug("Guest login failing
(credentialsInvalidate=true) on presence of a password");
}
- loginSucceeded = false;
+ succeeded = false;
passwordCallback.clearPassword();
};
} catch (IOException ioe) {
@@ -96,21 +99,24 @@ public class GuestLoginModule implements LoginModule {
}
}
if (debug) {
- LOG.debug("Guest login " + loginSucceeded);
+ LOG.debug("Guest login " + succeeded);
}
- return loginSucceeded;
+ return succeeded;
}
@Override
public boolean commit() throws LoginException {
- if (loginSucceeded) {
- subject.getPrincipals().addAll(principals);
- }
-
if (debug) {
LOG.debug("commit");
}
- return loginSucceeded;
+
+ if (!succeeded) {
+ return false;
+ }
+
+ subject.getPrincipals().addAll(principals);
+ commitSucceeded = true;
+ return true;
}
@Override
@@ -119,6 +125,15 @@ public class GuestLoginModule implements LoginModule {
if (debug) {
LOG.debug("abort");
}
+ if (!succeeded) {
+ return false;
+ } else if (succeeded && commitSucceeded) {
+ // we succeeded, but another required module failed
+ logout();
+ } else {
+ // our commit failed
+ succeeded = false;
+ }
return true;
}
@@ -129,6 +144,9 @@ public class GuestLoginModule implements LoginModule {
if (debug) {
LOG.debug("logout");
}
+
+ succeeded = false;
+ commitSucceeded = false;
return true;
}
}
diff --git
a/activemq-jaas/src/main/java/org/apache/activemq/jaas/LDAPLoginModule.java
b/activemq-jaas/src/main/java/org/apache/activemq/jaas/LDAPLoginModule.java
index f0834a0..dced7ce 100644
--- a/activemq-jaas/src/main/java/org/apache/activemq/jaas/LDAPLoginModule.java
+++ b/activemq-jaas/src/main/java/org/apache/activemq/jaas/LDAPLoginModule.java
@@ -72,9 +72,13 @@ public class LDAPLoginModule implements LoginModule {
private Subject subject;
private CallbackHandler handler;
private LDAPLoginProperty [] config;
- private String username;
+ private Principal user;
private Set<GroupPrincipal> groups = new HashSet<GroupPrincipal>();
+ /** the authentication status*/
+ private boolean succeeded = false;
+ private boolean commitSucceeded = false;
+
@Override
public void initialize(Subject subject, CallbackHandler callbackHandler,
Map sharedState, Map options) {
this.subject = subject;
@@ -118,7 +122,7 @@ public class LDAPLoginModule implements LoginModule {
String password;
- username = ((NameCallback)callbacks[0]).getName();
+ String username = ((NameCallback)callbacks[0]).getName();
if (username == null)
return false;
@@ -130,28 +134,56 @@ public class LDAPLoginModule implements LoginModule {
// authenticate will throw LoginException
// in case of failed authentication
authenticate(username, password);
+
+ user = new UserPrincipal(username);
+ succeeded = true;
return true;
}
@Override
public boolean logout() throws LoginException {
- username = null;
+ subject.getPrincipals().remove(user);
+ subject.getPrincipals().removeAll(groups);
+
+ user = null;
+ groups.clear();
+
+ succeeded = false;
+ commitSucceeded = false;
return true;
}
@Override
public boolean commit() throws LoginException {
+ if (!succeeded) {
+ user = null;
+ groups.clear();
+ return false;
+ }
+
Set<Principal> principals = subject.getPrincipals();
- principals.add(new UserPrincipal(username));
+ principals.add(user);
for (GroupPrincipal gp : groups) {
principals.add(gp);
}
+
+ commitSucceeded = true;
return true;
}
@Override
public boolean abort() throws LoginException {
- username = null;
+ if (!succeeded) {
+ return false;
+ } else if (succeeded && commitSucceeded) {
+ // we succeeded, but another required module failed
+ logout();
+ } else {
+ // our commit failed
+ user = null;
+ groups.clear();
+ succeeded = false;
+ }
return true;
}
diff --git
a/activemq-jaas/src/main/java/org/apache/activemq/jaas/PropertiesLoginModule.java
b/activemq-jaas/src/main/java/org/apache/activemq/jaas/PropertiesLoginModule.java
index 5346bd7..153a125 100644
---
a/activemq-jaas/src/main/java/org/apache/activemq/jaas/PropertiesLoginModule.java
+++
b/activemq-jaas/src/main/java/org/apache/activemq/jaas/PropertiesLoginModule.java
@@ -50,13 +50,16 @@ public class PropertiesLoginModule extends PropertiesLoader
implements LoginModu
private Map<String,Set<String>> groups;
private String user;
private final Set<Principal> principals = new HashSet<Principal>();
- private boolean loginSucceeded;
+
+ /** the authentication status*/
+ private boolean succeeded = false;
+ private boolean commitSucceeded = false;
@Override
public void initialize(Subject subject, CallbackHandler callbackHandler,
Map sharedState, Map options) {
this.subject = subject;
this.callbackHandler = callbackHandler;
- loginSucceeded = false;
+ succeeded = false;
init(options);
users = load(USER_FILE_PROP_NAME, "user", options).getProps();
groups = load(GROUP_FILE_PROP_NAME, "group",
options).invertedPropertiesValuesMap();
@@ -91,63 +94,77 @@ public class PropertiesLoginModule extends PropertiesLoader
implements LoginModu
if (!password.equals(new String(tmpPassword))) {
throw new FailedLoginException("Password does not match");
}
- loginSucceeded = true;
+ succeeded = true;
if (debug) {
LOG.debug("login " + user);
}
- return loginSucceeded;
+ return succeeded;
}
@Override
public boolean commit() throws LoginException {
- boolean result = loginSucceeded;
- if (result) {
- principals.add(new UserPrincipal(user));
-
- Set<String> matchedGroups = groups.get(user);
- if (matchedGroups != null) {
- for (String entry : matchedGroups) {
- principals.add(new GroupPrincipal(entry));
- }
+ if (!succeeded) {
+ clear();
+ if (debug) {
+ LOG.debug("commit, result: false");
}
+ return false;
+ }
- subject.getPrincipals().addAll(principals);
+ principals.add(new UserPrincipal(user));
+
+ Set<String> matchedGroups = groups.get(user);
+ if (matchedGroups != null) {
+ for (String entry : matchedGroups) {
+ principals.add(new GroupPrincipal(entry));
+ }
}
- // will whack loginSucceeded
- clear();
+ subject.getPrincipals().addAll(principals);
if (debug) {
- LOG.debug("commit, result: " + result);
+ LOG.debug("commit, result: true");
}
- return result;
+
+ commitSucceeded = true;
+ return true;
}
@Override
public boolean abort() throws LoginException {
- clear();
-
if (debug) {
LOG.debug("abort");
}
+ if (!succeeded) {
+ return false;
+ } else if (succeeded && commitSucceeded) {
+ // we succeeded, but another required module failed
+ logout();
+ } else {
+ // our commit failed
+ clear();
+ succeeded = false;
+ }
return true;
}
@Override
public boolean logout() throws LoginException {
subject.getPrincipals().removeAll(principals);
- principals.clear();
clear();
if (debug) {
LOG.debug("logout");
}
+
+ succeeded = false;
+ commitSucceeded = false;
return true;
}
private void clear() {
user = null;
- loginSucceeded = false;
+ principals.clear();
}
}
