Merge pull request #134 from RIPAGlobal/feature/further-extension-sch…

…ema-handling-fixes

Various fixes for extension schema

README.md

@@ -457,7 +457,9 @@ You can extend schema with custom data by defining an extension class and callin
 * Must call `super` in `def initialize`, providing data as shown in the example below
 * Must define class methods for `::id` and `::scim_attributes`
 
-The `::id` class method defines a unique schema ID that is used to namespace payloads or paths in JSON responses describing extended resources, JSON payloads creating them or PATCH paths modifying them. The SCIM RFCs would refer to this as the URN. For example, we might choose to use the [RFC-defined User extension schema](https://tools.ietf.org/html/rfc7643#section-4.3) to define a couple of extra fields our User model happens to support:
+The `::id` class method defines a unique schema ID that is used to namespace payloads or paths in JSON responses describing extended resources, JSON payloads creating them or PATCH paths modifying them. The RFCs require this to be a URN ([see RFC 2141](https://tools.ietf.org/html/rfc2141)). Your extension's ID URN must be globally unique. Depending on your expected use case, you should review the [IANA registration considerations that RFC 7643 describes](https://tools.ietf.org//html/rfc7643#section-10) and definitely review the [syntactic structure declaration therein](https://tools.ietf.org/html/rfc7643#section-10.2.1) (`urn:ietf:params:scim:{type}:{name}{:other}`).
+
+For example, we might choose to use the [RFC-defined User extension schema](https://tools.ietf.org/html/rfc7643#section-4.3) to define a couple of extra fields our User model happens to support:
 
 ```ruby
 class UserEnterpriseExtension < Scimitar::Schema::Base

app/models/scimitar/resources/base.rb

@@ -35,14 +35,45 @@ def initialize(options = {})
         @errors = ActiveModel::Errors.new(self)
       end
 
+      # Scimitar has at present a general limitation in handling schema IDs,
+      # which just involves stripping them and requiring attributes across all
+      # extension schemas to be overall unique.
+      #
+      # This method takes an options payload for the initializer and strips out
+      # *recognised* schema IDs, so that the resulting attribute data matches
+      # the resource attribute map.
+      #
+      # +attributes+:: Attributes to assign via initializer; typically a POST
+      #                payload of attributes that has been run through Rails
+      #                strong parameters for safety.
+      #
+      # Returns a new object of the same class as +options+ with recognised
+      # schema IDs removed.
+      #
       def flatten_extension_attributes(options)
-        flattened = options.dup
-        self.class.extended_schemas.each do |extended_schema|
-          if extension_attrs = flattened.delete(extended_schema.id)
-            flattened.merge!(extension_attrs)
+        flattened             = options.class.new
+        lower_case_schema_ids = self.class.extended_schemas.map do | schema |
+          schema.id.downcase()
+        end
+
+        options.each do | key, value |
+          path = Scimitar::Support::Utilities::path_str_to_array(
+            self.class.extended_schemas,
+            key
+          )
+
+          if path.first.include?(':') && lower_case_schema_ids.include?(path.first.downcase)
+            path.shift()
+          end
+
+          if path.empty?
+            flattened.merge!(value)
+          else
+            flattened[path.join('.')] = value
           end
         end
-        flattened
+
+        return flattened
       end
 
       # Can be used to extend an existing resource type's schema. For example:

lib/scimitar/support/utilities.rb

@@ -57,9 +57,10 @@ def self.dot_path(array, value)
       #              <tt>scim_resource_type.extended_schemas</tt> value. The
       #              Array should be empty if there are no extensions.
       #
-      # +path_str+:: Path string, e.g. <tt>"password"</tt>, <tt>"name.givenName"</tt>,
+      # +path_str+:: Path String, e.g. <tt>"password"</tt>, <tt>"name.givenName"</tt>,
       #              <tt>"urn:ietf:params:scim:schemas:extension:enterprise:2.0:User"</tt> (special case),
       #              <tt>"urn:ietf:params:scim:schemas:extension:enterprise:2.0:User:organization"</tt>
+      #              (if given a Symbol, it'll be converted to a String).
       #
       # Returns an array of components, e.g. <tt>["password"]</tt>, <tt>["name",
       # "givenName"]</tt>,
@@ -74,6 +75,7 @@ def self.dot_path(array, value)
       # path-free payload.
       #
       def self.path_str_to_array(schemas, path_str)
+        path_str   = path_str.to_s
         components = []
 
         # Note the ":" separating the schema ID (URN) from the attribute.
@@ -84,11 +86,14 @@ def self.path_str_to_array(schemas, path_str)
         # particular, https://tools.ietf.org/html/rfc7644#page-35.
         #
         if path_str.include?(':')
+          lower_case_path_str = path_str.downcase()
+
           schemas.each do |schema|
-            attributes_after_schema_id = path_str.downcase.split(schema.id.downcase + ':').drop(1)
+            lower_case_schema_id       = schema.id.downcase()
+            attributes_after_schema_id = lower_case_path_str.split(lower_case_schema_id + ':').drop(1)
 
             if attributes_after_schema_id.empty?
-              components += [schema.id]
+              components += [schema.id] if lower_case_path_str == lower_case_schema_id
             else
               attributes_after_schema_id.each do |component|
                 components += [schema.id] + component.split('.')

spec/apps/dummy/app/models/mock_user.rb

@@ -18,6 +18,7 @@ class MockUser < ActiveRecord::Base
     work_phone_number
     organization
     department
+    manager
     mock_groups
   }
 
@@ -90,13 +91,15 @@ def self.scim_attributes_map
         }
       ],
       active: :is_active,
+      primaryEmail: :scim_primary_email,
 
       # Custom extension schema - see configuration in
       # "spec/apps/dummy/config/initializers/scimitar.rb".
       #
       organization: :organization,
       department:   :department,
-      primaryEmail: :scim_primary_email,
+      manager:      :manager,
+
       userGroups: [
         {
           list:      :mock_groups,
@@ -130,7 +133,8 @@ def self.scim_queryable_attributes
     }
   end
 
-  # reader
+  # Custom attribute reader
+  #
   def scim_primary_email
     work_email_address
   end

spec/apps/dummy/config/initializers/scimitar.rb

@@ -33,10 +33,13 @@ def scim_resource_type_url(options)
 
   module ScimSchemaExtensions
     module User
+
+      # This "looks like" part of the standard Enterprise extension.
+      #
       class Enterprise < Scimitar::Schema::Base
         def initialize(options = {})
           super(
-            name:            'ExtendedUser',
+            name:            'EnterpriseExtendedUser',
             description:     'Enterprise extension for a User',
             id:              self.class.id,
             scim_attributes: self.class.scim_attributes
@@ -55,8 +58,33 @@ def self.scim_attributes
           ]
         end
       end
+
+      # In https://github.com/RIPAGlobal/scimitar/issues/122 we learn that with
+      # more than one extension, things can go wrong - so now we test with two.
+      #
+      class Manager < Scimitar::Schema::Base
+        def initialize(options = {})
+          super(
+            name:            'ManagementExtendedUser',
+            description:     'Management extension for a User',
+            id:              self.class.id,
+            scim_attributes: self.class.scim_attributes
+          )
+        end
+
+        def self.id
+          'urn:ietf:params:scim:schemas:extension:manager:1.0:User'
+        end
+
+        def self.scim_attributes
+          [
+            Scimitar::Schema::Attribute.new(name: 'manager', type: 'string')
+          ]
+        end
+      end
     end
   end
 
   Scimitar::Resources::User.extend_schema ScimSchemaExtensions::User::Enterprise
+  Scimitar::Resources::User.extend_schema ScimSchemaExtensions::User::Manager
 end

spec/apps/dummy/db/migrate/20210304014602_create_mock_users.rb

@@ -19,6 +19,7 @@ def change
       #
       t.text :organization
       t.text :department
+      t.text :manager
     end
   end
 end

spec/apps/dummy/db/schema.rb

@@ -41,6 +41,7 @@
     t.text "work_phone_number"
     t.text "organization"
     t.text "department"
+    t.text "manager"
   end
 
   add_foreign_key "mock_groups_users", "mock_groups"

spec/controllers/scimitar/schemas_controller_spec.rb

@@ -30,10 +30,10 @@ def index
       expect(response).to be_ok
 
       parsed_body = JSON.parse(response.body)
-      expect(parsed_body['Resources']&.size).to eql(3)
+      expect(parsed_body['Resources']&.size).to eql(4)
 
       schema_names = parsed_body['Resources'].map {|schema| schema['name']}
-      expect(schema_names).to match_array(['User', 'ExtendedUser', 'Group'])
+      expect(schema_names).to match_array(['User', 'EnterpriseExtendedUser', 'ManagementExtendedUser', 'Group'])
     end
 
     it 'returns only the User schema when its id is provided' do

spec/models/scimitar/resources/base_spec.rb

@@ -305,7 +305,7 @@ def self.scim_attributes
 
       ExtensionSchema = Class.new(Scimitar::Schema::Base) do
         def self.id
-          'extension-id'
+          'urn:extension'
         end
 
         def self.scim_attributes
@@ -333,13 +333,13 @@ def self.resource_type_id
 
       context '#initialize' do
         it 'allows setting extension attributes' do
-          resource = resource_class.new('extension-id' => {relationship: 'GAGA'})
+          resource = resource_class.new('urn:extension' => {relationship: 'GAGA'})
           expect(resource.relationship).to eql('GAGA')
         end
 
         it 'allows setting complex extension attributes' do
           user_groups = [{ value: '123' }, { value: '456'}]
-          resource = resource_class.new('extension-id' => {userGroups: user_groups})
+          resource = resource_class.new('urn:extension' => {userGroups: user_groups})
           expect(resource.userGroups.map(&:value)).to eql(['123', '456'])
         end
       end # "context '#initialize' do"
@@ -348,8 +348,8 @@ def self.resource_type_id
         it 'namespaces the extension attributes' do
           resource = resource_class.new(relationship: 'GAGA')
           hash = resource.as_json
-          expect(hash["schemas"]).to eql(['custom-id', 'extension-id'])
-          expect(hash["extension-id"]).to eql("relationship" => 'GAGA')
+          expect(hash["schemas"]).to eql(['custom-id', 'urn:extension'])
+          expect(hash["urn:extension"]).to eql("relationship" => 'GAGA')
         end
       end # "context '#as_json' do"
 
@@ -362,10 +362,10 @@ def self.resource_type_id
 
         context 'validation' do
           it 'validates into custom schema' do
-            resource = resource_class.new('extension-id' => {})
+            resource = resource_class.new('urn:extension' => {})
             expect(resource.valid?).to eql(false)
 
-            resource = resource_class.new('extension-id' => {relationship: 'GAGA'})
+            resource = resource_class.new('urn:extension' => {relationship: 'GAGA'})
             expect(resource.relationship).to eql('GAGA')
             expect(resource.valid?).to eql(true)
           end

spec/models/scimitar/resources/mixin_spec.rb

@@ -288,10 +288,15 @@ def self.scim_queryable_attributes
             'name'        => {'givenName'=>'Foo', 'familyName'=>'Bar'},
             'groups'      => [{'display'=>g1.display_name, 'value'=>g1.id.to_s}, {'display'=>g3.display_name, 'value'=>g3.id.to_s}],
             'meta'        => {'location'=>"https://test.com/mock_users/#{uuid}", 'resourceType'=>'User'},
-            'schemas'     => ['urn:ietf:params:scim:schemas:core:2.0:User', 'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User'],
+            'schemas'     => [
+              'urn:ietf:params:scim:schemas:core:2.0:User',
+              'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User',
+              'urn:ietf:params:scim:schemas:extension:manager:1.0:User',
+            ],
             'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User' => {
               'organization' => 'SOMEORG',
             },
+            'urn:ietf:params:scim:schemas:extension:manager:1.0:User' => {},
           })
         end
       end # "context 'with list of requested attributes' do"
@@ -333,13 +338,19 @@ def self.scim_queryable_attributes
             'externalId'  => 'AA02984',
             'groups'      => [{'display'=>g1.display_name, 'value'=>g1.id.to_s}, {'display'=>g3.display_name, 'value'=>g3.id.to_s}],
             'meta'        => {'location'=>"https://test.com/mock_users/#{uuid}", 'resourceType'=>'User'},
-            'schemas'     => ['urn:ietf:params:scim:schemas:core:2.0:User', 'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User'],
-
+            'schemas'     => [
+              'urn:ietf:params:scim:schemas:core:2.0:User',
+              'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User',
+              'urn:ietf:params:scim:schemas:extension:manager:1.0:User',
+            ],
             'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User' => {
               'organization' => 'SOMEORG',
               'department'   => nil,
-              'primaryEmail' => instance.work_email_address
-            }
+              'primaryEmail' => instance.work_email_address,
+            },
+            'urn:ietf:params:scim:schemas:extension:manager:1.0:User' => {
+              'manager' => nil
+            },
           })
         end
       end # "context 'with a UUID, renamed primary key column' do"
@@ -463,9 +474,13 @@ def self.scim_timestamps_map
               ],
 
               'meta'    => {'location'=>'https://test.com/static_map_test', 'resourceType'=>'User'},
-              'schemas' => ['urn:ietf:params:scim:schemas:core:2.0:User', 'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User'],
-
-              'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User' => {}
+              'schemas' => [
+                'urn:ietf:params:scim:schemas:core:2.0:User',
+                'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User',
+                'urn:ietf:params:scim:schemas:extension:manager:1.0:User',
+              ],
+              'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User' => {},
+              'urn:ietf:params:scim:schemas:extension:manager:1.0:User' => {},
             })
           end
         end # "context 'using static mappings' do"
@@ -492,9 +507,13 @@ def self.scim_timestamps_map
               ],
 
               'meta'    => {'location'=>'https://test.com/dynamic_map_test', 'resourceType'=>'User'},
-              'schemas' => ['urn:ietf:params:scim:schemas:core:2.0:User', 'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User'],
-
-              'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User' => {}
+              'schemas' => [
+                'urn:ietf:params:scim:schemas:core:2.0:User',
+                'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User',
+                'urn:ietf:params:scim:schemas:extension:manager:1.0:User',
+              ],
+              'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User' => {},
+              'urn:ietf:params:scim:schemas:extension:manager:1.0:User' => {},
             })
           end
         end # "context 'using dynamic lists' do"