Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new DefinedBy property to authenticator config. #5938

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

package org.wso2.carbon.identity.application.common;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.wso2.carbon.identity.application.common.model.FederatedAuthenticatorConfig;
import org.wso2.carbon.identity.application.common.model.LocalAuthenticatorConfig;
import org.wso2.carbon.identity.application.common.model.RequestPathAuthenticatorConfig;
Expand All @@ -31,6 +33,7 @@
public class ApplicationAuthenticatorService {

private static volatile ApplicationAuthenticatorService instance;
private static final Log LOG = LogFactory.getLog(ApplicationAuthenticatorService.class);

Check warning on line 36 in components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/ApplicationAuthenticatorService.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/ApplicationAuthenticatorService.java#L36

Added line #L36 was not covered by tests

private List<LocalAuthenticatorConfig> localAuthenticators = new ArrayList<>();
private List<FederatedAuthenticatorConfig> federatedAuthenticators = new ArrayList<>();
Expand Down Expand Up @@ -88,6 +91,9 @@

public void addLocalAuthenticator(LocalAuthenticatorConfig authenticator) {
if (authenticator != null) {
if (authenticator.getDefinedByType() == null) {
LOG.debug("The defined by type is not set for the : " + authenticator.getName());

Check warning on line 95 in components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/ApplicationAuthenticatorService.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/ApplicationAuthenticatorService.java#L95

Added line #L95 was not covered by tests
}
localAuthenticators.add(authenticator);
}
}
Expand All @@ -100,6 +106,9 @@

public void addFederatedAuthenticator(FederatedAuthenticatorConfig authenticator) {
if (authenticator != null) {
if (authenticator.getDefinedByType() == null) {
LOG.debug("The defined by type is not set for the : " + authenticator.getName());

Check warning on line 110 in components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/ApplicationAuthenticatorService.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/ApplicationAuthenticatorService.java#L110

Added line #L110 was not covered by tests
}
federatedAuthenticators.add(authenticator);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
import org.apache.axiom.om.OMElement;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.wso2.carbon.identity.base.IdentityConstants;

import java.io.Serializable;
import java.util.ArrayList;
Expand All @@ -46,6 +49,7 @@
public class FederatedAuthenticatorConfig implements Serializable {

private static final long serialVersionUID = -2361107623257323257L;
private static final Logger LOG = LoggerFactory.getLogger(LocalAuthenticatorConfig.class);

Check warning on line 52 in components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/FederatedAuthenticatorConfig.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/FederatedAuthenticatorConfig.java#L52

Added line #L52 was not covered by tests

@XmlElement(name = "Name")
protected String name;
Expand All @@ -63,6 +67,9 @@
@XmlElement(name = "Tags")
protected String[] tags;

@XmlElement(name = "DefinedBy")
protected IdentityConstants.DefinedByType definedByType;

public static FederatedAuthenticatorConfig build(OMElement federatedAuthenticatorConfigOM) {

if (federatedAuthenticatorConfigOM == null) {
Expand Down Expand Up @@ -101,9 +108,18 @@
Property[] propertiesArr = propertiesArrList.toArray(new Property[propertiesArrList.size()]);
federatedAuthenticatorConfig.setProperties(propertiesArr);
}
} else if ("DefinedBy".equals(elementName)) {
federatedAuthenticatorConfig.setDefinedByType(
IdentityConstants.DefinedByType.valueOf(element.getText()));

Check warning on line 113 in components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/FederatedAuthenticatorConfig.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/FederatedAuthenticatorConfig.java#L112-L113

Added lines #L112 - L113 were not covered by tests
}
}

// TODO: Remove warn log, once feature is ready.
if (federatedAuthenticatorConfig.getDefinedByType() == null) {
federatedAuthenticatorConfig.setDefinedByType(IdentityConstants.DefinedByType.SYSTEM);
LOG.debug("The defined by type is not set for the : " + federatedAuthenticatorConfig.getName());

Check warning on line 120 in components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/FederatedAuthenticatorConfig.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/FederatedAuthenticatorConfig.java#L119-L120

Added lines #L119 - L120 were not covered by tests
Thisara-Welmilla marked this conversation as resolved.
Show resolved Hide resolved
}

return federatedAuthenticatorConfig;
}

Expand Down Expand Up @@ -230,4 +246,24 @@

tags = tagList;
}

/**
* Get the tag list of the Local authenticator.
*
* @return String[]
*/
public IdentityConstants.DefinedByType getDefinedByType() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have this DefinedByType enum defined relative to application authenticator than using IdentityConstants

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot do this, as it cause circular dependency.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then still make sure this is related to authenticator configs. You can have a different class to denote constants related to authenticators


return definedByType;

Check warning on line 257 in components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/FederatedAuthenticatorConfig.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/FederatedAuthenticatorConfig.java#L257

Added line #L257 was not covered by tests
}

