[OPENAM-5417] Policy Conditions of same type can not be combined in OpenAM 12 Created: 15/Jan/15  Updated: 20/Nov/16  Resolved: 20/Nov/16

Status: Closed
Project: OpenAM
Component/s: policy, policy editor
Affects Version/s: 12.0.0
Fix Version/s: 12.0.1, 13.0.0

Type: Bug Priority: Major
Reporter: Nathalie Hoet Assignee: Charles Sparey
Resolution: Fixed Votes: 0
Labels: EDISON, release-notes, verified
Remaining Estimate: 0h
Time Spent: 36h
Original Estimate: 0h

Issue Links:
Duplicate
is duplicated by OPENAM-5633 Can't create Environment Condition wi... Resolved
is duplicated by OPENAM-5601 Defining environment conditions remov... Resolved
Sprint: Sprint 76 - Sustaining
Support Ticket IDs:
QA Assignee: Alex Walker [X] (Inactive)
Verified Version/s:

 Description   

In Policy editor, create a policy and add two conditions of the same type with a logical operator (either OR or AND, neither work).

For example take a simpleTime condition. Access allowed on (Monday to Tuesday) OR on (Thursday to Friday).

You can see both conditions in the editor when you create them.
When you save the policy however, it removes the second condition and when you open the policy again it is not in the editor.
Checking in the Configuration store shows that it is not saved there properly either.



 Comments   
Comment by Andrew Forrest [ 15/Jan/15 ]

Creating two subject conditions of the same type appears to be fine.

Comment by Andrew Forrest [ 15/Jan/15 ]

This appears to only be an issue with environment conditions.

Comment by Andrew Forrest [ 15/Jan/15 ]

Same issue with REST endpoint:

{
   "description":"My policy",
   "resources":[
      "http://www.example.com:80/admin/*"
   ],
   "applicationName":"iPlanetAMWebAgentService",
   "actionValues":{
      "UPDATE":true,
      "CREATE":true,
      "READ":true
   },
   "subject":{
      "type":"Identity",
      "subjectValues":[
         "amAdmin"
      ]
   },
   "condition":{
      "type":"AND",
      "conditions":[
         {
            "type":"OAuth2Scope",
            "requiredScopes":[
               "test"
            ]
         },
         {
            "type":"OAuth2Scope",
            "requiredScopes":[
               "hello"
            ]
         }
      ]
   },
   "resourceAttributes":[
      {
         "type":"User",
         "propertyName":"cn"
      },
      {
         "type":"Static",
         "propertyName":"colour",
         "propertyValues":[
            "green"
         ]
      }
   ]
}

Response:

{
   "name":"MyPolicy4",
   "active":true,
   "description":"My policy",
   "resources":[
      "http://www.example.com:80/admin/*"
   ],
   "applicationName":"iPlanetAMWebAgentService",
   "actionValues":{
      "UPDATE":true,
      "CREATE":true,
      "READ":true
   },
   "subject":{
      "type":"Identity",
      "subjectValues":[
         "amAdmin"
      ]
   },
   "condition":{
      "type":"AND",
      "conditions":[
         {
            "type":"OAuth2Scope",
            "requiredScopes":[
               "test"
            ]
         }
      ]
   },
   "resourceAttributes":[
      {
         "type":"User",
         "propertyName":"cn",
         "propertyValues":[

         ]
      },
      {
         "type":"Static",
         "propertyName":"colour",
         "propertyValues":[
            "green"
         ]
      }
   ],
   "lastModifiedBy":"id=amadmin,ou=user,dc=openam,dc=forgerock,dc=org",
   "lastModifiedDate":"2015-01-15T11:40:12.768Z",
   "createdBy":"id=amadmin,ou=user,dc=openam,dc=forgerock,dc=org",
   "creationDate":"2015-01-15T11:40:12.768Z"
}
Comment by Andrew Forrest [ 15/Jan/15 ]

The above demonstrates that this isn't a UI issue and therefore it's something occurring on the server.

Comment by Andrew Forrest [ 15/Jan/15 ]

The scenario is confined to two environment conditions of the same type and appears to be occurring during the parsing of the incoming json into a policy instance.

Comment by Andrew Forrest [ 15/Jan/15 ]

So the issue is occurring during parsing of the JSON data by the jackson framework (namely org.codehaus.jackson.map.deser.std.CollectionDeserializer#deserialize(JsonParser, DeserializationContext, Collection<Object>):decompiled line 221).

The issue here is that deserialised objects are added to a set. For subject conditions the equals object hasn't been implemented, so each new instance of a subject condition will not match. However, for environment conditions there is a common super class (EntitlementConditionAdaptor) and this overrides the equals method which simply compares the conditions types. Therefore, when adding environment conditions to the set, if a condition already exists with the same type value, it will be ignored.

A fix to this problem is to investigate the use of the environment conditions equals methods, and if safe, add some further equality logic so to include condition state in the comparison.

Comment by Andrew Forrest [ 15/Jan/15 ]

The set that is being populated in this scenario boils down to the sets specified under the LogicalXXX classes (LogicalSubject and LogicalCondition). Another fix could be to change these from sets to lists. However, fixing up the equals method is the favourable solution.

Comment by Charles Sparey [ 30/Jan/15 ]

Yes, having examined this in detail, the missing code is in the equals methods (that are missing) in the classes derived from EntitlementConditionAdapter. Most derived classes have equality overrides that test for additional properties, but the faulty ones - such as OAuth2ScopeCondition.java do not. The fix, is to add new equality overrides to the derived classes that do not have them, comparing the additional properties at that level (all the way to the most derived child).

Comment by Charles Sparey [ 30/Jan/15 ]

Specific level 2 classes that need modification are:
AMIdentityMembershipCondition, AuthenticateToRealmCondition, AuthenticateToServiceCondition, AuthLevelCondition, AuthSchemeCondition, IPvXCondition, LDAPFilterCondition, OAuth2ScopeCondition, ResourceEnvIPCondition [This class has slightly different structure to the others so will need additional evaluation], SessionCondition, SessionPropertyCondition and SimpleTimeCondition. In the current version all these use the base class equality method only - which will fail (causing the problem as described in the bug).

Comment by Charles Sparey [ 05/Feb/15 ]

12.0.1 version checked in as revision 12351 in the 12.0.x branch.

Comment by Charles Sparey [ 05/Feb/15 ]

13.0.0 version checked in as revision 12354 into the trunk.

Comment by Charles Sparey [ 05/Feb/15 ]

Marking as resolved as per the previous comments. Total time spent in this sprint on this issue for both the 12&13x versions as well as testing and code reviews was ~36hrs.

Comment by Alex Walker [X] (Inactive) [ 06/Feb/15 ]

Bug verified in OpenAM 13.0.0-SNAPSHOT Build 12373 (2015-February-06 02:57), closing.

Comment by Richard Hruza [ 14/Jul/15 ]

Verified and Closed

Tested with: OpenAM 12.0.1 Build 14322 (2015-June-22 16:03)

Comment by Quentin CASTEL [X] (Inactive) [ 20/Nov/16 ]

modification of the status, in order to migrate the 'Zendesk ID' field to 'Support Ticket ID' field.

Comment by Quentin CASTEL [X] (Inactive) [ 20/Nov/16 ]

modification of the status, in order to migrate the 'Zendesk ID' field to 'Support Ticket ID' field.

Generated at Tue Oct 27 03:38:01 UTC 2020 using Jira 7.13.12#713012-sha1:6e07c38070d5191bbf7353952ed38f111754533a.