Implement identity mapping CN with prefix and suffix

[jmrt47]

Implement a class to map identities from client certificate CN with customizable and optional prefix and suffix. Format of Identity returned is <prefix><CN><suffix>.

The mapper can be configured by existing property: provider.serverCertIdentityMap.classList

Prefix and suffix can be optionally specified using the properties:

provider.clientCertIdentityMap.customized.prefix
provider.clientCertIdentityMap.customized.suffix

Default value is an empty string in both cases.

[guusdk]

Thanks for your contribution! I have two major suggestions:

• Instead of JiveGlobals use SystemProperty - it offers more flexibility.
• Instead of creating a new class, add this functionality as an optional feature to the parent class.

It might be a good idea to code this in such a way that a change of the configured values does not require an Openfire restart to take effect.

[jmrt47]

@guusdk thanks for your valuable feedback and suggestions. I applied the following changes:

• Use SystemProperty instead of JiveGlobals
• Changes of property values are applied without restart of openfire being require
• Extension of CNCertificateIdentityMapping class with optional feature instead of new class
• Changed the property names to the following:

provider.clientCertIdentityMap.cn.prefix
provider.clientCertIdentityMap.cn.suffix

[guusdk]

Great, thanks! I suggest that you make these new properties as constants in the class, like this:

public static final SystemProperty<String> CN_PREFIX = SystemProperty.Builder.ofType(String.class)
    .setKey("provider.clientCertIdentityMap.cn.prefix")
    .setDefaultValue("")
    .setDynamic(true)
    .build();

public static final SystemProperty<String> CN_SUFFIX = SystemProperty.Builder.ofType(String.class)
    .setKey("provider.clientCertIdentityMap.cn.suffix")
    .setDefaultValue("")
    .setDynamic(true)
    .build();

You can then remove the cnPrefix and cnSuffix fields. The properties can be used directly in your code, like so:

names.add(CN_PREFIX.getValue() + matcher.group(2) + CN_SUFFIX.getValue());

To make your new properties self-documenting, please add a line in openfire_i18n.properties (and maybe a translation file in your native language) that describes each property. The line should start with system_property. followed by the property key, like this:

system_property.provider.clientCertIdentityMap.cn.prefix=DESCRIPTION OF THE PROPERTY
system_property.provider.clientCertIdentityMap.cn.suffix=DESCRIPTION OF THE PROPERTY

[jmrt47]

Thanks again for your review and input. I took it into consideration and while checking the code I noticed, that the change would cause addind a prefix and suffix for client and server CN identitiess simutaniously. This shouldn't be the case, which is why I change it to:

• Separate CN Identity mapping in one class for server and one for client which are than used by default
• Refactored serverCertIdentityMap and clientCertIdentityMap class configuration to use SystemProperty
• Make pattern for CN parsing configurable by SystemProperty for client and server identities
• Documented all properties in EN and DE

In case this change has a too big impact, I like to propose to go back to the initial Idea by adding a customazabile CN Parsing class which than can be configured using provider.serverCertIdentityMap.classList or provider.clientCertIdentityMap.classList.

[Fishbowler]

Sorry this has been left hanging for a while.

What's the real-world use-case for the change? A person who comes in identifying by certificate as "alice" gets mapped to an Openfire username of "user-alice-staffmember"? Or have I gotten that backwards?

[jmrt47]

You got it right, the use-case is that a username "user-alice-staffmember" is stored in database but the certificate used for authentication just has alice in the CN field. The basic idea is to get a configurable implementation of CNCertificateIdentityMapping. If this is a valid acceptable change request, I like to propose two ideas on how to move on:

1. Revert most changes and just add one configurable extended CNCertificateIdentityMapping class which is not used by default. In case someone requires a more flexible implementation it can be set using provider.clientCertIdentityMap.classList.
2. Change CertificateManager to allow changes to field serverCertMapping and field clientCertMapping on runtime. This way a custom implementation can easily be added using a Plugin.

[Fishbowler]

Did you mean to close this?

[jmrt47]

Yes, I think this PR isn't the right approach to address this feature need.