/**
* Set the tag list for Local authenticator config.
*
* @param type authenticator.
*/
public void setDefinedByType(IdentityConstants.DefinedByType type) {

definedByType = type;
}

Check warning on line 268 in components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/FederatedAuthenticatorConfig.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/FederatedAuthenticatorConfig.java#L267-L268

Added lines #L267 - L268 were not covered by tests
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
public class IdentityProvider implements Serializable {

private static final long serialVersionUID = 2199048941051702943L;
private static final Log LOG = LogFactory.getLog(IdentityProvider.class);

Check warning on line 61 in components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/IdentityProvider.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/IdentityProvider.java#L61

Added line #L61 was not covered by tests

private static final Log log = LogFactory.getLog(IdentityProvider.class);
private static final String FILE_ELEMENT_IDENTITY_PROVIDER_NAME = "IdentityProviderName";
Expand Down Expand Up @@ -419,6 +420,13 @@
if (federatedAuthenticatorConfigs == null) {
return;
}

// TODO: Remove warn log, once feature is ready.
for (FederatedAuthenticatorConfig config: federatedAuthenticatorConfigs) {
if (config.getDefinedByType() == null) {
LOG.debug("The defined by type is not set for the : " + config.getName());

Check warning on line 427 in components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/IdentityProvider.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/IdentityProvider.java#L427

Added line #L427 was not covered by tests
}
}
Set<FederatedAuthenticatorConfig> propertySet =
new HashSet<FederatedAuthenticatorConfig>(Arrays.asList(federatedAuthenticatorConfigs));
this.federatedAuthenticatorConfigs = propertySet.toArray(new FederatedAuthenticatorConfig[propertySet.size()]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import org.apache.axiom.om.OMElement;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.wso2.carbon.identity.base.IdentityConstants;

import java.io.Serializable;
Expand All @@ -46,6 +48,7 @@
public class LocalAuthenticatorConfig implements Serializable {

private static final long serialVersionUID = 3363298518257599291L;
private static final Logger LOG = LoggerFactory.getLogger(LocalAuthenticatorConfig.class);

Check warning on line 51 in components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/LocalAuthenticatorConfig.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/LocalAuthenticatorConfig.java#L51

Added line #L51 was not covered by tests

@XmlElement(name = "Name")
protected String name;
Expand All @@ -63,6 +66,9 @@
@XmlElement(name = "Tags")
protected String[] tags;

@XmlElement(name = "DefinedBy")
protected IdentityConstants.DefinedByType definedByType;

/*
* <LocalAuthenticatorConfig> <Name></Name> <DisplayName></DisplayName> <IsEnabled></IsEnabled>
* <Properties></Properties> </LocalAuthenticatorConfig>
Expand Down Expand Up @@ -111,8 +117,16 @@
Property[] propertiesArr = propertiesArrList.toArray(new Property[0]);
localAuthenticatorConfig.setProperties(propertiesArr);
}
} else if ("DefinedBy".equals(member.getLocalName())) {
localAuthenticatorConfig.setDefinedByType(IdentityConstants.DefinedByType.valueOf(member.getText()));

Check warning on line 121 in components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/LocalAuthenticatorConfig.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/LocalAuthenticatorConfig.java#L121

Added line #L121 was not covered by tests
}
}

if (localAuthenticatorConfig.getDefinedByType() == null) {
localAuthenticatorConfig.setDefinedByType(IdentityConstants.DefinedByType.SYSTEM);
LOG.debug("The defined by type is not set for the : " + localAuthenticatorConfig.getName());

Check warning on line 127 in components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/LocalAuthenticatorConfig.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/LocalAuthenticatorConfig.java#L126-L127

Added lines #L126 - L127 were not covered by tests
}

return localAuthenticatorConfig;
}

Expand Down Expand Up @@ -224,4 +238,24 @@

tags = tagList;
}

/**
* Get the tag list of the Local authenticator.
*
* @return String[]
*/
public IdentityConstants.DefinedByType getDefinedByType() {

return definedByType;

Check warning on line 249 in components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/LocalAuthenticatorConfig.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/LocalAuthenticatorConfig.java#L249

Added line #L249 was not covered by tests
}

/**
* Set the tag list for Local authenticator config.
*
* @param type authenticator.
*/
public void setDefinedByType(IdentityConstants.DefinedByType type) {

definedByType = type;
}

Check warning on line 260 in components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/LocalAuthenticatorConfig.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/model/LocalAuthenticatorConfig.java#L259-L260

Added lines #L259 - L260 were not covered by tests
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ private ApplicationConstants() {
public static final String IDP_NAME = "idpName";
public static final String IDP_AUTHENTICATOR_NAME = "authenticatorName";
public static final String IDP_AUTHENTICATOR_DISPLAY_NAME = "authenticatorDisplayName";
public static final String IDP_AUTHENTICATOR_DEFINED_BY_TYPE = "definedByType";
public static final String APPLICATION_DOMAIN = "Application";
// Regex for validating application name.
public static final String APP_NAME_VALIDATING_REGEX = "^[a-zA-Z0-9 ._-]*$";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
import org.wso2.carbon.identity.application.mgt.dao.PaginatableFilterableApplicationDAO;
import org.wso2.carbon.identity.application.mgt.internal.ApplicationManagementServiceComponent;
import org.wso2.carbon.identity.application.mgt.internal.ApplicationManagementServiceComponentHolder;
import org.wso2.carbon.identity.base.IdentityConstants;
Thisara-Welmilla marked this conversation as resolved.
Show resolved Hide resolved
import org.wso2.carbon.identity.base.IdentityException;
import org.wso2.carbon.identity.base.IdentityRuntimeException;
import org.wso2.carbon.identity.core.CertificateRetrievingException;
Expand Down Expand Up @@ -1566,6 +1567,15 @@
ApplicationConstants.LOCAL_IDP_NAME,
lclAuthenticator.getName(),
lclAuthenticator.getDisplayName());
} else {
Thisara-Welmilla marked this conversation as resolved.
Show resolved Hide resolved
if (lclAuthenticator.getDefinedByType() == null) {
log.debug("Authenticator already exists. Updating the authenticator, but the " +

Check warning on line 1572 in components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.java#L1572

Added line #L1572 was not covered by tests
"defined by type is not set.");
} else {
log.debug("Authenticator already exists. Updating the authenticator.The defined "
+ "by type is set to: " + lclAuthenticator.getDefinedByType().toString());

Check warning on line 1576 in components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.java#L1575-L1576

Added lines #L1575 - L1576 were not covered by tests
//TODO: Update database with defined by properties for local authenticators.
}
}
if (authenticatorId > 0) {
// ID, TENANT_ID, AUTHENTICATOR_ID
Expand Down Expand Up @@ -3088,6 +3098,8 @@
.get(ApplicationConstants.IDP_AUTHENTICATOR_NAME));
localAuthenticator.setDisplayName(authenticatorInfo
.get(ApplicationConstants.IDP_AUTHENTICATOR_DISPLAY_NAME));
localAuthenticator.setDefinedByType(IdentityConstants.DefinedByType.valueOf(
authenticatorInfo.get(ApplicationConstants.IDP_AUTHENTICATOR_DEFINED_BY_TYPE)));

Check warning on line 3102 in components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.java

View check run for this annotation

Codecov / codecov/patch

components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.java#L3101-L3102

Added lines #L3101 - L3102 were not covered by tests
stepLocalAuth.get(step).add(localAuthenticator);
} else {
Map<String, List<FederatedAuthenticatorConfig>> stepFedIdps = stepFedIdPAuthenticators
Expand All @@ -3106,6 +3118,8 @@
.get(ApplicationConstants.IDP_AUTHENTICATOR_NAME));
fedAuthenticator.setDisplayName(authenticatorInfo
.get(ApplicationConstants.IDP_AUTHENTICATOR_DISPLAY_NAME));
fedAuthenticator.setDefinedByType(IdentityConstants.DefinedByType.valueOf(
authenticatorInfo.get(ApplicationConstants.IDP_AUTHENTICATOR_DEFINED_BY_TYPE)));
idpAuths.add(fedAuthenticator);
}

