diff --git a/CHANGES.md b/CHANGES.md index 02de03547e6..bf95b084771 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -16,6 +16,7 @@ Apollo 1.10.0 * [refactor: let open api more easier to use and development](https://github.com/apolloconfig/apollo/pull/3943) * [feat(scripts): use bash to call openapi](https://github.com/apolloconfig/apollo/pull/3980) * [Support search by item](https://github.com/apolloconfig/apollo/pull/3977) +* [Implement password policies to avoid weak passwords](https://github.com/apolloconfig/apollo/pull/4008) ------------------ All issues and pull requests are [here](https://github.com/ctripcorp/apollo/milestone/8?closed=1) \ No newline at end of file diff --git a/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/UserInfoController.java b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/UserInfoController.java index 3d5ac10d43a..e204fd169af 100644 --- a/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/UserInfoController.java +++ b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/UserInfoController.java @@ -24,6 +24,12 @@ import com.ctrip.framework.apollo.portal.spi.UserInfoHolder; import com.ctrip.framework.apollo.portal.spi.UserService; import com.ctrip.framework.apollo.portal.spi.springsecurity.SpringSecurityUserService; +import com.ctrip.framework.apollo.portal.util.checker.AuthUserPasswordChecker; +import com.ctrip.framework.apollo.portal.util.checker.CheckResult; +import java.io.IOException; +import java.util.List; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; @@ -32,28 +38,25 @@ import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import java.io.IOException; -import java.util.List; - @RestController public class UserInfoController { private final UserInfoHolder userInfoHolder; private final LogoutHandler logoutHandler; private final UserService userService; + private final AuthUserPasswordChecker passwordChecker; public UserInfoController( final UserInfoHolder userInfoHolder, final LogoutHandler logoutHandler, - final UserService userService) { + final UserService userService, + final AuthUserPasswordChecker passwordChecker) { this.userInfoHolder = userInfoHolder; this.logoutHandler = logoutHandler; this.userService = userService; + this.passwordChecker = passwordChecker; } - @PreAuthorize(value = "@permissionValidator.isSuperAdmin()") @PostMapping("/users") public void createOrUpdateUser(@RequestBody UserPO user) { @@ -61,12 +64,16 @@ public void createOrUpdateUser(@RequestBody UserPO user) { throw new BadRequestException("Username and password can not be empty."); } + CheckResult pwdCheckRes = passwordChecker.checkWeakPassword(user.getPassword()); + if (!pwdCheckRes.isSuccess()) { + throw new BadRequestException(pwdCheckRes.getMessage()); + } + if (userService instanceof SpringSecurityUserService) { ((SpringSecurityUserService) userService).createOrUpdate(user); } else { throw new UnsupportedOperationException("Create or update user operation is unsupported"); } - } @GetMapping("/user") @@ -81,8 +88,8 @@ public void logout(HttpServletRequest request, HttpServletResponse response) thr @GetMapping("/users") public List searchUsersByKeyword(@RequestParam(value = "keyword") String keyword, - @RequestParam(value = "offset", defaultValue = "0") int offset, - @RequestParam(value = "limit", defaultValue = "10") int limit) { + @RequestParam(value = "offset", defaultValue = "0") int offset, + @RequestParam(value = "limit", defaultValue = "10") int limit) { return userService.searchUsers(keyword, offset, limit); } @@ -91,5 +98,4 @@ public UserInfo getUserByUserId(@PathVariable String userId) { return userService.findByUserId(userId); } - } diff --git a/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/util/checker/AuthUserPasswordChecker.java b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/util/checker/AuthUserPasswordChecker.java new file mode 100644 index 00000000000..3abdbfaccff --- /dev/null +++ b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/util/checker/AuthUserPasswordChecker.java @@ -0,0 +1,68 @@ +/* + * Copyright 2021 Apollo Authors + * + * Licensed 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 com.ctrip.framework.apollo.portal.util.checker; + +import com.google.common.base.Strings; +import java.util.Arrays; +import java.util.List; +import java.util.regex.Pattern; +import org.springframework.stereotype.Component; + +@Component +public class AuthUserPasswordChecker implements UserPasswordChecker { + + private static final Pattern PWD_PATTERN = Pattern + .compile("^(?=.*[0-9].*)(?=.*[a-zA-Z].*).{8,20}$"); + + private static final List LIST_OF_CODE_FRAGMENT = Arrays.asList( + "111", "222", "333", "444", "555", "666", "777", "888", "999", "000", + "001122", "112233", "223344", "334455", "445566", "556677", "667788", "778899", "889900", + "009988", "998877", "887766", "776655", "665544", "554433", "443322", "332211", "221100", + "0123", "1234", "2345", "3456", "4567", "5678", "6789", "7890", + "0987", "9876", "8765", "7654", "6543", "5432", "4321", "3210", + "1q2w", "2w3e", "3e4r", "5t6y", "abcd", "qwer", "asdf", "zxcv" + ); + + @Override + public CheckResult checkWeakPassword(String password) { + if (!PWD_PATTERN.matcher(password).matches()) { + return new CheckResult(Boolean.FALSE, + "Password needs a number and letter and between 8~20 characters"); + } + if (isCommonlyUsed(password)) { + return new CheckResult(Boolean.FALSE, + "Passwords cannot be consecutive, regular letters or numbers. And cannot be commonly used. " + + "e.g: abcd1234, 1234qwer, 1q2w3e4r, 1234asdfghjk, ..."); + } + return new CheckResult(Boolean.TRUE, null); + } + + /** + * @return The password contains code fragment or is blank. + */ + private boolean isCommonlyUsed(String password) { + if (Strings.isNullOrEmpty(password)) { + return true; + } + for (String s : LIST_OF_CODE_FRAGMENT) { + if (password.toLowerCase().contains(s)) { + return true; + } + } + return false; + } +} diff --git a/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/util/checker/CheckResult.java b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/util/checker/CheckResult.java new file mode 100644 index 00000000000..e4453527f9f --- /dev/null +++ b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/util/checker/CheckResult.java @@ -0,0 +1,36 @@ +/* + * Copyright 2021 Apollo Authors + * + * Licensed 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 com.ctrip.framework.apollo.portal.util.checker; + +public class CheckResult { + + private final boolean success; + private final String message; + + public CheckResult(boolean success, String message) { + this.success = success; + this.message = message; + } + + public boolean isSuccess() { + return success; + } + + public String getMessage() { + return message; + } +} diff --git a/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/util/checker/UserPasswordChecker.java b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/util/checker/UserPasswordChecker.java new file mode 100644 index 00000000000..73a66159bee --- /dev/null +++ b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/util/checker/UserPasswordChecker.java @@ -0,0 +1,22 @@ +/* + * Copyright 2021 Apollo Authors + * + * Licensed 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 com.ctrip.framework.apollo.portal.util.checker; + +public interface UserPasswordChecker { + + CheckResult checkWeakPassword(String password); +} diff --git a/apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/UserInfoControllerTest.java b/apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/UserInfoControllerTest.java new file mode 100644 index 00000000000..f720268c080 --- /dev/null +++ b/apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/UserInfoControllerTest.java @@ -0,0 +1,73 @@ +/* + * Copyright 2021 Apollo Authors + * + * Licensed 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 com.ctrip.framework.apollo.portal.controller; + +import com.ctrip.framework.apollo.common.exception.BadRequestException; +import com.ctrip.framework.apollo.portal.entity.po.UserPO; +import com.ctrip.framework.apollo.portal.spi.springsecurity.SpringSecurityUserService; +import com.ctrip.framework.apollo.portal.util.checker.AuthUserPasswordChecker; +import com.ctrip.framework.apollo.portal.util.checker.CheckResult; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class UserInfoControllerTest { + + @InjectMocks + private UserInfoController userInfoController; + @Mock + private SpringSecurityUserService userService; + @Mock + private AuthUserPasswordChecker userPasswordChecker; + + @Test + public void testCreateOrUpdateUser() { + UserPO user = new UserPO(); + user.setUsername("username"); + user.setPassword("password"); + + Mockito.when(userPasswordChecker.checkWeakPassword(Mockito.anyString())) + .thenReturn(new CheckResult(Boolean.TRUE, "")); + + userInfoController.createOrUpdateUser(user); + } + + @Test(expected = BadRequestException.class) + public void testCreateOrUpdateUserFailed() { + UserPO user = new UserPO(); + user.setUsername("username"); + user.setPassword("password"); + + String msg = "fake error message"; + + Mockito.when(userPasswordChecker.checkWeakPassword(Mockito.anyString())) + .thenReturn(new CheckResult(Boolean.FALSE, msg)); + + try { + userInfoController.createOrUpdateUser(user); + } catch (BadRequestException e) { + Assert.assertEquals(msg, e.getMessage()); + throw e; + } + } + +} \ No newline at end of file diff --git a/apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/util/AuthUserPasswordCheckerTest.java b/apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/util/AuthUserPasswordCheckerTest.java new file mode 100644 index 00000000000..ceb65cfc9b9 --- /dev/null +++ b/apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/util/AuthUserPasswordCheckerTest.java @@ -0,0 +1,84 @@ +/* + * Copyright 2021 Apollo Authors + * + * Licensed 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 com.ctrip.framework.apollo.portal.util; + +import com.ctrip.framework.apollo.portal.util.checker.AuthUserPasswordChecker; +import com.ctrip.framework.apollo.portal.util.checker.CheckResult; +import java.util.Arrays; +import java.util.List; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +public class AuthUserPasswordCheckerTest { + + private AuthUserPasswordChecker checker; + + @Before + public void setup() { + checker = new AuthUserPasswordChecker(); + } + + @Test + public void testRegexMatch() { + List unMatchList = Arrays.asList( + "11111111", + "oibjdiel", + "oso87b6", + "0vb9xibowkd8bz9dsxbef" + ); + String exceptedErrMsg = "Password needs a number and letter and between 8~20 characters"; + + for (String p : unMatchList) { + CheckResult res = checker.checkWeakPassword(p); + Assert.assertFalse(res.isSuccess()); + Assert.assertEquals(exceptedErrMsg, res.getMessage()); + } + + List matchList = Arrays.asList( + "pziv0g87", + "8f7zjpf8sci93", + "Upz4jF8u2yjV3wn8zp6c" + ); + + for (String p : matchList) { + CheckResult res = checker.checkWeakPassword(p); + Assert.assertTrue(res.isSuccess()); + } + } + + @Test + public void testIsWeakPassword() { + List weakPwdList = Arrays.asList( + "a1234567", "b98765432", "c11111111", "d2222222", "e3333333", "f4444444", + "g5555555", "h6666666", "i7777777", "j8888888", "k9999999", "l0000000", + "1q2w3e4r", "qwertyuiop1", "asdfghjkl2", "asdfghjkl3", "abcd1234" + ); + String exceptedErrMsg = + "Passwords cannot be consecutive, regular letters or numbers. And cannot be commonly used."; + + for (String p : weakPwdList) { + CheckResult res = checker.checkWeakPassword(p); + Assert.assertFalse(res.isSuccess()); + Assert.assertTrue(res.getMessage().startsWith(exceptedErrMsg)); + } + + CheckResult res = checker.checkWeakPassword("1s39gvisk"); + Assert.assertTrue(res.isSuccess()); + } + +} \ No newline at end of file