Yair Zaslavsky has uploaded a new change for review.
Change subject: aaa: Stop using proxies
......................................................................
aaa: Stop using proxies
This patch will remove usage of DirectoryProxy and AuthenticatorProxy in a
gradual manner.
1. Adding getters to ExtensionProxy to both proxies, so we can start working
with the extensions API
at bll commands
2. Changing LoginBaseCommand and AuthenticationFilter to work directly with the
new API
3. Reducing to mininum the code in AuthenticatorProxy - this proxy is still
used as a bridge between
authz and profile.
Change-Id: I916012eab61a96bdb0f366d9dc8462325d7f726f
Topic: AAA
Signed-off-by: Yair Zaslavsky <yzasl...@redhat.com>
---
M
backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticationFilter.java
M
backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticatorProxy.java
M
backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/DirectoryProxy.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java
4 files changed, 147 insertions(+), 131 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/02/26602/1
diff --git
a/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticationFilter.java
b/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticationFilter.java
index 356ffc8..a5327b6 100644
---
a/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticationFilter.java
+++
b/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticationFilter.java
@@ -16,6 +16,8 @@
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
+import org.ovirt.engine.api.extensions.aaa.Authn;
+
/**
* This filter should be added to the {@code web.xml} file to the applications
that want to use the authentication
* mechanism implemented in this package.
@@ -62,8 +64,10 @@
profiles = new ArrayList<AuthenticationProfile>(1);
for (AuthenticationProfile profile :
AuthenticationProfileRepository.getInstance().getProfiles()) {
if (profile != null) {
- Authenticator authenticator =
profile.getAuthenticator();
- if (authenticator.isNegotiationAuth()) {
+ if ((((AuthenticatorProxy)
profile.getAuthenticator()).getExtension()
+ .getContext()
+ .<Long> get(Authn.ContextKeys.CAPABILITIES)
+ .longValue() &
Authn.Capabilities.AUTHENTICATE_NEGOTIATE) ==
Authn.Capabilities.AUTHENTICATE_NEGOTIATE) {
profiles.add(0, profile);
}
diff --git
a/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticatorProxy.java
b/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticatorProxy.java
index 6db7af4..a2f7ccd 100644
---
a/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticatorProxy.java
+++
b/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticatorProxy.java
@@ -3,9 +3,6 @@
import java.util.Properties;
import org.ovirt.engine.api.extensions.Base;
-import org.ovirt.engine.api.extensions.ExtMap;
-import org.ovirt.engine.api.extensions.aaa.Authn;
-import org.ovirt.engine.api.extensionsold.AAAExtensionException;
import org.ovirt.engine.core.extensions.mgr.ExtensionProxy;
public class AuthenticatorProxy extends Authenticator {
@@ -16,28 +13,9 @@
this.extension = extension;
}
- @Override
@Deprecated
+ @Override
public void init() {
- }
-
- @Override
- @Deprecated
- public void authenticate(String user, String password) {
- ExtMap outputMap = extension.invoke(new ExtMap().mput(
- Base.InvokeKeys.COMMAND,
- Authn.InvokeCommands.AUTHENTICATE_CREDENTIALS
- ).mput(
- Authn.InvokeKeys.USER,
- user
- ).mput(
- Authn.InvokeKeys.CREDENTIALS,
- password
- )
- );
- if (outputMap.<Integer> get(Authn.InvokeKeys.RESULT) ==
Authn.AuthResult.CREDENTIALS_INVALID) {
- throw new
AAAExtensionException(AAAExtensionException.AAAExtensionError.INCORRECT_CREDENTIALS,
"");
- }
}
@Override
@@ -53,17 +31,10 @@
.getProperty("ovirt.engine.aaa.authn.profile.name");
}
- @Override
- @Deprecated
- public boolean isNegotiationAuth() {
- return (extension.getContext().<Long>
get(Authn.ContextKeys.CAPABILITIES).longValue() &
Authn.Capabilities.AUTHENTICATE_NEGOTIATE) ==
Authn.Capabilities.AUTHENTICATE_NEGOTIATE;
+ public ExtensionProxy getExtension() {
+ return extension;
}
- @Override
- @Deprecated
- public boolean isPasswordAuth() {
- return (extension.getContext().<Long>
get(Authn.ContextKeys.CAPABILITIES).longValue() &
Authn.Capabilities.AUTHENTICATE_PASSWORD) ==
Authn.Capabilities.AUTHENTICATE_PASSWORD;
- }
}
diff --git
a/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/DirectoryProxy.java
b/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/DirectoryProxy.java
index 4efc2e9..66c116f 100644
---
a/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/DirectoryProxy.java
+++
b/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/DirectoryProxy.java
@@ -123,6 +123,10 @@
return extension.getContext().<String>
get(Base.ContextKeys.INSTANCE_NAME);
}
+ public ExtensionProxy getExtension() {
+ return extension;
+ }
+
private List<DirectoryUser> populateUsers(final ExtUUID command, final
ExtMap extMap) {
final List<DirectoryUser> directoryUsers = new ArrayList<>();
queryImpl(new QueryResultHandler() {
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java
index 136290a..8e75bcb 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java
@@ -1,15 +1,17 @@
package org.ovirt.engine.core.bll;
import java.util.Collections;
-import java.util.EnumMap;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import org.apache.commons.lang.StringUtils;
-import org.ovirt.engine.api.extensionsold.AAAExtensionException;
-import
org.ovirt.engine.api.extensionsold.AAAExtensionException.AAAExtensionError;
+import org.ovirt.engine.api.extensions.Base;
+import org.ovirt.engine.api.extensions.ExtMap;
+import org.ovirt.engine.api.extensions.aaa.Authn;
import org.ovirt.engine.core.aaa.AuthenticationProfile;
import org.ovirt.engine.core.aaa.AuthenticationProfileRepository;
-import org.ovirt.engine.core.aaa.Authenticator;
+import org.ovirt.engine.core.aaa.AuthenticatorProxy;
import org.ovirt.engine.core.aaa.Directory;
import org.ovirt.engine.core.aaa.DirectoryUser;
import org.ovirt.engine.core.aaa.DirectoryUtils;
@@ -25,6 +27,7 @@
import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector;
import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase;
+import org.ovirt.engine.core.extensions.mgr.ExtensionProxy;
import org.ovirt.engine.core.utils.log.Log;
import org.ovirt.engine.core.utils.log.LogFactory;
import org.ovirt.engine.core.utils.threadpool.ThreadPoolUtil;
@@ -32,26 +35,27 @@
public abstract class LoginBaseCommand<T extends LoginUserParameters> extends
CommandBase<T> {
protected static final Log log = LogFactory.getLog(LoginBaseCommand.class);
- private static final EnumMap<AAAExtensionError, AuditLogType> auditLogMap
= new EnumMap<>(AAAExtensionError.class);
- private static final EnumMap<AAAExtensionError, VdcBllMessages>
vdcBllMessagesMap = new EnumMap<>(AAAExtensionError.class);
+ private static final Map<Integer, AuditLogType> auditLogMap = new
HashMap<>();
+ private static final Map<Integer, VdcBllMessages> vdcBllMessagesMap = new
HashMap<>();
static {
- auditLogMap.put(AAAExtensionError.CREDENTIALS_EXPIRED,
AuditLogType.USER_ACCOUNT_PASSWORD_EXPIRED);
- auditLogMap.put(AAAExtensionError.GENERAL_ERROR,
AuditLogType.USER_VDC_LOGIN_FAILED);
- auditLogMap.put(AAAExtensionError.INCORRECT_CREDENTIALS,
AuditLogType.AUTH_FAILED_INVALID_CREDENTIALS);
- auditLogMap.put(AAAExtensionError.LOCKED_OR_DISABLED_ACCOUNT,
AuditLogType.USER_ACCOUNT_DISABLED_OR_LOCKED);
- auditLogMap.put(AAAExtensionError.TIMED_OUT,
AuditLogType.AUTH_FAILED_CONNECTION_TIMED_OUT);
- auditLogMap.put(AAAExtensionError.SERVER_IS_NOT_AVAILABLE,
AuditLogType.AUTH_FAILED_CONNECTION_ERROR);
+ auditLogMap.put(Authn.AuthResult.CREDENTIALS_EXPIRED,
AuditLogType.USER_ACCOUNT_PASSWORD_EXPIRED);
+ auditLogMap.put(Authn.AuthResult.GENERAL_ERROR,
AuditLogType.USER_VDC_LOGIN_FAILED);
+ auditLogMap.put(Authn.AuthResult.CREDENTIALS_INVALID,
AuditLogType.AUTH_FAILED_INVALID_CREDENTIALS);
+ auditLogMap.put(Authn.AuthResult.ACCOUNT_LOCKED,
AuditLogType.USER_ACCOUNT_DISABLED_OR_LOCKED);
+ auditLogMap.put(Authn.AuthResult.ACCOUNT_DISABLED,
AuditLogType.USER_ACCOUNT_DISABLED_OR_LOCKED);
+ auditLogMap.put(Authn.AuthResult.TIMED_OUT,
AuditLogType.USER_ACCOUNT_DISABLED_OR_LOCKED);
- vdcBllMessagesMap.put(AAAExtensionError.GENERAL_ERROR,
VdcBllMessages.USER_FAILED_TO_AUTHENTICATE);
- vdcBllMessagesMap.put(AAAExtensionError.INCORRECT_CREDENTIALS,
+ vdcBllMessagesMap.put(Authn.AuthResult.GENERAL_ERROR,
VdcBllMessages.USER_FAILED_TO_AUTHENTICATE);
+ vdcBllMessagesMap.put(Authn.AuthResult.CREDENTIALS_INVALID,
VdcBllMessages.USER_FAILED_TO_AUTHENTICATE_WRONG_USERNAME_OR_PASSWORD);
- vdcBllMessagesMap.put(AAAExtensionError.LOCKED_OR_DISABLED_ACCOUNT,
VdcBllMessages.USER_ACCOUNT_DISABLED);
- vdcBllMessagesMap.put(AAAExtensionError.SERVER_IS_NOT_AVAILABLE,
-
VdcBllMessages.USER_FAILED_TO_AUTHENTICATE_SERVER_IS_NOT_AVAILABLE);
- vdcBllMessagesMap.put(AAAExtensionError.TIMED_OUT,
VdcBllMessages.USER_FAILED_TO_AUTHENTICATE_TIMED_OUT);
- vdcBllMessagesMap.put(AAAExtensionError.CREDENTIALS_EXPIRED,
VdcBllMessages.USER_PASSWORD_EXPIRED);
+ vdcBllMessagesMap.put(Authn.AuthResult.ACCOUNT_LOCKED,
VdcBllMessages.USER_ACCOUNT_DISABLED);
+ vdcBllMessagesMap.put(Authn.AuthResult.ACCOUNT_DISABLED,
VdcBllMessages.USER_ACCOUNT_DISABLED);
+ vdcBllMessagesMap.put(Authn.AuthResult.TIMED_OUT,
VdcBllMessages.USER_FAILED_TO_AUTHENTICATE_TIMED_OUT);
+ vdcBllMessagesMap.put(Authn.AuthResult.CREDENTIALS_EXPIRED,
VdcBllMessages.USER_PASSWORD_EXPIRED);
}
+
+ private ExtensionProxy extension;
public LoginBaseCommand(T parameters) {
super(parameters);
@@ -151,8 +155,10 @@
}
// Check that the authenticator provided by the profile supports
password authentication:
- Authenticator authenticator = profile.getAuthenticator();
- if (!(authenticator.isPasswordAuth())) {
+ AuthenticatorProxy authenticator = (AuthenticatorProxy)
profile.getAuthenticator();
+ extension = authenticator.getExtension();
+
+ if (!isPasswordAuth()) {
log.errorFormat(
"Can't login user \"{0}\" because the authentication profile
\"{1}\" doesn't support password " +
"authentication.",
@@ -177,89 +183,63 @@
password = curPassword;
}
// Perform the actual authentication:
- try {
- authenticator.authenticate(loginName, password);
- } catch (AAAExtensionException ex) {
- log.infoFormat(
- "Can't login user \"{0}\" with authentication profile
\"{1}\" because the authentication failed.",
- loginName,
- profileName);
- AuditLogType auditLogType = auditLogMap.get(ex.getError());
- //if found matching audit log type, and it's not general login
failure audit log (which will be logged anyway due to CommandBase.log)
- if (auditLogType != null && auditLogType !=
AuditLogType.USER_VDC_LOGIN_FAILED) {
- logEventForUser(loginName, auditLogType);
+ boolean authenticated = authenticate(loginName, password);
+ if (authenticated) {
+ Directory directory = profile.getDirectory();
+ DirectoryUser directoryUser = directory.findUser(loginName);
+ if (directoryUser == null) {
+ log.infoFormat(
+ "Can't login user \"{0}\" with authentication profile
\"{1}\" because the user doesn't exist in the "
+ +
+ "directory.",
+ loginName,
+ profileName
+ );
+
addCanDoActionMessage(VdcBllMessages.USER_MUST_EXIST_IN_DIRECTORY);
+ return false;
}
- VdcBllMessages canDoActionMsg =
vdcBllMessagesMap.get(ex.getError());
- getReturnValue().setSucceeded(false);
- if (canDoActionMsg == VdcBllMessages.USER_PASSWORD_EXPIRED) {
- boolean addedUserPasswordExpiredCDA = false;
- if (authenticator.getChangeExpiredPasswordMsg() != null) {
-
addCanDoActionMessage(VdcBllMessages.USER_PASSWORD_EXPIRED_CHANGE_MSG_PROVIDED);
-
getReturnValue().getCanDoActionMessages().add(String.format("$MSG %1$s",
authenticator.getChangeExpiredPasswordMsg()));
- addedUserPasswordExpiredCDA = true;
- }
- if (authenticator.getChangeExpiredPasswordURL() != null) {
-
addCanDoActionMessage(VdcBllMessages.USER_PASSWORD_EXPIRED_CHANGE_URL_PROVIDED);
-
getReturnValue().getCanDoActionMessages().add(String.format("$URL %1$s",
authenticator.getChangeExpiredPasswordURL()));
- addedUserPasswordExpiredCDA = true;
- }
- if (!addedUserPasswordExpiredCDA) {
-
addCanDoActionMessage(VdcBllMessages.USER_PASSWORD_EXPIRED);
- }
- } else {
-
getReturnValue().getCanDoActionMessages().add(canDoActionMsg.name());
+ // Check that the user exists in the database, if it doesn't exist
then we need to add it now:
+ dbUser = getDbUserDAO().getByExternalId(directory.getName(),
directoryUser.getId());
+ if (dbUser == null) {
+ dbUser = new DbUser(directoryUser);
+ String groupIds =
DirectoryUtils.getGroupIdsFromUser(directoryUser);
+ dbUser.setGroupIds(groupIds);
+ dbUser.setId(Guid.newGuid());
+ getDbUserDAO().save(dbUser);
}
- return false;
+
+ // Check login permissions. We do it here and not via the
+ // getPermissionCheckSubjects mechanism, because we need the user
to be logged in to
+ // the system in order to perform this check. The user is indeed
logged in when running every command
+ // except the login command
+ if (!checkUserAndGroupsAuthorization(dbUser.getId(),
+ dbUser.getGroupIds(),
+ getActionType().getActionGroup(),
+ MultiLevelAdministrationHandler.BOTTOM_OBJECT_ID,
+ VdcObjectType.Bottom,
+ true)) {
+
addCanDoActionMessage(VdcBllMessages.USER_NOT_AUTHORIZED_TO_PERFORM_ACTION);
+ return false;
+ }
+
+ // Retrieve the MLA admin status of the user.
+ // This may be redundant in some use-cases, but looking forward to
Single Sign On,
+ // we will want this info
+ boolean isAdmin =
MultiLevelAdministrationHandler.isAdminUser(dbUser);
+ log.debugFormat("Checking if user {0} is an admin, result {1}",
dbUser.getLoginName(), isAdmin);
+ dbUser.setAdmin(isAdmin);
+ setCurrentUser(dbUser);
+
+ // Add the user password to the session, as it will be needed later
+ // when trying to log on to virtual machines:
+ SessionDataContainer.getInstance().setPassword(password);
+ return true;
}
+ return false;
// Check that the user exists in the directory associated to the
authentication profile:
- Directory directory = profile.getDirectory();
- DirectoryUser directoryUser = directory.findUser(loginName);
- if (directoryUser == null) {
- log.infoFormat(
- "Can't login user \"{0}\" with authentication profile \"{1}\"
because the user doesn't exist in the " +
- "directory.",
- loginName, profileName
- );
- addCanDoActionMessage(VdcBllMessages.USER_MUST_EXIST_IN_DIRECTORY);
- return false;
- }
-
-
- // Check that the user exists in the database, if it doesn't exist
then we need to add it now:
- dbUser = getDbUserDAO().getByExternalId(directory.getName(),
directoryUser.getId());
- if (dbUser == null) {
- dbUser = new DbUser(directoryUser);
- String groupIds =
DirectoryUtils.getGroupIdsFromUser(directoryUser);
- dbUser.setGroupIds(groupIds);
- dbUser.setId(Guid.newGuid());
- getDbUserDAO().save(dbUser);
- }
-
- // Check login permissions. We do it here and not via the
- // getPermissionCheckSubjects mechanism, because we need the user to
be logged in to
- // the system in order to perform this check. The user is indeed
logged in when running every command
- // except the login command
- if (!checkUserAndGroupsAuthorization(dbUser.getId(),
dbUser.getGroupIds(), getActionType().getActionGroup(),
MultiLevelAdministrationHandler.BOTTOM_OBJECT_ID, VdcObjectType.Bottom, true)) {
-
addCanDoActionMessage(VdcBllMessages.USER_NOT_AUTHORIZED_TO_PERFORM_ACTION);
- return false;
- }
-
- // Retrieve the MLA admin status of the user.
- // This may be redundant in some use-cases, but looking forward to
Single Sign On,
- // we will want this info
- boolean isAdmin = MultiLevelAdministrationHandler.isAdminUser(dbUser);
- log.debugFormat("Checking if user {0} is an admin, result {1}",
dbUser.getLoginName(), isAdmin);
- dbUser.setAdmin(isAdmin);
- setCurrentUser(dbUser);
-
- // Add the user password to the session, as it will be needed later
- // when trying to log on to virtual machines:
- SessionDataContainer.getInstance().setPassword(password);
-
- return true;
}
private void logEventForUser(String userName, AuditLogType auditLogType) {
@@ -297,4 +277,61 @@
logable.setUserName(getParameters().getLoginName());
AuditLogDirector.log(logable, AuditLogType.USER_VDC_LOGIN_FAILED);
}
+
+ private boolean isPasswordAuth() {
+ return (extension.getContext().<Long>
get(Authn.ContextKeys.CAPABILITIES).longValue() &
Authn.Capabilities.AUTHENTICATE_PASSWORD) ==
Authn.Capabilities.AUTHENTICATE_PASSWORD;
+ }
+
+ private boolean authenticate(String user, String password) {
+ boolean result = true;
+ ExtMap outputMap = extension.invoke(new ExtMap().mput(
+ Base.InvokeKeys.COMMAND,
+ Authn.InvokeCommands.AUTHENTICATE_CREDENTIALS
+ ).mput(
+ Authn.InvokeKeys.USER,
+ user
+ ).mput(
+ Authn.InvokeKeys.CREDENTIALS,
+ password
+ ));
+
+ int authResult = outputMap.<Integer>get(Authn.InvokeKeys.RESULT);
+ if (authResult != Authn.AuthResult.SUCCESS) {
+ log.infoFormat(
+ "Can't login user \"{0}\" with authentication profile
\"{1}\" because the authentication failed.",
+ user,
+ getParameters().getProfileName());
+
+ AuditLogType auditLogType = auditLogMap.get(authResult);
+ // if found matching audit log type, and it's not general login
failure audit log (which will be logged
+ // anyway due to CommandBase.log)
+ if (auditLogType != null && auditLogType !=
AuditLogType.USER_VDC_LOGIN_FAILED) {
+ logEventForUser(user, auditLogType);
+ }
+
+ if (authResult == Authn.AuthResult.CREDENTIALS_EXPIRED) {
+ boolean addedUserPasswordExpiredCDA = false;
+ if (outputMap.<String>
get(Authn.InvokeKeys.CREDENTIALS_CHANGE_URL) != null) {
+
addCanDoActionMessage(VdcBllMessages.USER_PASSWORD_EXPIRED_CHANGE_URL_PROVIDED);
+
getReturnValue().getCanDoActionMessages().add(String.format("$URL %1$s",
+ outputMap.<String>
get(Authn.InvokeKeys.CREDENTIALS_CHANGE_URL)));
+ addedUserPasswordExpiredCDA = true;
+ }
+ if (outputMap.<String> get(Authn.InvokeKeys.USER_MESSAGE) !=
null) {
+
addCanDoActionMessage(VdcBllMessages.USER_PASSWORD_EXPIRED_CHANGE_MSG_PROVIDED);
+
getReturnValue().getCanDoActionMessages().add(String.format("$MSG %1$s",
+ outputMap.<String>
get(Authn.InvokeKeys.USER_MESSAGE)));
+ addedUserPasswordExpiredCDA = true;
+ }
+ if (!addedUserPasswordExpiredCDA) {
+
addCanDoActionMessage(VdcBllMessages.USER_PASSWORD_EXPIRED);
+ }
+ } else {
+ addCanDoActionMessage(vdcBllMessagesMap.get(authResult));
+ }
+ result = false;
+ }
+ return result;
+ }
+
}
--
To view, visit http://gerrit.ovirt.org/26602
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I916012eab61a96bdb0f366d9dc8462325d7f726f
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
