Skip to content

Commit f13cf59

Browse files
author
Rene Glover
authored
4.19 fix saml account selector (apache#10311)
1 parent 99ea77d commit f13cf59

File tree

13 files changed

+211
-61
lines changed

13 files changed

+211
-61
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
@@ -133,10 +133,12 @@ public String authenticate(final String command, final Map<String, Object[]> par
133133
}
134134

135135
if (userUuid != null && domainUuid != null) {
136+
s_logger.debug("User [" + currentUserAccount.getUsername() + "] is requesting to switch from user profile [" + currentUserAccount.getId() + "] to useraccount [" + userUuid + "] in domain [" + domainUuid + "]");
136137
final User user = _userDao.findByUuid(userUuid);
137138
final Domain domain = _domainDao.findByUuid(domainUuid);
138139
final UserAccount nextUserAccount = _accountService.getUserAccountById(user.getId());
139140
if (nextUserAccount != null && !nextUserAccount.getAccountState().equals(Account.State.ENABLED.toString())) {
141+
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 [" + nextUserAccount.getAccountName() + "] is not enabled");
140142
throw new ServerApiException(ApiErrorCode.ACCOUNT_ERROR, _apiServer.getSerializedApiError(ApiErrorCode.PARAM_ERROR.getHttpCode(),
141143
"The requested user account is locked and cannot be switched to, please contact your administrator.",
142144
params, responseType));
@@ -147,20 +149,26 @@ public String authenticate(final String command, final Map<String, Object[]> par
147149
|| !nextUserAccount.getExternalEntity().equals(currentUserAccount.getExternalEntity())
148150
|| (nextUserAccount.getDomainId() != domain.getId())
149151
|| (nextUserAccount.getSource() != User.Source.SAML2)) {
152+
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");
150153
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, _apiServer.getSerializedApiError(ApiErrorCode.PARAM_ERROR.getHttpCode(),
151154
"User account is not allowed to switch to the requested account",
152155
params, responseType));
153156
}
154157
try {
155158
if (_apiServer.verifyUser(nextUserAccount.getId())) {
159+
s_logger.info("User [" + currentUserAccount.getUsername() + "] user profile switch is accepted: from [" + currentUserId + "] to user profile [" + userUuid + "] in domain [" + domainUuid + "] with account [" + nextUserAccount.getAccountName() + "]");
160+
// 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)
161+
session.setAttribute("nextUserId", user.getId());
156162
final LoginCmdResponse loginResponse = (LoginCmdResponse) _apiServer.loginUser(session, nextUserAccount.getUsername(), nextUserAccount.getUsername() + nextUserAccount.getSource().toString(),
157163
nextUserAccount.getDomainId(), null, remoteAddress, params);
158164
SAMLUtils.setupSamlUserCookies(loginResponse, resp);
159-
resp.sendRedirect(SAML2AuthManager.SAMLCloudStackRedirectionUrl.value());
165+
session.removeAttribute("nextUserId");
166+
s_logger.debug("User [" + currentUserAccount.getUsername() + "] user profile switch cookies set: from [" + currentUserId + "] to user profile [" + userUuid + "] in domain [" + domainUuid + "] with account [" + nextUserAccount.getAccountName() + "]");
167+
//resp.sendRedirect(SAML2AuthManager.SAMLCloudStackRedirectionUrl.value());
160168
return ApiResponseSerializer.toSerializedString(loginResponse, responseType);
161169
}
162170
} catch (CloudAuthenticationException | IOException exception) {
163-
s_logger.debug("Failed to switch to request SAML user account due to: " + exception.getMessage());
171+
s_logger.debug("User [" + currentUserAccount.getUsername() + "] user profile switch cookies set FAILED: from [" + currentUserId + "] to user profile [" + userUuid + "] in domain [" + domainUuid + "] with account [" + nextUserAccount.getAccountName() + "]", exception);
164172
}
165173
} else {
166174
List<UserAccountVO> switchableAccounts = _userAccountDao.getAllUsersByNameAndEntity(currentUserAccount.getUsername(), currentUserAccount.getExternalEntity());
@@ -178,6 +186,9 @@ public String authenticate(final String command, final Map<String, Object[]> par
178186
accountResponse.setAccountName(userAccount.getAccountName());
179187
accountResponse.setIdpId(user.getExternalEntity());
180188
accountResponses.add(accountResponse);
189+
if (s_logger.isDebugEnabled()) {
190+
s_logger.debug("Returning available useraccount for [" + currentUserAccount.getUsername() + "]: UserUUID: [" + user.getUuid() + "], DomainUUID: [" + domain.getUuid() + "], Account: [" + userAccount.getAccountName() + "]");
191+
}
181192
}
182193
ListResponse<SamlUserAccountResponse> response = new ListResponse<SamlUserAccountResponse>();
183194
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
@@ -192,7 +192,7 @@ public String authenticate(final String command, final Map<String, Object[]> par
192192
String authnId = SAMLUtils.generateSecureRandomId();
193193
samlAuthManager.saveToken(authnId, domainPath, idpMetadata.getEntityId());
194194
s_logger.debug("Sending SAMLRequest id=" + authnId);
195-
String redirectUrl = SAMLUtils.buildAuthnRequestUrl(authnId, spMetadata, idpMetadata, SAML2AuthManager.SAMLSignatureAlgorithm.value());
195+
String redirectUrl = SAMLUtils.buildAuthnRequestUrl(authnId, spMetadata, idpMetadata, SAML2AuthManager.SAMLSignatureAlgorithm.value(), SAML2AuthManager.SAMLRequirePasswordLogin.value());
196196
resp.sendRedirect(redirectUrl);
197197
return "";
198198
} 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
@@ -543,6 +543,6 @@ public ConfigKey<?>[] getConfigKeys() {
543543
SAMLCloudStackRedirectionUrl, SAMLUserAttributeName,
544544
SAMLIdentityProviderMetadataURL, SAMLDefaultIdentityProviderId,
545545
SAMLSignatureAlgorithm, SAMLAppendDomainSuffix, SAMLTimeout, SAMLCheckSignature,
546-
SAMLForceAuthn, SAMLUserSessionKeyPathAttribute};
546+
SAMLForceAuthn, SAMLUserSessionKeyPathAttribute, SAMLRequirePasswordLogin};
547547
}
548548
}

‎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
@@ -151,11 +151,11 @@ public static String getValueFromAssertions(final List<Assertion> assertions, fi
151151
return null;
152152
}
153153

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

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

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

205197
return authnRequest;
206198
}
207199

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

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

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