[OPENAM-11987] SmsServerPropertiesResource removes password when unchanged. Created: 24/Oct/17  Updated: 01/Mar/18  Resolved: 25/Oct/17

Status: Resolved
Project: OpenAM
Component/s: rest
Affects Version/s: 13.5.0, 13.5.1, 14.0.0, 14.1.0, 14.1.1
Fix Version/s: 13.5.2, 14.5.1, 14.1.2

Type: Bug Priority: Major
Reporter: Sachiko Wallace Assignee: Sachiko Wallace
Resolution: Fixed Votes: 0
Labels: EDISON
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Sprint: AM Sustaining Sprint 44
Story Points: 2
Needs backport:
No
Support Ticket IDs:
Verified Version/s:
Needs QA verification:
Yes
Functional tests:
No
Are the reproduction steps defined?:
Yes and I used the same an in the description

 Description   

Bug description

SmsServerPropertiesResource removes password when unchanged.

How to reproduce the issue

1. run ldapsearch to check "org.forgerock.services.cts.store.password" exists :

 $./ldapsearch -p 51389 -D "cn=Directory Manager" -w cangetin -b ou=services,dc=openam,dc=forgerock,dc=org "objectclass=*" | grep "serverconfig=org.forgerock.services.cts.store.password" 

2. login to admin console
3. click [Configure] -> [Server Defaults] -> [Advanced] tab
4. scroll down and change "org.forgerock.services.cts.store.max.connections" to 11.
5. click [Save Changes]
6. run ldapsearch again to check "org.forgerock.services.cts.store.password" still exists :

 $./ldapsearch -p 51389 -D "cn=Directory Manager" -w cangetin -b ou=services,dc=openam,dc=forgerock,dc=org "objectclass=*" | grep "serverconfig=org.forgerock.services.cts.store.password" 

NOTE: if you've already configured session failover and CTS store per instance, then you might see more than one "org.forgerock.services.cts.store.password" so you need to check the value under server-default

Expected behaviour
SmsServerPropertiesResource shouldn't remove org.forgerock.services.cts.store.password when the value is null.
Current behaviour
SmsServerPropertiesResource marks org.forgerock.services.cts.store.password as unwanted and remove from server default

Work around

Always specify org.forgerock.services.cts.store.password when making a change on server default advanced tab.

Code analysis

Fix to OPENAM-11850 has made SmsServerPropertiesResource to remove "org.forgerock.services.cts.store.password" from "newValues" lists. This tricked removeUnusedAdvancedAttributes() method that "org.forgerock.services.cts.store.password" is no longer needed and therefore need to be removed.

The fix would be to skip removing attributes that are listed in "PASSWORD_ATTRIBUTES". The downside is that this way, users wouldn't be able to remove "PASSWORD_ATTRIBUTES" from XUI Advanced tab.

org.forgerock.openam.core.rest.sms.SmsServerPropertiesResource.java
    private void removeUnusedAdvancedAttributes(SSOToken token, Set<String> newAttributeNames, String serverName)
            throws SSOException, SMSException, InternalServerErrorException {

        ServiceConfig serviceConfig = getServerConfigs(token).getSubConfig(serverName);
        List<String> attributesToRemove = getAdvancedTabAttributeNames(serviceConfig);
        attributesToRemove.removeAll(newAttributeNames);
        attributesToRemove.removeAll(PASSWORD_ATTRIBUTES);  <--- FIX
        try {
            serverConfiguration.removeServerConfiguration(token, serverName, attributesToRemove);
        } catch (IOException e) {
            throw new InternalServerErrorException("Failed to remove server configuration", e);
        }
    }


 Comments   
Comment by Filip Kubáň [X] (Inactive) [ 01/Nov/17 ]

Verified on: OpenAM 13.5.2-M8 Build 1e79511bcb (2017-October-30 15:13)
unchanged password is not removed

Comment by Ľubomír Mlích [ 14/Nov/17 ]

Verified on OpenAM 14.1.2-M2 Build e8116f5a64 (2017-November-06 11:10) 

Generated at Tue Sep 22 10:52:41 UTC 2020 using Jira 7.13.12#713012-sha1:6e07c38070d5191bbf7353952ed38f111754533a.