diff --git a/store/src/java-test/com/zimbra/cs/service/account/ChangePasswordTest.java b/store/src/java-test/com/zimbra/cs/service/account/ChangePasswordTest.java index 582173dcfb..1a23e205af 100644 --- a/store/src/java-test/com/zimbra/cs/service/account/ChangePasswordTest.java +++ b/store/src/java-test/com/zimbra/cs/service/account/ChangePasswordTest.java @@ -10,9 +10,6 @@ import java.util.HashMap; import java.util.Map; -import com.zimbra.cs.account.AccountServiceException.AuthFailedServiceException; -import com.zimbra.cs.account.AuthToken; -import com.zimbra.cs.service.AuthProvider; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -23,14 +20,16 @@ import com.zimbra.common.soap.AccountConstants; import com.zimbra.common.soap.Element; import com.zimbra.cs.account.Account; +import com.zimbra.cs.account.AuthToken; import com.zimbra.cs.account.MockProvisioning; import com.zimbra.cs.account.Provisioning; import com.zimbra.cs.ldap.LdapUtil; import com.zimbra.cs.mailbox.MailboxTestUtil; +import com.zimbra.cs.service.AuthProvider; import com.zimbra.cs.service.mail.ServiceTestUtil; +import com.zimbra.soap.JaxbUtil; import com.zimbra.soap.account.message.ChangePasswordRequest; import com.zimbra.soap.type.AccountSelector; -import com.zimbra.soap.JaxbUtil; public class ChangePasswordTest { private static final String USERNAME_1 = "ron@zcs.fazigu.org"; @@ -114,7 +113,7 @@ public void testNeedsAuth() { Assert.assertFalse("handler.needsAuth()", handler.needsAuth(context)); } - + @Test public void testBasicHandlerWithResetPasswordAuthTokenUsageIfMustChangePasswordIsEnabled() throws Exception { AuthToken authToken = AuthProvider.getAuthToken(account3, AuthToken.Usage.RESET_PASSWORD , AuthToken.TokenType.AUTH); @@ -130,8 +129,12 @@ public void testBasicHandlerWithResetPasswordAuthTokenUsageIfMustChangePasswordI final Element response = handler.handle(JaxbUtil.jaxbToElement(request), context); Assert.assertNotNull("response", response); - String at = response.getAttribute(AccountConstants.E_AUTH_TOKEN); - Assert.assertNotNull("authtoken", at); + try { + response.getAttribute(AccountConstants.E_AUTH_TOKEN); + } catch (final ServiceException ex) { + Assert.assertEquals("service exception type", ServiceException.INVALID_REQUEST, ex.getCode()); + Assert.assertTrue("exception message", ex.getMessage().contains("missing required attribute: authToken")); + } } @After @@ -142,4 +145,3 @@ public void tearDown() throws Exception { prov.deleteAccount(account3.getId()); } } - diff --git a/store/src/java/com/zimbra/cs/service/account/Auth.java b/store/src/java/com/zimbra/cs/service/account/Auth.java index dcdb51b0b7..db0c99c001 100644 --- a/store/src/java/com/zimbra/cs/service/account/Auth.java +++ b/store/src/java/com/zimbra/cs/service/account/Auth.java @@ -20,8 +20,6 @@ */ package com.zimbra.cs.service.account; -import com.zimbra.common.localconfig.LC; -import com.zimbra.cs.service.AuthProviderException; import io.jsonwebtoken.Claims; import java.util.Arrays; @@ -29,8 +27,6 @@ import java.util.Iterator; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; -import java.util.stream.Stream; import javax.servlet.ServletRequest; import javax.servlet.http.HttpServletRequest; @@ -41,13 +37,12 @@ import com.zimbra.common.account.Key; import com.zimbra.common.account.Key.AccountBy; import com.zimbra.common.account.ZAttrProvisioning.AutoProvAuthMech; +import com.zimbra.common.localconfig.LC; import com.zimbra.common.service.ServiceException; import com.zimbra.common.soap.AccountConstants; import com.zimbra.common.soap.Element; import com.zimbra.common.soap.HeaderConstants; import com.zimbra.common.util.Constants; -import com.zimbra.common.util.Pair; -import com.zimbra.common.util.StringUtil; import com.zimbra.common.util.UUIDUtil; import com.zimbra.common.util.ZimbraCookie; import com.zimbra.common.util.ZimbraLog; @@ -329,20 +324,17 @@ public Element handle(Element request, Map context) throws Servi AppSpecificPasswords appPasswords = TwoFactorAuth.getFactory().getAppSpecificPasswords(acct, acctValuePassedIn); appPasswords.authenticate(password); } else { - prov.authAccount(acct, code, AuthContext.Protocol.soap, authCtxt); + Element responseElement = authAccountInternal(prov, acct, code, authCtxt, context, request, twoFactorManager, zsc, tokenType); + if (responseElement != null) { + return responseElement; + } return needTwoFactorAuth(context, request, acct, twoFactorManager, zsc, tokenType, recoveryCode); } } else { if (password != null || recoveryCode != null) { - try { - prov.authAccount(acct, code, AuthContext.Protocol.soap, authCtxt); - } catch (AccountServiceException ase) { - if (AccountServiceException.CHANGE_PASSWORD.equals(ase.getCode())) { - ZimbraLog.account.info("zimbraPasswordMustChange is enabled so creating a auth-token used to change password."); - return needResetPassword(context, request, acct, twoFactorManager, zsc, tokenType); - } else { - throw ase; - } + Element responseElement = authAccountInternal(prov, acct, code, authCtxt, context, request, twoFactorManager, zsc, tokenType); + if (responseElement != null) { + return responseElement; } } else { // it's ok to not have a password if the client is using a 2FA auth token for the 2nd step of 2FA @@ -485,6 +477,21 @@ private Element needTwoFactorAuth(Map context, Element requestEl } } + private Element authAccountInternal(Provisioning prov, Account acct, String code, Map authCtxt, + Map context, Element request, TwoFactorAuth twoFactorManager, ZimbraSoapContext zsc, + TokenType tokenType) throws ServiceException { + try { + prov.authAccount(acct, code, AuthContext.Protocol.soap, authCtxt); + return null; + } catch (AccountServiceException ase) { + if (AccountServiceException.CHANGE_PASSWORD.equals(ase.getCode())) { + ZimbraLog.account.info("zimbraPasswordMustChange is enabled so creating an auth-token used to change password."); + return needResetPassword(context, request, acct, twoFactorManager, zsc, tokenType); + } else { + throw ase; + } + } + } /** * This method is used to create a temporary auth token with usage RESET_PASSWORD. * This auth token further be used for changing the password. diff --git a/store/src/java/com/zimbra/cs/service/account/ChangePassword.java b/store/src/java/com/zimbra/cs/service/account/ChangePassword.java index 491744f6f8..90831d16a7 100644 --- a/store/src/java/com/zimbra/cs/service/account/ChangePassword.java +++ b/store/src/java/com/zimbra/cs/service/account/ChangePassword.java @@ -122,7 +122,7 @@ public Element handle(Element request, Map context) throws Servi } Element response = zsc.createElement(AccountConstants.CHANGE_PASSWORD_RESPONSE); - if (!dryRun) { + if (!dryRun && Usage.AUTH == at.getUsage()) { at = AuthProvider.getAuthToken(acct); at.encodeAuthResp(response, false); response.addAttribute(AccountConstants.E_LIFETIME, at.getExpires() - System.currentTimeMillis(), Element.Disposition.CONTENT); diff --git a/store/src/java/com/zimbra/cs/service/admin/Auth.java b/store/src/java/com/zimbra/cs/service/admin/Auth.java index a823b9a336..159c3a91ed 100644 --- a/store/src/java/com/zimbra/cs/service/admin/Auth.java +++ b/store/src/java/com/zimbra/cs/service/admin/Auth.java @@ -110,7 +110,7 @@ public Element handle(Element request, Map context) throws Servi prov.authAccount(acct, password, AuthContext.Protocol.soap, authCtxt); } catch (AccountServiceException ase) { if (AccountServiceException.CHANGE_PASSWORD.equals(ase.getCode())) { - ZimbraLog.account.info("zimbraPasswordMustChange is enabled so creating a auth-token used to change password."); + ZimbraLog.account.info("zimbraPasswordMustChange is enabled so creating an auth-token used to change password."); usage = Usage.RESET_PASSWORD; } else { throw ase; @@ -120,7 +120,7 @@ public Element handle(Element request, Map context) throws Servi AuthMech authedByMech = (AuthMech) authCtxt.get(AuthContext.AC_AUTHED_BY_MECH); TokenType tokenType = null; - if (AccountUtil.isTwoFactorAccount(acct)) { + if (usage != Usage.RESET_PASSWORD && AccountUtil.isTwoFactorAccount(acct)) { if (StringUtil.isNullOrEmpty(twoFactorCode)) { String reqTokenType = request.getAttribute(AccountConstants.A_TOKEN_TYPE, ""); tokenType = TokenType.fromCode(reqTokenType); diff --git a/store/src/java/com/zimbra/cs/service/admin/ChangePassword.java b/store/src/java/com/zimbra/cs/service/admin/ChangePassword.java index d9f282f686..5d86bd8abc 100644 --- a/store/src/java/com/zimbra/cs/service/admin/ChangePassword.java +++ b/store/src/java/com/zimbra/cs/service/admin/ChangePassword.java @@ -16,6 +16,8 @@ */ package com.zimbra.cs.service.admin; +import java.util.Map; + import com.zimbra.common.account.Key; import com.zimbra.common.service.ServiceException; import com.zimbra.common.soap.AccountConstants; @@ -25,14 +27,13 @@ import com.zimbra.cs.account.Account; import com.zimbra.cs.account.AccountServiceException; import com.zimbra.cs.account.AuthToken; +import com.zimbra.cs.account.AuthToken.Usage; import com.zimbra.cs.account.AuthTokenException; import com.zimbra.cs.account.Domain; import com.zimbra.cs.account.Provisioning; import com.zimbra.cs.service.AuthProvider; import com.zimbra.soap.ZimbraSoapContext; -import java.util.Map; - public class ChangePassword extends AdminDocumentHandler { @Override public Element handle(Element request, Map context) throws ServiceException { @@ -113,7 +114,7 @@ public Element handle(Element request, Map context) throws Servi } Element response = zsc.createElement(AdminConstants.CHANGE_PASSWORD_RESPONSE); - if (!dryRun) { + if (!dryRun && Usage.AUTH == at.getUsage()) { at = AuthProvider.getAuthToken(acct); at.encodeAuthResp(response, true); response.addAttribute(AccountConstants.E_LIFETIME, at.getExpires() - System.currentTimeMillis(), Element.Disposition.CONTENT);