Expand Down Expand Up @@ -5017,6 +5031,9 @@
returnData
.put(ApplicationConstants.IDP_AUTHENTICATOR_DISPLAY_NAME, rs.getString(3));
}
// TODO: Read from database and set the DefinedBy property to the authenticator.
returnData.put(ApplicationConstants.IDP_AUTHENTICATOR_DEFINED_BY_TYPE,
IdentityConstants.DefinedByType.SYSTEM.toString());
} finally {
IdentityApplicationManagementUtil.closeStatement(prepStmt);
}
Expand All @@ -5038,7 +5055,7 @@
int authenticatorId = -1;
PreparedStatement prepStmt = null;
ResultSet rs = null;
// TENANT_ID, IDP_ID, NAME,IS_ENABLED, DISPLAY_NAME
// TENANT_ID, IDP_ID, NAME,IS_ENABLED, DISPLAY_NAME, DEFINED_BY
String sqlStmt = ApplicationMgtDBQueries.STORE_LOCAL_AUTHENTICATOR;
try {
String dbProductName = conn.getMetaData().getDatabaseProductName();
Expand All @@ -5050,6 +5067,7 @@
prepStmt.setString(4, authenticatorName);
prepStmt.setString(5, "1");
prepStmt.setString(6, authenticatorDispalyName);
//TODO: prepStmt.setString(7, IdentityConstants.DefinedByType.SYSTEM.toString());
prepStmt.execute();
rs = prepStmt.getGeneratedKeys();
if (rs.next()) {
Expand Down
Loading
Loading