JAMES-1930 Authenticator should not mimic backend failure into login failure
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/bf283582 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/bf283582 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/bf283582 Branch: refs/heads/master Commit: bf28358271b21f742a5a8188f909d95332dd174c Parents: 2bf11b0 Author: Benoit Tellier <[email protected]> Authored: Thu Feb 9 15:00:06 2017 +0700 Committer: Antoine Duprat <[email protected]> Committed: Tue Feb 14 11:29:29 2017 +0100 ---------------------------------------------------------------------- .../james/mailbox/store/Authenticator.java | 4 +- .../mailbox/store/StoreMailboxManager.java | 2 +- .../store/UserRepositoryAuthenticator.java | 24 ++---- .../store/UserRepositoryAuthenticatorTest.java | 82 ++++++++++++++++++++ 4 files changed, 92 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/bf283582/mailbox/store/src/main/java/org/apache/james/mailbox/store/Authenticator.java ---------------------------------------------------------------------- diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/Authenticator.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/Authenticator.java index 71e6eb5..f0587b3 100644 --- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/Authenticator.java +++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/Authenticator.java @@ -19,6 +19,8 @@ package org.apache.james.mailbox.store; +import org.apache.james.mailbox.exception.MailboxException; + /** * Authenticates user credentials. */ @@ -32,5 +34,5 @@ public interface Authenticator { * @return true when the user is authentic, * false otherwise */ - boolean isAuthentic(String userid, CharSequence passwd); + boolean isAuthentic(String userid, CharSequence passwd) throws MailboxException; } http://git-wip-us.apache.org/repos/asf/james-project/blob/bf283582/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java ---------------------------------------------------------------------- diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java index 110e08d..0175215 100644 --- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java +++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java @@ -409,7 +409,7 @@ public class StoreMailboxManager implements MailboxManager { * @param passwd the password * @return success true if login success false otherwise */ - private boolean login(String userid, String passwd) { + private boolean login(String userid, String passwd) throws MailboxException { return authenticator.isAuthentic(userid, passwd); } http://git-wip-us.apache.org/repos/asf/james-project/blob/bf283582/server/container/mailbox-adapter/src/main/java/org/apache/james/adapter/mailbox/store/UserRepositoryAuthenticator.java ---------------------------------------------------------------------- diff --git a/server/container/mailbox-adapter/src/main/java/org/apache/james/adapter/mailbox/store/UserRepositoryAuthenticator.java b/server/container/mailbox-adapter/src/main/java/org/apache/james/adapter/mailbox/store/UserRepositoryAuthenticator.java index 6d6f4b3..31de75c 100644 --- a/server/container/mailbox-adapter/src/main/java/org/apache/james/adapter/mailbox/store/UserRepositoryAuthenticator.java +++ b/server/container/mailbox-adapter/src/main/java/org/apache/james/adapter/mailbox/store/UserRepositoryAuthenticator.java @@ -21,42 +21,30 @@ package org.apache.james.adapter.mailbox.store; import javax.inject.Inject; -import org.apache.james.lifecycle.api.LogEnabled; +import org.apache.james.mailbox.exception.MailboxException; import org.apache.james.mailbox.store.Authenticator; import org.apache.james.user.api.UsersRepository; import org.apache.james.user.api.UsersRepositoryException; -import org.slf4j.Logger; /** * Authenticator which use an UsersRepository to check if the user and password * match */ -public class UserRepositoryAuthenticator implements Authenticator, LogEnabled { +public class UserRepositoryAuthenticator implements Authenticator { - private UsersRepository repos; - private Logger log; + private final UsersRepository repos; @Inject - public void setUsersRepository(UsersRepository repos) { + public UserRepositoryAuthenticator(UsersRepository repos) { this.repos = repos; } - /** - * @see - * org.apache.james.mailbox.store.Authenticator#isAuthentic(java.lang.String - * , java.lang.CharSequence) - */ - public boolean isAuthentic(String userid, CharSequence passwd) { + public boolean isAuthentic(String userid, CharSequence passwd) throws MailboxException { try { return repos.test(userid, passwd.toString()); } catch (UsersRepositoryException e) { - log.info("Unable to access UsersRepository", e); + throw new MailboxException("Unable to access UsersRepository", e); } - return false; - } - - public void setLog(Logger log) { - this.log = log; } } http://git-wip-us.apache.org/repos/asf/james-project/blob/bf283582/server/container/mailbox-adapter/src/test/java/org/apache/james/adapter/mailbox/store/UserRepositoryAuthenticatorTest.java ---------------------------------------------------------------------- diff --git a/server/container/mailbox-adapter/src/test/java/org/apache/james/adapter/mailbox/store/UserRepositoryAuthenticatorTest.java b/server/container/mailbox-adapter/src/test/java/org/apache/james/adapter/mailbox/store/UserRepositoryAuthenticatorTest.java new file mode 100644 index 0000000..48c8333 --- /dev/null +++ b/server/container/mailbox-adapter/src/test/java/org/apache/james/adapter/mailbox/store/UserRepositoryAuthenticatorTest.java @@ -0,0 +1,82 @@ +/**************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one * + * or more contributor license agreements. See the NOTICE file * + * distributed with this work for additional information * + * regarding copyright ownership. The ASF licenses this file * + * to you under the Apache License, Version 2.0 (the * + * "License"); you may not use this file except in compliance * + * with the License. You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, * + * software distributed under the License is distributed on an * + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * + * KIND, either express or implied. See the License for the * + * specific language governing permissions and limitations * + * under the License. * + ****************************************************************/ + +package org.apache.james.adapter.mailbox.store; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.apache.james.mailbox.exception.MailboxException; +import org.apache.james.user.api.UsersRepository; +import org.apache.james.user.api.UsersRepositoryException; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +public class UserRepositoryAuthenticatorTest { + + public static final String PASSWORD = "password"; + public static final String USER = "user"; + public static final String BAD_PASSWORD = "badPassword"; + public static final String BAD_USER = "badUser"; + private UsersRepository usersRepository; + private UserRepositoryAuthenticator testee; + + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + @Before + public void setUp() throws Exception { + usersRepository = mock(UsersRepository.class); + testee = new UserRepositoryAuthenticator(usersRepository); + } + + @Test + public void isAuthenticShouldReturnTrueWhenGoodLoginPassword() throws Exception { + when(usersRepository.test(USER, PASSWORD)).thenReturn(true); + + assertThat(testee.isAuthentic(USER, PASSWORD)).isTrue(); + } + + @Test + public void isAuthenticShouldReturnFalseWhenWrongPassword() throws Exception { + when(usersRepository.test(USER, BAD_PASSWORD)).thenReturn(false); + + assertThat(testee.isAuthentic(USER, BAD_PASSWORD)).isFalse(); + } + + @Test + public void isAuthenticShouldReturnFalseWhenBadUser() throws Exception { + when(usersRepository.test(USER, BAD_PASSWORD)).thenReturn(false); + + assertThat(testee.isAuthentic(BAD_USER, BAD_PASSWORD)).isFalse(); + } + + @Test + public void isAuthenticShouldFailOnUserRepositoryFailure() throws Exception { + when(usersRepository.test(USER, PASSWORD)).thenThrow(new UsersRepositoryException("")); + + expectedException.expect(MailboxException.class); + + testee.isAuthentic(USER, PASSWORD); + } + +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
