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

OAuth2 Provider Service REST defaultACR input data not validated.

    XMLWordPrintable

    Details

    • AM Sustaining Sprint 55
    • 3
    • No
    • Yes
    • No
    • Yes and I used the same an in the description

      Description

      Bug description

      Incorrect OAuth2Provider defaultACR values are not validated and cause later issues like OPENAM-13496. (-OPENAM-8886- made the changes to be a list)

      OPENAM-13496 covers the generic issue where what bad values had happens and it is hard to tell diagnose what these attributes are. This bug is created so we avoid attributes values that can easily go wrong moving forward and making at least the OAuth2Provider does include validation.

      How to reproduce the issue

      1. Create a new OAuth2Provider service in some realm /test
      2. Now try to see what REST call is sent when the default ACR is set on the Advance OIDC tab is done. You can copy the "Curl" used in the Chrome Dev tool when the REST is update
      3. Modify the REST call for '"defaultACR": "abc"' from a list to a string.
      4. Notice that the REST update works but it causes issue in OPENAM-13496
      Expected behaviour
      This should have validation failure and the REST update fail
      Current behaviour
      This values goes in and causes REST query later on.
      

      Work around

      Make sure no errornoeus REST values

      Code analysis

      Notice that the ListValueValidator schema is not added to OAuth2Provider.xml (compared to AgentService.xml) and also that this schema have MapValueValidator there. So it seems that this may be a missing schema (made worst by the introduction of a attribute (defaultACR) that now is a list type. So add this to the OAuth2Provider.xml:
       

      OAuth2Provider.xml
      ....
                          <AttributeSchema name="ListValueValidator"  syntax="string"
       type="validator" >
                              <DefaultValues>
                                  <Value>com.sun.identity.common.configuration.ListValueValidator</Value>
                              </DefaultValues>
                          </AttributeSchema>
      

      To cater for migration again!!!:-

      public class UpgradeOAuth2ProviderStep extends AbstractUpgradeStep {
           private boolean shouldUpgradeDefaultAcrValues(Map<String, Set<String>> attributes) {
               boolean hasSingleValue = attributes.get(DEFAULT_ACR).size() == 1;
      -        boolean isSpaceSeparatedValue = hasSingleValue && attributes.get(DEFAULT_ACR).iterator().next().split(" ").length > 1;
      -        return hasSingleValue && isSpaceSeparatedValue;
      +        boolean isSpaceSeparatedValue = false;
      +        boolean isNotAList = false;
      +        if (hasSingleValue) {
      +            String values = attributes.get(DEFAULT_ACR).iterator().next();
      +            isSpaceSeparatedValue = values.split(" ").length > 1;
      +            isNotAList = !(values.trim().startsWith("["));
      +        }
      +        return hasSingleValue && ( isSpaceSeparatedValue || isNotAList );
           }
      

        Attachments

          Issue Links

            Activity

              People

              chee-weng.chea C-Weng C
              chee-weng.chea C-Weng C
              Filip Kubáň [X] Filip Kubáň [X] (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: