-
Notifications
You must be signed in to change notification settings - Fork 540
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
base: master
Are you sure you want to change the base?
Changes from 4 commits
b40fb77
6ab1997
1a9b323
e7bbfc2
25ef6ea
d4a99a1
c9166db
ef7779c
1404a70
aa39c91
a8f8170
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -46,6 +49,7 @@ | |
public class FederatedAuthenticatorConfig implements Serializable { | ||
|
||
private static final long serialVersionUID = -2361107623257323257L; | ||
private static final Logger LOG = LoggerFactory.getLogger(LocalAuthenticatorConfig.class); | ||
|
||
@XmlElement(name = "Name") | ||
protected String name; | ||
|
@@ -63,6 +67,9 @@ public class FederatedAuthenticatorConfig implements Serializable { | |
@XmlElement(name = "Tags") | ||
protected String[] tags; | ||
|
||
@XmlElement(name = "DefinedBy") | ||
protected IdentityConstants.DefinedByType definedByType; | ||
|
||
public static FederatedAuthenticatorConfig build(OMElement federatedAuthenticatorConfigOM) { | ||
|
||
if (federatedAuthenticatorConfigOM == null) { | ||
|
@@ -101,9 +108,18 @@ public static FederatedAuthenticatorConfig build(OMElement federatedAuthenticato | |
Property[] propertiesArr = propertiesArrList.toArray(new Property[propertiesArrList.size()]); | ||
federatedAuthenticatorConfig.setProperties(propertiesArr); | ||
} | ||
} else if ("DefinedBy".equals(elementName)) { | ||
federatedAuthenticatorConfig.setDefinedByType( | ||
IdentityConstants.DefinedByType.valueOf(element.getText())); | ||
} | ||
} | ||
|
||
// TODO: Remove warn log, once feature is ready. | ||
if (federatedAuthenticatorConfig.getDefinedByType() == null) { | ||
federatedAuthenticatorConfig.setDefinedByType(IdentityConstants.DefinedByType.SYSTEM); | ||
LOG.warn("The defined by type is not set for the : " + federatedAuthenticatorConfig.getName()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's have a debug log here rather than a warn. Also, let's indicate that 'system' is set by default |
||
} | ||
|
||
return federatedAuthenticatorConfig; | ||
} | ||
|
||
|
@@ -230,4 +246,24 @@ public void setTags(String[] tagList) { | |
|
||
tags = tagList; | ||
} | ||
|
||
/** | ||
* Get the tag list of the Local authenticator. | ||
* | ||
* @return String[] | ||
*/ | ||
public IdentityConstants.DefinedByType getDefinedByType() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We cannot do this, as it cause circular dependency. |
||
|
||
return definedByType; | ||
} | ||
|
||
/** | ||
* Set the tag list for Local authenticator config. | ||
* | ||
* @param type authenticator. | ||
*/ | ||
public void setDefinedByType(IdentityConstants.DefinedByType type) { | ||
|
||
definedByType = type; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,6 +58,7 @@ | |
public class IdentityProvider implements Serializable { | ||
|
||
private static final long serialVersionUID = 2199048941051702943L; | ||
private static final Log LOG = LogFactory.getLog(IdentityProvider.class); | ||
|
||
private static final Log log = LogFactory.getLog(IdentityProvider.class); | ||
private static final String FILE_ELEMENT_IDENTITY_PROVIDER_NAME = "IdentityProviderName"; | ||
|
@@ -419,6 +420,13 @@ public void setFederatedAuthenticatorConfigs( | |
if (federatedAuthenticatorConfigs == null) { | ||
return; | ||
} | ||
|
||
// TODO: Remove warn log, once feature is ready. | ||
for (FederatedAuthenticatorConfig config: federatedAuthenticatorConfigs) { | ||
if (config.getDefinedByType() == null) { | ||
LOG.warn("The defined by type is not set for the : " + config.getName()); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment above applies here |
||
Set<FederatedAuthenticatorConfig> propertySet = | ||
new HashSet<FederatedAuthenticatorConfig>(Arrays.asList(federatedAuthenticatorConfigs)); | ||
this.federatedAuthenticatorConfigs = propertySet.toArray(new FederatedAuthenticatorConfig[propertySet.size()]); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, IMO this is not a good practice to follow.
We clearly know that this can be null for exisitng authenticators, so we should move to the default which is system