Skip to content

Commit 1950492

Browse files
Rene Gloverdhslove
Rene Glover
authored and
dhslove
committed
4.19 fix saml account selector (apache#10311)
1 parent 543a7ec commit 1950492

File tree

13 files changed

+212
-63
lines changed

13 files changed

+212
-63
lines changed

‎plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmd.java‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,12 @@ public String authenticate(final String command, final Map<String, Object[]> par
131131
}
132132

133133
if (userUuid != null && domainUuid != null) {
134+
logger.debug("User [" + currentUserAccount.getUsername() + "] is requesting to switch from user profile [" + currentUserAccount.getId() + "] to useraccount [" + userUuid + "] in domain [" + domainUuid + "]");
134135
final User user = _userDao.findByUuid(userUuid);
135136
final Domain domain = _domainDao.findByUuid(domainUuid);
136137
final UserAccount nextUserAccount = _accountService.getUserAccountById(user.getId());
137138
if (nextUserAccount != null && !nextUserAccount.getAccountState().equals(Account.State.ENABLED.toString())) {
139+
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 [" + nextUserAccount.getAccountName() + "] is not enabled");
138140
throw new ServerApiException(ApiErrorCode.ACCOUNT_ERROR, _apiServer.getSerializedApiError(ApiErrorCode.PARAM_ERROR.getHttpCode(),
139141
"The requested user account is locked and cannot be switched to, please contact your administrator.",
140142
params, responseType));
@@ -145,20 +147,26 @@ public String authenticate(final String command, final Map<String, Object[]> par
145147
|| !nextUserAccount.getExternalEntity().equals(currentUserAccount.getExternalEntity())
146148
|| (nextUserAccount.getDomainId() != domain.getId())
147149
|| (nextUserAccount.getSource() != User.Source.SAML2)) {
150+
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");
148151
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, _apiServer.getSerializedApiError(ApiErrorCode.PARAM_ERROR.getHttpCode(),
149152
"User account is not allowed to switch to the requested account",
150153
params, responseType));
151154
}
152155
try {
153156
if (_apiServer.verifyUser(nextUserAccount.getId())) {
157+
logger.info("User [" + currentUserAccount.getUsername() + "] user profile switch is accepted: from [" + currentUserId + "] to user profile [" + userUuid + "] in domain [" + domainUuid + "] with account [" + nextUserAccount.getAccountName() + "]");
158+
// 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)
159+
session.setAttribute("nextUserId", user.getId());
154160
final LoginCmdResponse loginResponse = (LoginCmdResponse) _apiServer.loginUser(session, nextUserAccount.getUsername(), nextUserAccount.getUsername() + nextUserAccount.getSource().toString(),
155161
nextUserAccount.getDomainId(), null, remoteAddress, params);
156162
SAMLUtils.setupSamlUserCookies(loginResponse, resp);
157-
resp.sendRedirect(SAML2AuthManager.SAMLCloudStackRedirectionUrl.value());
163+
session.removeAttribute("nextUserId");
164+
logger.debug("User [" + currentUserAccount.getUsername() + "] user profile switch cookies set: from [" + currentUserId + "] to user profile [" + userUuid + "] in domain [" + domainUuid + "] with account [" + nextUserAccount.getAccountName() + "]");
165+
//resp.sendRedirect(SAML2AuthManager.SAMLCloudStackRedirectionUrl.value());
158166
return ApiResponseSerializer.toSerializedString(loginResponse, responseType);
159167
}
160168
} catch (CloudAuthenticationException | IOException exception) {
161-
logger.debug("Failed to switch to request SAML user account due to: " + exception.getMessage());
169+
logger.debug("User [{}] user profile switch cookies set FAILED: from [{}] to user profile [{}] in domain [{}] with account [{}]", currentUserAccount.getUsername(), currentUserId, userUuid, domainUuid, nextUserAccount.getAccountName(), exception);
162170
}
163171
} else {
164172
List<UserAccountVO> switchableAccounts = _userAccountDao.getAllUsersByNameAndEntity(currentUserAccount.getUsername(), currentUserAccount.getExternalEntity());
@@ -176,6 +184,9 @@ public String authenticate(final String command, final Map<String, Object[]> par
176184
accountResponse.setAccountName(userAccount.getAccountName());
177185
accountResponse.setIdpId(user.getExternalEntity());
178186
accountResponses.add(accountResponse);
187+
if (logger.isDebugEnabled()) {
188+
logger.debug("Returning available useraccount for [" + currentUserAccount.getUsername() + "]: UserUUID: [" + user.getUuid() + "], DomainUUID: [" + domain.getUuid() + "], Account: [" + userAccount.getAccountName() + "]");
189+
}
179190
}
180191
ListResponse<SamlUserAccountResponse> response = new ListResponse<SamlUserAccountResponse>();
181192
response.setResponses(accountResponses);

‎plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ public String authenticate(final String command, final Map<String, Object[]> par
190190
String authnId = SAMLUtils.generateSecureRandomId();
191191
samlAuthManager.saveToken(authnId, domainPath, idpMetadata.getEntityId());
192192
logger.debug("Sending SAMLRequest id=" + authnId);
193-
String redirectUrl = SAMLUtils.buildAuthnRequestUrl(authnId, spMetadata, idpMetadata, SAML2AuthManager.SAMLSignatureAlgorithm.value());
193+
String redirectUrl = SAMLUtils.buildAuthnRequestUrl(authnId, spMetadata, idpMetadata, SAML2AuthManager.SAMLSignatureAlgorithm.value(), SAML2AuthManager.SAMLRequirePasswordLogin.value());
194194
resp.sendRedirect(redirectUrl);
195195
return "";
196196
} if (params.containsKey("SAMLart")) {

‎plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ public interface SAML2AuthManager extends PluggableAPIAuthenticator, PluggableSe
7979
ConfigKey<String> SAMLUserSessionKeyPathAttribute = new ConfigKey<String>("Advanced", String.class, "saml2.user.sessionkey.path", "",
8080
"The Path attribute of sessionkey cookie when SAML users have logged in. If not set, it will be set to the path of SAML redirection URL (saml2.redirect.url).", true);
8181

82+
ConfigKey<Boolean> SAMLRequirePasswordLogin = new ConfigKey<Boolean>("Advanced", Boolean.class, "saml2.require.password", "true",
83+
"When enabled SAML2 will validate that the SAML login was performed with a password. If disabled, other forms of authentication are allowed (two-factor, certificate, etc) on the SAML Authentication Provider", true);
84+
85+
8286
SAMLProviderMetadata getSPMetadata();
8387
SAMLProviderMetadata getIdPMetadata(String entityId);
8488
Collection<SAMLProviderMetadata> getAllIdPMetadata();

‎plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,6 @@ public ConfigKey<?>[] getConfigKeys() {
541541
SAMLCloudStackRedirectionUrl, SAMLUserAttributeName,
542542
SAMLIdentityProviderMetadataURL, SAMLDefaultIdentityProviderId,
543543
SAMLSignatureAlgorithm, SAMLAppendDomainSuffix, SAMLTimeout, SAMLCheckSignature,
544-
SAMLForceAuthn, SAMLUserSessionKeyPathAttribute};
544+
SAMLForceAuthn, SAMLUserSessionKeyPathAttribute, SAMLRequirePasswordLogin};
545545
}
546546
}

