Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@sadilchamishka
sadilchamishka committed Nov 14, 2023
1 parent 66308e8 commit 845b602
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 18 deletions.
3 changes: 2 additions & 1 deletion components/org.wso2.carbon.identity.application.authenticator.samlsso/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,8 @@
org.wso2.carbon.user.core.service; version="${carbon.kernel.imp.pkg.version.range}",
org.wso2.carbon.user.core.tenant; version="${carbon.kernel.imp.pkg.version.range}",
org.wso2.carbon.utils; version="${carbon.kernel.imp.pkg.version.range}",
org.wso2.carbon.identity.organization.management.service; version="${org.wso2.identity.organization.mgt.core.imp.pkg.version.range}",
org.wso2.carbon.identity.organization.management.service.util; version="${org.wso2.identity.organization.mgt.core.imp.pkg.version.range}",
org.wso2.carbon.identity.organization.management.service.exception; version="${org.wso2.identity.organization.mgt.core.imp.pkg.version.range}",
org.osgi.service.component.*;version="${imp.package.version.osgi.services}",
org.xml.sax
</Import-Package>
Expand Down
3 changes: 1 addition & 2 deletions ...ity/application/authenticator/samlsso/internal/SAMLSSOAuthenticatorServiceDataHolder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ public OrganizationManager getOrganizationManager() {
return organizationManager;
}

public void setOrganizationManager(
OrganizationManager organizationManager) {
public void setOrganizationManager(OrganizationManager organizationManager) {

this.organizationManager = organizationManager;
}
Expand Down
54 changes: 40 additions & 14 deletions ...so2/carbon/identity/application/authenticator/samlsso/manager/DefaultSAML2SSOManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@
import org.w3c.dom.Element;
import org.w3c.dom.NodeList;
import org.wso2.carbon.base.MultitenantConstants;
import org.wso2.carbon.context.PrivilegedCarbonContext;
import org.wso2.carbon.identity.application.authentication.framework.config.builder.FileBasedConfigurationBuilder;
import org.wso2.carbon.identity.application.authentication.framework.config.model.AuthenticatorConfig;
import org.wso2.carbon.identity.application.authentication.framework.context.AuthenticationContext;
Expand All @@ -105,6 +104,8 @@
import org.wso2.carbon.identity.core.ServiceURLBuilder;
import org.wso2.carbon.identity.core.URLBuilderException;
import org.wso2.carbon.identity.core.util.IdentityUtil;
import org.wso2.carbon.identity.organization.management.service.exception.OrganizationManagementException;
import org.wso2.carbon.identity.organization.management.service.util.OrganizationManagementUtil;
import org.wso2.carbon.identity.saml.common.util.SAMLInitializer;
import org.wso2.carbon.user.api.UserRealm;
import org.wso2.carbon.user.api.UserStoreException;
Expand Down Expand Up @@ -253,8 +254,9 @@ public String buildRequest(HttpServletRequest request, boolean isLogout, boolean
SSOUtils.addSignatureToHTTPQueryString(httpQueryString, signatureAlgo,
new X509CredentialImpl(MultitenantConstants.SUPER_TENANT_DOMAIN_NAME, null));
} else {
String tenantDomain = getSigningTenantDomain(context.getTenantDomain());
SSOUtils.addSignatureToHTTPQueryString(httpQueryString, signatureAlgo,
new X509CredentialImpl(context.getTenantDomain(), null));
new X509CredentialImpl(tenantDomain, null));
}
}
if (loginPage.indexOf("?") > -1) {
Expand Down Expand Up @@ -310,8 +312,9 @@ public String buildPostRequest(HttpServletRequest request, boolean isLogout,
if (!isLogout) {
requestMessage = buildAuthnRequest(request, isPassive, loginPage, context);
if (SSOUtils.isAuthnRequestSigned(properties)) {
String tenantDomain = getSigningTenantDomain(context.getTenantDomain());
SSOUtils.setSignature(requestMessage, signatureAlgo, digestAlgo, includeCert,
new X509CredentialImpl(context.getTenantDomain(), null));
new X509CredentialImpl(tenantDomain, null));
}
} else {
String username = (String) request.getSession().getAttribute(SSOConstants.LOGOUT_USERNAME);
Expand All @@ -323,8 +326,9 @@ public String buildPostRequest(HttpServletRequest request, boolean isLogout,
requestMessage = buildLogoutRequest(username, sessionIndex, loginPage, nameQualifier, spNameQualifier,
nameIdFormat, context);
if (SSOUtils.isLogoutRequestSigned(properties)) {
String tenantDomain = getSigningTenantDomain(context.getTenantDomain());
SSOUtils.setSignature(requestMessage, signatureAlgo, digestAlgo, includeCert,
new X509CredentialImpl(context.getTenantDomain(), null));
new X509CredentialImpl(tenantDomain, null));
}
}

Expand Down Expand Up @@ -1295,16 +1299,7 @@ protected void validateAssertionValidityPeriod(Assertion assertion) throws SAMLS
*/
protected Assertion getDecryptedAssertion(EncryptedAssertion encryptedAssertion) throws Exception {

String tenantDomain = this.tenantDomain;
/* When assertion decryption request is initiated by an organization, the key of the corresponding root tenant
should be used. */
String organizationId = PrivilegedCarbonContext.getThreadLocalCarbonContext().getOrganizationId();
if (StringUtils.isNotEmpty(organizationId)) {
String rootOrganizationId = SAMLSSOAuthenticatorServiceDataHolder.getInstance().getOrganizationManager()
.getPrimaryOrganizationId(organizationId);
tenantDomain = SAMLSSOAuthenticatorServiceDataHolder.getInstance().getOrganizationManager()
.resolveTenantDomain(rootOrganizationId);
}
String tenantDomain = getSigningTenantDomain(this.tenantDomain);
X509Credential credential = new X509CredentialImpl(tenantDomain, null);
KeyInfoCredentialResolver keyResolver = new StaticKeyInfoCredentialResolver(credential);
EncryptedKey key = getEncryptedKey(encryptedAssertion);
Expand Down Expand Up @@ -1385,4 +1380,35 @@ private EncryptedKey getEncryptedKey(EncryptedAssertion encryptedAssertion) thro
}
throw new Exception("Could not obtain the encrypted key from the encrypted assertion.");
}

