Skip to content

Commit

Permalink
Addressed comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@Thisara-Welmilla
Thisara-Welmilla committed Oct 11, 2024
1 parent bcfc49e commit fb2b665
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 15 deletions.
7 changes: 6 additions & 1 deletion ...rbon/identity/api/server/authenticators/v1/core/ServerAuthenticatorManagementService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -422,14 +422,19 @@ private void addIdp(IdentityProvider identityProvider, List<Authenticator> authe
authenticator.setType(Authenticator.TypeEnum.FEDERATED);
authenticator.setImage(identityProvider.getImageUrl());
authenticator.setDescription(identityProvider.getIdentityProviderDescription());
// Only older existing IDP has multiple federated authenticator,

/* For the /authenticators APIs, per IDP have an item in the response payload, not per federated authenticator
within the IDP. If an IDP has more than one federated authenticator, it is considered as an existing older
authenticator and should always be classified as a SYSTEM type. Otherwise, it can be classified as either
SYSTEM or USER, depending on the 'definedBy' type of the federated authenticator. */
if (identityProvider.getFederatedAuthenticatorConfigs().length == 1) {
DefinedByType definedByType =
identityProvider.getFederatedAuthenticatorConfigs()[0].getDefinedByType();
authenticator.definedBy(Authenticator.DefinedByEnum.valueOf(definedByType.toString()));
} else {
authenticator.definedBy(Authenticator.DefinedByEnum.SYSTEM);
}

if (CollectionUtils.isNotEmpty(configTagsListDistinct)) {
authenticator.setTags(configTagsListDistinct);
}
Expand Down
40 changes: 26 additions & 14 deletions ...main/java/org/wso2/carbon/identity/api/server/idp/v1/core/ServerIdpManagementService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -674,8 +674,7 @@ public FederatedAuthenticatorListResponse updateFederatedAuthenticators(String i
// Need to create a clone, since modifying the fields of the original object, will modify the cached
// IDP object.
IdentityProvider idpToUpdate = createIdPClone(idp);
updateFederatedAuthenticatorConfig(idpToUpdate, authenticatorRequest);

updateFederatedAuthenticatorConfig(idpToUpdate, authenticatorRequest, false);
IdentityProvider updatedIdp = IdentityProviderServiceHolder.getIdentityProviderManager()
.updateIdPByResourceId(
idpId, idpToUpdate, ContextLoader.getTenantDomainFromContext());
Expand All @@ -684,7 +683,6 @@ public FederatedAuthenticatorListResponse updateFederatedAuthenticators(String i
throw handleIdPException(e, Constants.ErrorMessage.ERROR_CODE_ERROR_UPDATING_IDP, StringUtils.EMPTY);
}
}

/**
* Update federated authenticator of and IDP.
*
Expand All @@ -709,7 +707,7 @@ public FederatedAuthenticator updateFederatedAuthenticator(String idpId, String
IdentityProvider idpToUpdate = createIdPClone(idp);

// Create new FederatedAuthenticatorConfig to store the federated authenticator information.
FederatedAuthenticatorConfig authConfig = createFederatedAuthenticatorConfig(federatedAuthenticatorId,
FederatedAuthenticatorConfig authConfig = updateFederatedAuthenticatorConfig(federatedAuthenticatorId,
authenticator);
FederatedAuthenticatorConfig[] fedAuthConfigs = createFederatedAuthenticatorArrayClone
(federatedAuthenticatorId, idp.getFederatedAuthenticatorConfigs());
Expand Down Expand Up @@ -1760,7 +1758,7 @@ private MetaOutboundConnector createMetaOutboundConnector(ProvisioningConnectorC
}

private void updateFederatedAuthenticatorConfig(IdentityProvider idp, FederatedAuthenticatorRequest
federatedAuthenticatorRequest) {
federatedAuthenticatorRequest, boolean isNewFederatedAuthenticator) {

if (federatedAuthenticatorRequest != null) {
List<FederatedAuthenticator> federatedAuthenticators = federatedAuthenticatorRequest.getAuthenticators();
Expand All @@ -1772,7 +1770,8 @@ private void updateFederatedAuthenticatorConfig(IdentityProvider idp, FederatedA
authConfig.setName(base64URLDecode(authenticator.getAuthenticatorId()));
authConfig.setDisplayName(getDisplayNameOfAuthenticator(authConfig.getName()));
authConfig.setEnabled(authenticator.getIsEnabled());
authConfig.setDefinedByType(resolveDefinedByType(authConfig.getName()));
authConfig.setDefinedByType(resolveDefinedByType(authenticator.getName(),
authenticator.getDefinedBy().toString(), isNewFederatedAuthenticator));
List<org.wso2.carbon.identity.api.server.idp.v1.model.Property> authProperties =
authenticator.getProperties();
if (IdentityApplicationConstants.Authenticator.SAML2SSO.FED_AUTH_NAME.equals(authConfig.getName())) {
Expand Down Expand Up @@ -2112,7 +2111,7 @@ private IdentityProvider createIDP(IdentityProviderPOSTRequest identityProviderP
}
idp.setFederationHub(identityProviderPOSTRequest.getIsFederationHub());

updateFederatedAuthenticatorConfig(idp, identityProviderPOSTRequest.getFederatedAuthenticators());
updateFederatedAuthenticatorConfig(idp, identityProviderPOSTRequest.getFederatedAuthenticators(), true);
if (identityProviderPOSTRequest.getProvisioning() != null) {
updateOutboundConnectorConfig(idp, identityProviderPOSTRequest.getProvisioning().getOutboundConnectors());
updateJIT(idp, identityProviderPOSTRequest.getProvisioning().getJit());
Expand Down Expand Up @@ -2833,7 +2832,7 @@ private ProvisioningConnectorConfig createProvisioningConnectorConfig(String out
* @param authenticator Internal federated authenticator config.
* @return Federated authenticator config of the specified ID.
*/
private FederatedAuthenticatorConfig createFederatedAuthenticatorConfig(String federatedAuthenticatorId,
private FederatedAuthenticatorConfig updateFederatedAuthenticatorConfig(String federatedAuthenticatorId,
FederatedAuthenticatorPUTRequest
authenticator) {

Expand All @@ -2842,7 +2841,8 @@ private FederatedAuthenticatorConfig createFederatedAuthenticatorConfig(String f
authConfig.setName(authenticatorName);
authConfig.setDisplayName(getDisplayNameOfAuthenticator(authenticatorName));
authConfig.setEnabled(authenticator.getIsEnabled());
authConfig.setDefinedByType(resolveDefinedByType(authenticatorName));
authConfig.setDefinedByType(resolveDefinedByType(authenticator.getName(),
authenticator.getDefinedBy().toString(), false));
List<org.wso2.carbon.identity.api.server.idp.v1.model.Property> authProperties = authenticator.getProperties();
if (IdentityApplicationConstants.Authenticator.SAML2SSO.FED_AUTH_NAME.equals(authenticatorName)) {
validateSamlMetadata(authProperties);
Expand All @@ -2856,14 +2856,26 @@ private FederatedAuthenticatorConfig createFederatedAuthenticatorConfig(String f
return authConfig;
}

private DefinedByType resolveDefinedByType(String authenticatorName) {
private DefinedByType resolveDefinedByType(
String authenticatorName, String definedByType, boolean isNewFederatedAuthenticator) {

// Resolve definedBy type: If there is authenticator by same name and its type is system: SYSTEM. If not: USER.
/* For new federated authenticators:
If 'definedByType' is not null, use the value provided in the request payload. If not, default to SYSTEM. */
if (isNewFederatedAuthenticator) {
if (definedByType != null) {
return DefinedByType.valueOf(definedByType);
} else {
return DefinedByType.SYSTEM;
}
}
/* For existing federated authenticators, disregard any value provided in the request payload.
Instead, resolve and retrieve the 'definedBy' type of the corresponding existing authenticator.
If the authenticator config is present in the ApplicationAuthenticatorService list, return its type,
if not return USER. */
FederatedAuthenticatorConfig authenticatorConfig = ApplicationAuthenticatorService.getInstance()
.getFederatedAuthenticatorByName(authenticatorName);
if (authenticatorConfig != null &&
DefinedByType.SYSTEM.equals(authenticatorConfig.getDefinedByType())) {
return DefinedByType.SYSTEM;
if (authenticatorConfig != null) {
return DefinedByType.valueOf(authenticatorConfig.getDefinedByType().toString());
}
return DefinedByType.USER;
}
Expand Down

0 comments on commit fb2b665

Please sign in to comment.