Uploaded image for project: 'OpenAM'
  1. OpenAM
  2. OPENAM-11987

SmsServerPropertiesResource removes password when unchanged.

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • 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
    • Component/s: rest
    • Labels:
    • 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);
              }
          }
      

        Attachments

          Activity

            People

            • Assignee:
              sachiko Sachiko Wallace
              Reporter:
              sachiko Sachiko Wallace
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: