Alon Bar-Lev has uploaded a new change for review. Change subject: bll: LoginBaseCommand cleanup: avoid using object state ......................................................................
bll: LoginBaseCommand cleanup: avoid using object state there is no reason to use object state as helper to avoid parameter passing, per single flow. in some control paths variables were not set. Topic: AAA Change-Id: I6bda9adef3630f8e2f7e4da90838261f20a52ea0 Signed-off-by: Alon Bar-Lev <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginAdminUserCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java 2 files changed, 21 insertions(+), 28 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/50/29050/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginAdminUserCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginAdminUserCommand.java index 7736a34..2f82c01 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginAdminUserCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginAdminUserCommand.java @@ -20,9 +20,6 @@ if (!autheticated) { addCanDoActionMessage(VdcBllMessages.USER_NOT_AUTHORIZED_TO_PERFORM_ACTION); } - else { - autheticated = attachUserToSession(); - } } if (! autheticated) { logAutheticationFailure(); 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 c777855..b0357c8 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 @@ -70,15 +70,7 @@ vdcBllMessagesMap.put(Authn.AuthResult.CREDENTIALS_EXPIRED, VdcBllMessages.USER_PASSWORD_EXPIRED); } - private ExtensionProxy authnExtension; - - private AuthenticationProfile profile; - - private ExtMap authRecord; - private String engineSessionId; - - private String principal; public LoginBaseCommand(T parameters) { super(parameters); @@ -111,21 +103,20 @@ } @Override protected boolean canDoAction() { - boolean result = isUserCanBeAuthenticated() && attachUserToSession(); + boolean result = isUserCanBeAuthenticated(); if (! result) { logAutheticationFailure(); } return result; } - protected boolean attachUserToSession() { + private boolean attachUserToSession(AuthenticationProfile profile, ExtMap authRecord) { engineSessionId = UUID.randomUUID().toString(); SessionDataContainer.getInstance().setUser(engineSessionId, getCurrentUser()); SessionDataContainer.getInstance().refresh(engineSessionId); - SessionDataContainer.getInstance().setAuthn(engineSessionId, authnExtension); - if (principal != null) { - SessionDataContainer.getInstance().setPrincipal(engineSessionId, principal); - } + SessionDataContainer.getInstance().setAuthn(engineSessionId, profile.getAuthn()); + SessionDataContainer.getInstance().setPrincipal(engineSessionId, authRecord.<String>get(Authn.AuthRecord.PRINCIPAL)); + // Add the user password to the session, as it will be needed later // when trying to log on to virtual machines: if (getParameters().getPassword() != null) { @@ -145,7 +136,6 @@ } } catch (ParseException e) { log.warn("Error parsing AuthRecord.VALID_TO . Default VALID_TO value will be set on session"); - log.debug("Exception is ", e); } } SessionDataContainer.getInstance().setHardLimit(engineSessionId, validTo); @@ -153,7 +143,7 @@ } protected boolean isUserCanBeAuthenticated() { - profile = AuthenticationProfileRepository.getInstance().getProfile(getParameters().getProfileName()); + AuthenticationProfile profile = AuthenticationProfileRepository.getInstance().getProfile(getParameters().getProfileName()); if (profile == null) { log.errorFormat( "Can't login because authentication profile \"{1}\" doesn't exist.", @@ -163,8 +153,8 @@ return false; } - authnExtension = profile.getAuthn(); - authRecord = (ExtMap) getParameters().getAuthRecord(); + ExtensionProxy authnExtension = profile.getAuthn(); + ExtMap authRecord = (ExtMap) getParameters().getAuthRecord(); int reportReason = Acct.ReportReason.PRINCIPAL_LOGIN_CREDENTIALS; if (getParameters().getAuthType() != null) { if (AuthType.NEGOTIATION == getParameters().getAuthType()) { @@ -193,7 +183,7 @@ return false; } - if (!isPasswordAuth()) { + if (!isPasswordAuth(authnExtension)) { log.errorFormat( "Can't login user \"{0}\" because the authentication profile \"{1}\" doesn't support password " + @@ -218,7 +208,7 @@ loginName = curUser.getLoginName(); password = curPassword; } - authenticate(loginName, password); + authRecord = authenticate(profile, loginName, password); } // Perform the actual authentication: if (authRecord == null) { @@ -316,7 +306,8 @@ dbUser.getLoginName(), principalRecord.<String> get(Authz.PrincipalRecord.NAME) ); - return true; + + return attachUserToSession(profile, authRecord); } private void logEventForUser(String userName, AuditLogType auditLogType) { @@ -355,12 +346,14 @@ AuditLogDirector.log(logable, AuditLogType.USER_VDC_LOGIN_FAILED); } - private boolean isPasswordAuth() { + private boolean isPasswordAuth(ExtensionProxy authnExtension) { return (authnExtension.getContext().<Long> get(Authn.ContextKeys.CAPABILITIES).longValue() & Authn.Capabilities.AUTHENTICATE_PASSWORD) != 0; } - private void authenticate(String user, String password) { + private ExtMap authenticate(AuthenticationProfile profile, String user, String password) { + ExtensionProxy authnExtension = profile.getAuthn(); + ExtMap authRecord = null; ExtensionProxy mapper = profile.getMapper(); if (mapper != null) { user = mapper.invoke(new ExtMap().mput( @@ -383,7 +376,7 @@ password )); - principal = outputMap.<String> get(Authn.InvokeKeys.PRINCIPAL); + String principal = outputMap.<String> get(Authn.InvokeKeys.PRINCIPAL); int authResult = outputMap.<Integer>get(Authn.InvokeKeys.RESULT); if (authResult != Authn.AuthResult.SUCCESS) { log.infoFormat( @@ -423,8 +416,11 @@ } addCanDoActionMessage(msg); } - + } else { + authRecord = outputMap.<ExtMap> get(Authn.InvokeKeys.AUTH_RECORD); } + + return authRecord; } } -- To view, visit http://gerrit.ovirt.org/29050 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6bda9adef3630f8e2f7e4da90838261f20a52ea0 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alon Bar-Lev <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
