weizhouapache commented on code in PR #6924:
URL: https://github.com/apache/cloudstack/pull/6924#discussion_r1034820810
##########
server/src/main/java/com/cloud/user/AccountManagerImpl.java:
##########
@@ -139,8 +142,8 @@
import com.cloud.projects.dao.ProjectAccountDao;
import com.cloud.projects.dao.ProjectDao;
import com.cloud.region.ha.GlobalLoadBalancingRulesService;
-import com.cloud.server.auth.UserAuthenticator;
-import com.cloud.server.auth.UserAuthenticator.ActionOnFailedAuthentication;
+import org.apache.cloudstack.auth.UserAuthenticator;
Review Comment:
@harikrishna-patnala
the imports in many classes are not ordered after moving
com.cloud.server.auth.UserAuthenticator to
org.apache.cloudstack.auth.UserAuthenticator
can you fix them ?
##########
ui/src/views/dashboard/Dashboard.vue:
##########
@@ -56,6 +60,7 @@ export default {
}
},
created () {
+ console.log(this.$store.getters.twoFaEnabled)
Review Comment:
this seems unnecesary
##########
setup/db/22beta4to22GA.sql:
##########
@@ -61,7 +61,7 @@ ALTER TABLE `cloud`.`user_ip_address` DROP COLUMN
public_ip_address;
ALTER TABLE `cloud`.`user_ip_address` CHANGE public_ip_address1
public_ip_address char(40) NOT NULL COMMENT 'the public ip address';
DROP VIEW if exists port_forwarding_rules_view;
-ALTER TABLE `cloud`.`port_forwarding_rules` ADD COLUMN `dest_ip_address1`
char(40) NOT NULL COMMENT 'the destination ip address';
+ ALTER TABLE `cloud`.`port_forwarding_rules` ADD COLUMN `dest_ip_address1`
char(40) NOT NULL COMMENT 'the destination ip address';
Review Comment:
this seems unnecessary.
##########
server/src/main/java/com/cloud/user/AccountManagerImpl.java:
##########
@@ -315,6 +322,31 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
private int _cleanupInterval;
private List<String> apiNameList;
+ protected static Map<String, UserTwoFactorAuthenticator>
userTwoFactorAuthenticationProvidersMap = new HashMap<>();
+
+ private List<UserTwoFactorAuthenticator>
userTwoFactorAuthenticationProviders;
+
+ static ConfigKey<Boolean> enableUserTwoFactorAuthentication = new
ConfigKey<Boolean>("Advanced",
+ Boolean.class,
+ "enable.user.two.factor.authentication",
+ "false",
+ "Determines whether two factor authentication is enabled or not.
This can also be configured at domain level.",
+ false,
Review Comment:
@harikrishna-patnala
these settings are dynamic , right ?
##########
server/src/main/java/com/cloud/user/AccountManagerImpl.java:
##########
@@ -315,6 +322,31 @@ public class AccountManagerImpl extends ManagerBase
implements AccountManager, M
private int _cleanupInterval;
private List<String> apiNameList;
+ protected static Map<String, UserTwoFactorAuthenticator>
userTwoFactorAuthenticationProvidersMap = new HashMap<>();
+
+ private List<UserTwoFactorAuthenticator>
userTwoFactorAuthenticationProviders;
+
+ static ConfigKey<Boolean> enableUserTwoFactorAuthentication = new
ConfigKey<Boolean>("Advanced",
+ Boolean.class,
+ "enable.user.two.factor.authentication",
+ "false",
+ "Determines whether two factor authentication is enabled or not.
This can also be configured at domain level.",
+ false,
+ ConfigKey.Scope.Domain);
+
+ ConfigKey<Boolean> mandateUserTwoFactorAuthentication = new
ConfigKey<Boolean>("Advanced",
+ Boolean.class,
+ "mandate.user.two.factor.authentication",
+ "false",
+ "Determines whether to make the two factor authentication
mandatory or not. This can also be configured at domain level.",
+ false,
+ ConfigKey.Scope.Domain);
+
+ ConfigKey<String> userTwoFactorAuthenticationDefaultProvider = new
ConfigKey<>("Advanced", String.class,
+ "user.two.factor.authentication.default.provider",
+ "GOOGLE",
+ "The default user two factor authentication provider plugin. Eg.
google, staticpin", true, ConfigKey.Scope.Domain);
Review Comment:
is there check on the value of config
`userTwoFactorAuthenticationDefaultProvider` ?
##########
plugins/user-two-factor-authenticators/static-pin/src/main/java/org/apache/cloudstack/auth/StaticPinUserTwoFactorAuthenticator.java:
##########
@@ -0,0 +1,74 @@
+// 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.cloudstack.auth;
+
+import javax.inject.Inject;
+
+import com.cloud.exception.CloudAuthenticationException;
+import com.cloud.user.UserAccount;
+import com.cloud.utils.exception.CloudRuntimeException;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.log4j.Logger;
+
+import com.cloud.user.dao.UserAccountDao;
+import com.cloud.utils.component.AdapterBase;
+
+import java.util.Random;
+
+public class StaticPinUserTwoFactorAuthenticator extends AdapterBase
implements UserTwoFactorAuthenticator {
+ public static final Logger s_logger =
Logger.getLogger(StaticPinUserTwoFactorAuthenticator.class);
+
+ @Inject
+ private UserAccountDao _userAccountDao;
+
+ @Override
+ public String getName() {
+ return "staticpin";
+ }
+
+ @Override
+ public String getDescription() {
+ return "Static Pin user two factor authentication provider Plugin";
+ }
+
+ @Override
+ public void check2FA(String code, UserAccount userAccount) throws
CloudAuthenticationException {
+ String expectedCode = getStaticPin(userAccount);
+ if (expectedCode.equals(code)) {
+ s_logger.info("2FA matches user's input");
+ return;
+ }
+ throw new CloudAuthenticationException("two-factor authentication code
provided is invalid");
+ }
+
+ private String getStaticPin(UserAccount userAccount) {
+ return userAccount.getKeyFor2fa();
+ }
+
+ @Override
+ public String setup2FAKey(UserAccount userAccount) {
+ if (StringUtils.isNotEmpty(userAccount.getKeyFor2fa())) {
+ throw new CloudRuntimeException(String.format("2FA key is already
setup for the user account %s", userAccount.getAccountName()));
+ }
+ long seed = System.currentTimeMillis();
+ Random rng = new Random(seed);
Review Comment:
@harikrishna-patnala
would be good to use SecureRandom ?
##########
server/src/main/java/com/cloud/user/AccountManagerImpl.java:
##########
@@ -3100,6 +3171,151 @@ public String getConfigComponentName() {
@Override
public ConfigKey<?>[] getConfigKeys() {
- return new ConfigKey<?>[] {UseSecretKeyInResponse};
+ return new ConfigKey<?>[] {UseSecretKeyInResponse,
enableUserTwoFactorAuthentication, userTwoFactorAuthenticationDefaultProvider,
mandateUserTwoFactorAuthentication};
+ }
+
+ public List<UserTwoFactorAuthenticator>
getUserTwoFactorAuthenticationProviders() {
+ return userTwoFactorAuthenticationProviders;
+ }
+
+ public void setUserTwoFactorAuthenticationProviders(final
List<UserTwoFactorAuthenticator> userTwoFactorAuthenticationProviders) {
+ this.userTwoFactorAuthenticationProviders =
userTwoFactorAuthenticationProviders;
+ }
+
+ private void initializeUserTwoFactorAuthenticationProvidersMap() {
+ if (userTwoFactorAuthenticationProviders != null) {
+ for (final UserTwoFactorAuthenticator userTwoFactorAuthenticator :
userTwoFactorAuthenticationProviders) {
+
userTwoFactorAuthenticationProvidersMap.put(userTwoFactorAuthenticator.getName().toLowerCase(),
userTwoFactorAuthenticator);
+ }
+ }
+ }
+
+ @Override
+ public void verifyUsingTwoFactorAuthenticationCode(final String code,
final Long domainId, final Long userAccountId) {
+
+ Account caller = CallContext.current().getCallingAccount();
+ Account owner = _accountService.getActiveAccountById(caller.getId());
+
+ checkAccess(caller, null, true, owner);
+
+ UserAccount userAccount =
_accountService.getUserAccountById(userAccountId);
+ if (!userAccount.isUser2faEnabled()) {
+ throw new CloudRuntimeException(String.format("Two factor
authentication is not enabled on the user: %s", userAccount.getUsername()));
+ }
+ UserTwoFactorAuthenticator userTwoFactorAuthenticator =
getUserTwoFactorAuthenticator(domainId, userAccountId);
+ userTwoFactorAuthenticator.check2FA(code, userAccount);
+ }
+
+ @Override
+ public UserTwoFactorAuthenticator getUserTwoFactorAuthenticator(Long
domainId, Long userAccountId) {
+ if (userAccountId != null) {
+ UserAccount userAccount =
_accountService.getUserAccountById(userAccountId);
+ if (userAccount.getUser2faProvider() != null) {
+ return
getUserTwoFactorAuthenticator(userAccount.getUser2faProvider());
+ }
+ }
+ final String name =
userTwoFactorAuthenticationDefaultProvider.valueIn(domainId);
+ return getUserTwoFactorAuthenticator(name);
+ }
+
+ @Override
+ public UserTwoFactorAuthenticationSetupResponse
setupUserTwoFactorAuthentication(SetupUserTwoFactorAuthenticationCmd cmd) {
+ String providerName = cmd.getProvider();
+
+ Account caller = CallContext.current().getCallingAccount();
+ Account owner = _accountService.getActiveAccountById(caller.getId());
+
+ if (cmd.getEnable()) {
+ checkAccess(caller, null, true, owner);
+ Long userId = CallContext.current().getCallingUserId();
+
+ UserTwoFactorAuthenticationSetupResponse response =
enableTwoFactorAuthentication(userId, providerName);
+ return response;
+ }
+
+ // Admin can disable 2FA of the users
+ Long userId = cmd.getUserId();
+ UserTwoFactorAuthenticationSetupResponse response =
disableTwoFactorAuthentication(userId, caller, owner);
+
+ return response;
+ }
+
+ protected UserTwoFactorAuthenticationSetupResponse
enableTwoFactorAuthentication(Long userId, String providerName) {
+ UserAccountVO userAccount = _userAccountDao.findById(userId);
+ UserVO userVO = _userDao.findById(userId);
+
+ if
(!enableUserTwoFactorAuthentication.valueIn(userAccount.getDomainId())) {
+ throw new CloudRuntimeException("2FA is not enabled for this
domain or at global level");
+ }
+
+ if (StringUtils.isEmpty(providerName)) {
+ throw new InvalidParameterValueException("Provider name is
mandatory to setup 2FA");
+ }
+ UserTwoFactorAuthenticator provider =
getUserTwoFactorAuthenticationProvider(providerName);
+ String code = provider.setup2FAKey(userAccount);
+ UserVO user = _userDao.createForUpdate();
+ user.setKeyFor2fa(code);
+ user.setUser2faProvider(provider.getName());
+ user.setUser2faEnabled(true);
+ _userDao.update(userId, user);
+
+ UserTwoFactorAuthenticationSetupResponse response = new
UserTwoFactorAuthenticationSetupResponse();
+ response.setId(userVO.getUuid());
+ response.setUsername(userAccount.getUsername());
+ response.setSecretCode(code);
+
+ return response;
}
+
+ protected UserTwoFactorAuthenticationSetupResponse
disableTwoFactorAuthentication(Long userId, Account caller, Account owner) {
+ UserVO userVO = null;
+ if (userId != null) {
+ userVO = validateUser(userId, caller.getDomainId());
+ if (userVO == null) {
+ throw new InvalidParameterValueException("Unable to find user=
" + userVO.getUsername() + " in domain id = " + caller.getDomainId());
+ }
+ owner =
_accountService.getActiveAccountById(userVO.getAccountId());
+ } else {
+ userId = CallContext.current().getCallingUserId();
+ userVO = _userDao.findById(userId);
+ }
+ checkAccess(caller, null, true, owner);
+
+ UserVO user = _userDao.createForUpdate();
+ user.setKeyFor2fa(null);
+ user.setUser2faProvider(null);
+ user.setUser2faEnabled(false);
+ _userDao.update(userVO.getId(), user);
+
+ UserTwoFactorAuthenticationSetupResponse response = new
UserTwoFactorAuthenticationSetupResponse();
+ response.setId(userVO.getUuid());
+ response.setUsername(userVO.getUsername());
+
+ return response;
+ }
+
+ private UserVO validateUser(Long userId, Long domainId) {
+ UserVO user = null;
+ if (userId != null) {
+ user = _userDao.findById(userId);
+ if (user == null) {
+ throw new InvalidParameterValueException("Invalid user ID
provided");
+ }
+ if (_accountDao.findById(user.getAccountId()).getDomainId() !=
domainId) {
+ throw new InvalidParameterValueException("User doesn't belong
to the specified account or domain");
+ }
+ }
+ return user;
+ }
+
+ public UserTwoFactorAuthenticator getUserTwoFactorAuthenticator(final
String name) {
+ if (StringUtils.isEmpty(name)) {
+ throw new CloudRuntimeException("Invalid
UserTwoFactorAuthenticator name provided");
+ }
+ if (!userTwoFactorAuthenticationProvidersMap.containsKey(name)) {
Review Comment:
is the provider name case-sensitive ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