/**
* Resolve the tenant domain which is used to sign the request/response.
*
* @param tenantDomain The tenant domain from the context.
* @return The tenant domain which is used to sign the request/response.
*/
private String getSigningTenantDomain(String tenantDomain) {

boolean isOrganization;
try {
isOrganization = OrganizationManagementUtil.isOrganization(tenantDomain);
} catch (OrganizationManagementException e) {
log.error("Error while checking the tenant domain is related to an organization", e);
return tenantDomain;
}
if (!isOrganization) {
return tenantDomain;
}
try {
String organizationId = SAMLSSOAuthenticatorServiceDataHolder.getInstance().getOrganizationManager()
.resolveOrganizationId(tenantDomain);
String rootOrganizationId = SAMLSSOAuthenticatorServiceDataHolder.getInstance().getOrganizationManager()
.getPrimaryOrganizationId(organizationId);
return SAMLSSOAuthenticatorServiceDataHolder.getInstance().getOrganizationManager()
.resolveTenantDomain(rootOrganizationId);
} catch (OrganizationManagementException e) {
log.error("Error while resolving tenant domain for organization id: ", e);
return tenantDomain;
}
}
}
6 changes: 5 additions & 1 deletion ...carbon/identity/application/authenticator/samlsso/manager/DefaultSAML2SSOManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import org.wso2.carbon.identity.application.common.util.IdentityApplicationConstants;
import org.wso2.carbon.identity.core.ServiceURLBuilder;
import org.wso2.carbon.identity.core.util.IdentityUtil;
import org.wso2.carbon.identity.organization.management.service.util.OrganizationManagementUtil;
import org.wso2.carbon.user.core.service.RealmService;
import org.wso2.carbon.user.core.tenant.TenantManager;

Expand All @@ -70,6 +71,7 @@
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.xpath.XPathFactory;

import static org.mockito.ArgumentMatchers.anyString;
import static org.powermock.api.mockito.PowerMockito.doNothing;
import static org.powermock.api.mockito.PowerMockito.mockStatic;
import static org.powermock.api.mockito.PowerMockito.when;
Expand Down Expand Up @@ -125,7 +127,7 @@
@PowerMockIgnore({"javax.xml.datatype.*","org.mockito.*","org.powermock.api.mockito.invocation.*","javax.crypto.Cipher"})
@PrepareForTest({FileBasedConfigurationBuilder.class, IdentityUtil.class, DocumentBuilderFactory.class,
KeyStoreManager.class, DOMImplementationRegistry.class, XPathFactory.class, FrameworkUtils.class,
ServiceURLBuilder.class})
ServiceURLBuilder.class, OrganizationManagementUtil.class})
public class DefaultSAML2SSOManagerTest {

@Mock
Expand Down Expand Up @@ -512,6 +514,8 @@ public Object[][] postRequestBuilderData() {
public void buildPostRequest(boolean isLogout, String tenantDomain, Object inboundRequestData,
Object outboundRequestData) throws Exception {

mockStatic(OrganizationManagementUtil.class);
when(OrganizationManagementUtil.isOrganization(anyString())).thenReturn(false);
mockStatic(FrameworkUtils.class);
doNothing().when(FrameworkUtils.class, TestConstants.END_TENANT_FLOW);

Expand Down

0 comments on commit 845b602

Please sign in to comment.