‎plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLUtils.java‎

Lines changed: 54 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -152,11 +152,11 @@ public static String getValueFromAssertions(final List<Assertion> assertions, fi
152152
return null;
153153
}
154154

155-
public static String buildAuthnRequestUrl(final String authnId, final SAMLProviderMetadata spMetadata, final SAMLProviderMetadata idpMetadata, final String signatureAlgorithm) {
155+
public static String buildAuthnRequestUrl(final String authnId, final SAMLProviderMetadata spMetadata, final SAMLProviderMetadata idpMetadata, final String signatureAlgorithm, boolean requirePasswordAuthentication) {
156156
String redirectUrl = "";
157157
try {
158158
DefaultBootstrap.bootstrap();
159-
AuthnRequest authnRequest = SAMLUtils.buildAuthnRequestObject(authnId, spMetadata.getEntityId(), idpMetadata.getSsoUrl(), spMetadata.getSsoUrl());
159+
AuthnRequest authnRequest = SAMLUtils.buildAuthnRequestObject(authnId, spMetadata.getEntityId(), idpMetadata.getSsoUrl(), spMetadata.getSsoUrl(), requirePasswordAuthentication);
160160
PrivateKey privateKey = null;
161161
if (spMetadata.getKeyPair() != null) {
162162
privateKey = spMetadata.getKeyPair().getPrivate();
@@ -169,28 +169,21 @@ public static String buildAuthnRequestUrl(final String authnId, final SAMLProvid
169169
return redirectUrl;
170170
}
171171

172-
public static AuthnRequest buildAuthnRequestObject(final String authnId, final String spId, final String idpUrl, final String consumerUrl) {
172+
public static AuthnRequest buildAuthnRequestObject(final String authnId, final String spId, final String idpUrl, final String consumerUrl, boolean requirePasswordAuthentication) {
173173
// Issuer object
174174
IssuerBuilder issuerBuilder = new IssuerBuilder();
175175
Issuer issuer = issuerBuilder.buildObject();
176176
issuer.setValue(spId);
177177

178-
// AuthnContextClass
179-
AuthnContextClassRefBuilder authnContextClassRefBuilder = new AuthnContextClassRefBuilder();
180-
AuthnContextClassRef authnContextClassRef = authnContextClassRefBuilder.buildObject(
181-
SAMLConstants.SAML20_NS,
182-
"AuthnContextClassRef", "saml");
183-
authnContextClassRef.setAuthnContextClassRef(AuthnContext.PPT_AUTHN_CTX);
184-
185-
// AuthnContext
186-
RequestedAuthnContextBuilder requestedAuthnContextBuilder = new RequestedAuthnContextBuilder();
187-
RequestedAuthnContext requestedAuthnContext = requestedAuthnContextBuilder.buildObject();
188-
requestedAuthnContext.setComparison(AuthnContextComparisonTypeEnumeration.EXACT);
189-
requestedAuthnContext.getAuthnContextClassRefs().add(authnContextClassRef);
190-
191178
// Creation of AuthRequestObject
192179
AuthnRequestBuilder authRequestBuilder = new AuthnRequestBuilder();
193180
AuthnRequest authnRequest = authRequestBuilder.buildObject();
181+
182+
// AuthnContextClass. When this is false, the authentication requirements are defered to the SAML IDP and its default or configured workflow
183+
if (requirePasswordAuthentication) {
184+
setRequestedAuthnContext(authnRequest, requirePasswordAuthentication);
185+
}
186+
194187
authnRequest.setID(authnId);
195188
authnRequest.setDestination(idpUrl);
196189
authnRequest.setVersion(SAMLVersion.VERSION_20);
@@ -201,11 +194,25 @@ public static AuthnRequest buildAuthnRequestObject(final String authnId, final S
201194
authnRequest.setAssertionConsumerServiceURL(consumerUrl);
202195
authnRequest.setProviderName(spId);
203196
authnRequest.setIssuer(issuer);
204-
authnRequest.setRequestedAuthnContext(requestedAuthnContext);
205197

206198
return authnRequest;
207199
}
208200

201+
public static void setRequestedAuthnContext(AuthnRequest authnRequest, boolean requirePasswordAuthentication) {
202+
AuthnContextClassRefBuilder authnContextClassRefBuilder = new AuthnContextClassRefBuilder();
203+
AuthnContextClassRef authnContextClassRef = authnContextClassRefBuilder.buildObject(
204+
SAMLConstants.SAML20_NS,
205+
"AuthnContextClassRef", "saml");
206+
authnContextClassRef.setAuthnContextClassRef(AuthnContext.PPT_AUTHN_CTX);
207+
208+
// AuthnContext
209+
RequestedAuthnContextBuilder requestedAuthnContextBuilder = new RequestedAuthnContextBuilder();
210+
RequestedAuthnContext requestedAuthnContext = requestedAuthnContextBuilder.buildObject();
211+
requestedAuthnContext.setComparison(AuthnContextComparisonTypeEnumeration.EXACT);
212+
requestedAuthnContext.getAuthnContextClassRefs().add(authnContextClassRef);
213+
authnRequest.setRequestedAuthnContext(requestedAuthnContext);
214+
}
215+
209216
public static LogoutRequest buildLogoutRequest(String logoutUrl, String spId, String nameIdString) {
210217
Issuer issuer = new IssuerBuilder().buildObject();
211218
issuer.setValue(spId);
@@ -285,23 +292,6 @@ public static String generateSAMLRequestSignature(final String urlEncodedString,
285292
}
286293

287294
public static void setupSamlUserCookies(final LoginCmdResponse loginResponse, final HttpServletResponse resp) throws IOException {
288-
resp.addCookie(new Cookie("userid", URLEncoder.encode(loginResponse.getUserId(), HttpUtils.UTF_8)));
289-
resp.addCookie(new Cookie("domainid", URLEncoder.encode(loginResponse.getDomainId(), HttpUtils.UTF_8)));
290-
resp.addCookie(new Cookie("role", URLEncoder.encode(loginResponse.getType(), HttpUtils.UTF_8)));
291-
resp.addCookie(new Cookie("username", URLEncoder.encode(loginResponse.getUsername(), HttpUtils.UTF_8)));
292-
resp.addCookie(new Cookie("account", URLEncoder.encode(loginResponse.getAccount(), HttpUtils.UTF_8)));
293-
resp.addCookie(new Cookie("isSAML", URLEncoder.encode("true", HttpUtils.UTF_8)));
294-
resp.addCookie(new Cookie("twoFaEnabled", URLEncoder.encode(loginResponse.is2FAenabled(), HttpUtils.UTF_8)));
295-
String providerFor2FA = loginResponse.getProviderFor2FA();
296-
if (StringUtils.isNotEmpty(providerFor2FA)) {
297-
resp.addCookie(new Cookie("twoFaProvider", URLEncoder.encode(loginResponse.getProviderFor2FA(), HttpUtils.UTF_8)));
298-
}
299-
String timezone = loginResponse.getTimeZone();
300-
if (timezone != null) {
301-
resp.addCookie(new Cookie("timezone", URLEncoder.encode(timezone, HttpUtils.UTF_8)));
302-
}
303-
resp.addCookie(new Cookie("userfullname", URLEncoder.encode(loginResponse.getFirstName() + " " + loginResponse.getLastName(), HttpUtils.UTF_8).replace("+", "%20")));
304-
305295
String redirectUrl = SAML2AuthManager.SAMLCloudStackRedirectionUrl.value();
306296
String path = SAML2AuthManager.SAMLUserSessionKeyPathAttribute.value();
307297
String domain = null;
@@ -317,13 +307,43 @@ public static void setupSamlUserCookies(final LoginCmdResponse loginResponse, fi
317307
} catch (URISyntaxException ex) {
318308
throw new CloudRuntimeException("Invalid URI: " + redirectUrl);
319309
}
310+
311+
addBaseCookies(loginResponse, resp, domain, path);
312+
313+
String providerFor2FA = loginResponse.getProviderFor2FA();
314+
if (StringUtils.isNotEmpty(providerFor2FA)) {
315+
resp.addCookie(newCookie(domain, path,"twoFaProvider", URLEncoder.encode(loginResponse.getProviderFor2FA(), HttpUtils.UTF_8)));
316+
}
317+
String timezone = loginResponse.getTimeZone();
318+
if (timezone != null) {
319+
resp.addCookie(newCookie(domain, path,"timezone", URLEncoder.encode(timezone, HttpUtils.UTF_8)));
320+
}
321+
320322
String sameSite = ApiServlet.getApiSessionKeySameSite();
321323
String sessionKeyCookie = String.format("%s=%s;Domain=%s;Path=%s;%s", ApiConstants.SESSIONKEY, loginResponse.getSessionKey(), domain, path, sameSite);
322324
LOGGER.debug("Adding sessionkey cookie to response: " + sessionKeyCookie);
323325
resp.addHeader("SET-COOKIE", sessionKeyCookie);
324326
resp.addHeader("SET-COOKIE", String.format("%s=%s;HttpOnly;Path=/client/api;%s", ApiConstants.SESSIONKEY, loginResponse.getSessionKey(), sameSite));
325327
}
326328

329+
private static void addBaseCookies(final LoginCmdResponse loginResponse, final HttpServletResponse resp, String domain, String path) throws IOException {
330+
resp.addCookie(newCookie(domain, path, "userid", URLEncoder.encode(loginResponse.getUserId(), HttpUtils.UTF_8)));
331+
resp.addCookie(newCookie(domain, path,"domainid", URLEncoder.encode(loginResponse.getDomainId(), HttpUtils.UTF_8)));
332+
resp.addCookie(newCookie(domain, path,"role", URLEncoder.encode(loginResponse.getType(), HttpUtils.UTF_8)));
333+
resp.addCookie(newCookie(domain, path,"username", URLEncoder.encode(loginResponse.getUsername(), HttpUtils.UTF_8)));
334+
resp.addCookie(newCookie(domain, path,"account", URLEncoder.encode(loginResponse.getAccount(), HttpUtils.UTF_8)));
335+
resp.addCookie(newCookie(domain, path,"isSAML", URLEncoder.encode("true", HttpUtils.UTF_8)));
336+
resp.addCookie(newCookie(domain, path,"twoFaEnabled", URLEncoder.encode(loginResponse.is2FAenabled(), HttpUtils.UTF_8)));
337+
resp.addCookie(newCookie(domain, path,"userfullname", URLEncoder.encode(loginResponse.getFirstName() + " " + loginResponse.getLastName(), HttpUtils.UTF_8).replace("+", "%20")));
338+
}
339+
340+
private static Cookie newCookie(final String domain, final String path, final String name, final String value) {
341+
Cookie cookie = new Cookie(name, value);
342+
cookie.setDomain(domain);
343+
cookie.setPath(path);
344+
return cookie;
345+
}
346+
327347
/**
328348
* Returns base64 encoded PublicKey
329349
* @param key PublicKey

‎plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAMLUtilsTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public void testBuildAuthnRequestObject() throws Exception {
5858
String idpUrl = "http://idp.domain.example";
5959
String spId = "cloudstack";
6060
String authnId = SAMLUtils.generateSecureRandomId();
61-
AuthnRequest req = SAMLUtils.buildAuthnRequestObject(authnId, spId, idpUrl, consumerUrl);
61+
AuthnRequest req = SAMLUtils.buildAuthnRequestObject(authnId, spId, idpUrl, consumerUrl, true);
6262
assertEquals(req.getAssertionConsumerServiceURL(), consumerUrl);
6363
assertEquals(req.getDestination(), idpUrl);
6464
assertEquals(req.getIssuer().getValue(), spId);
@@ -86,7 +86,7 @@ public void testBuildAuthnRequestUrlWithoutQueryParam() throws Exception {
8686
idpMetadata.setSsoUrl(idpUrl);
8787
idpMetadata.setEntityId(idpId);
8888

89-
URI redirectUrl = new URI(SAMLUtils.buildAuthnRequestUrl(authnId, spMetadata, idpMetadata, SAML2AuthManager.SAMLSignatureAlgorithm.value()));
89+
URI redirectUrl = new URI(SAMLUtils.buildAuthnRequestUrl(authnId, spMetadata, idpMetadata, SAML2AuthManager.SAMLSignatureAlgorithm.value(), true));
9090
assertThat(redirectUrl).hasScheme(urlScheme).hasHost(idpDomain).hasParameter("SAMLRequest");
9191
assertEquals(urlScheme, redirectUrl.getScheme());
9292
assertEquals(idpDomain, redirectUrl.getHost());
@@ -115,7 +115,7 @@ public void testBuildAuthnRequestUrlWithQueryParam() throws Exception {
115115
idpMetadata.setSsoUrl(idpUrl);
116116
idpMetadata.setEntityId(idpId);
117117

118-
URI redirectUrl = new URI(SAMLUtils.buildAuthnRequestUrl(authnId, spMetadata, idpMetadata, SAML2AuthManager.SAMLSignatureAlgorithm.value()));
118+
URI redirectUrl = new URI(SAMLUtils.buildAuthnRequestUrl(authnId, spMetadata, idpMetadata, SAML2AuthManager.SAMLSignatureAlgorithm.value(), true));
119119
assertThat(redirectUrl).hasScheme(urlScheme).hasHost(idpDomain).hasParameter("idpid").hasParameter("SAMLRequest");
120120
assertEquals(urlScheme, redirectUrl.getScheme());
121121
assertEquals(idpDomain, redirectUrl.getHost());

‎plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmdTest.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,15 +213,13 @@ public void testListAndSwitchSAMLAccountCmd() throws Exception {
213213
loginCmdResponse.set2FAenabled("false");
214214
Mockito.when(apiServer.loginUser(nullable(HttpSession.class), nullable(String.class), nullable(String.class),
215215
nullable(Long.class), nullable(String.class), nullable(InetAddress.class), nullable(Map.class))).thenReturn(loginCmdResponse);
216-
Mockito.doNothing().when(resp).sendRedirect(nullable(String.class));
217216
try {
218217
cmd.authenticate("command", params, session, null, HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp);
219218
} catch (ServerApiException exception) {
220219
fail("SAML list and switch account API failed to pass for all valid data: " + exception.getMessage());
221220
} finally {
222221
// accountService should have been called 4 times by now, for this case twice and 2 for cases above
223222
Mockito.verify(accountService, Mockito.times(4)).getUserAccountById(Mockito.anyLong());
224-
Mockito.verify(resp, Mockito.times(1)).sendRedirect(anyString());
225223
}
226224
}
227225

0 commit comments

Comments
 (0)