DaanHoogland commented on code in PR #10311:
URL: https://github.com/apache/cloudstack/pull/10311#discussion_r1938296402
##########
ui/src/components/header/SamlDomainSwitcher.vue:
##########
@@ -52,7 +52,7 @@
<script>
import store from '@/store'
import { api } from '@/api'
-import _ from 'lodash'
+// import _ from 'lodash'
Review Comment:
code in comment
##########
plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLUtils.java:
##########
@@ -200,7 +205,7 @@ public static AuthnRequest buildAuthnRequestObject(final
String authnId, final S
authnRequest.setAssertionConsumerServiceURL(consumerUrl);
authnRequest.setProviderName(spId);
authnRequest.setIssuer(issuer);
- authnRequest.setRequestedAuthnContext(requestedAuthnContext);
+ //authnRequest.setRequestedAuthnContext(requestedAuthnContext);
Review Comment:
please remove
##########
server/src/main/java/com/cloud/user/AccountManagerImpl.java:
##########
@@ -1573,11 +1590,17 @@ protected void
validateAndUpdateUsernameIfNeeded(UpdateUserCmd updateUserCmd, Us
if (duplicatedUser.getId() == user.getId()) {
continue;
}
- Account duplicatedUserAccountWithUserThatHasTheSameUserName =
_accountDao.findById(duplicatedUser.getAccountId());
+
+ // users can't exist in same account
+ if (duplicatedUser.getAccountId() == account.getId()) {
+ throw new
InvalidParameterValueException(String.format("Username [%s] already exists in
account [id=%s,name=%s]", duplicatedUser.getUsername(), account.getUuid(),
account.getAccountName()));
+ }
+
+ /**Account duplicatedUserAccountWithUserThatHasTheSameUserName =
_accountDao.findById(duplicatedUser.getAccountId());
if
(duplicatedUserAccountWithUserThatHasTheSameUserName.getDomainId() ==
account.getDomainId()) {
DomainVO domain =
_domainDao.findById(duplicatedUserAccountWithUserThatHasTheSameUserName.getDomainId());
throw new
InvalidParameterValueException(String.format("Username [%s] already exists in
domain [id=%s,name=%s]", duplicatedUser.getUsername(), domain.getUuid(),
domain.getName()));
- }
+ }*/
Review Comment:
please remove, no need to keep old code in comment.
##########
server/src/main/java/com/cloud/user/AccountManagerImpl.java:
##########
@@ -1573,11 +1590,17 @@ protected void
validateAndUpdateUsernameIfNeeded(UpdateUserCmd updateUserCmd, Us
if (duplicatedUser.getId() == user.getId()) {
continue;
}
- Account duplicatedUserAccountWithUserThatHasTheSameUserName =
_accountDao.findById(duplicatedUser.getAccountId());
+
+ // users can't exist in same account
+ if (duplicatedUser.getAccountId() == account.getId()) {
+ throw new
InvalidParameterValueException(String.format("Username [%s] already exists in
account [id=%s,name=%s]", duplicatedUser.getUsername(), account.getUuid(),
account.getAccountName()));
+ }
Review Comment:
this code is present above as well (lines 1450-1452 and could go in a method.
##########
plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmd.java:
##########
@@ -147,20 +149,26 @@ public String authenticate(final String command, final
Map<String, Object[]> par
||
!nextUserAccount.getExternalEntity().equals(currentUserAccount.getExternalEntity())
|| (nextUserAccount.getDomainId() != domain.getId())
|| (nextUserAccount.getSource() != User.Source.SAML2)) {
+ s_logger.warn("User [" + currentUserAccount.getUsername() + "]
is requesting to switch from user profile [" + currentUserId + "] to user
profile [" + userUuid + "] in domain [" + domainUuid + "] but the associated
target account is not found or invalid");
throw new ServerApiException(ApiErrorCode.PARAM_ERROR,
_apiServer.getSerializedApiError(ApiErrorCode.PARAM_ERROR.getHttpCode(),
"User account is not allowed to switch to the
requested account",
params, responseType));
}
try {
if (_apiServer.verifyUser(nextUserAccount.getId())) {
+ s_logger.info("User [" + currentUserAccount.getUsername()
+ "] user profile switch is accepted: from [" + currentUserId + "] to user
profile [" + userUuid + "] in domain [" + domainUuid + "] with account [" +
nextUserAccount.getAccountName() + "]");
+ // need to set a sessoin variable to inform the login
function of the specific user to login as, rather than using email only (which
could have multiple matches)
+ session.setAttribute("nextUserId", user.getId());
Review Comment:
budiness end of the additions to the code!
##########
ui/src/components/header/SamlDomainSwitcher.vue:
##########
@@ -88,7 +88,8 @@ export default {
this.showSwitcher = false
return
}
- this.samlAccounts = _.orderBy(samlAccounts, ['domainPath'], ['asc'])
+ this.samlAccounts = samlAccounts
+ // this.samlAccounts =_.orderBy(samlAccounts, ['domainPath'],
['asc'])
Review Comment:
code in comment
--
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]
