Working with user associations

[destinf]

Firstly, thank you for creating and maintaining this library!

I've been trying to incorporate this library into my application, but I'm running into problems with model associations. I see the examples for attributes directly tied to the user model but I can't seem to find anything on associations. For example, if a user has a one-to-one association with another model:

class User > ApplicationRecord
  has_one :important_dates
end

create_table "important_dates" do |t|
  t.datetime "birthday"
  t.datetime "anniversary"
end

is it currently possible with this library to set birthday and anniversary through SCIM? Thank you in advance!

[rreinhardt9]

Hello @destinf ! 👋

Hmm, I see that use case for sure and I do not think this is currently possible to set attributes through associations (only attributes directly on the user model).

The reason it won't work currently when setting an attribute is that it loops through the hash of mutable_user_attributes you supply in the configuration file,

scim_rails/app/controllers/scim_rails/scim_users_controller.rb

Lines 67 to 71 in 85dbd05

	def permitted_user_params
	ScimRails.config.mutable_user_attributes.each.with_object({}) do |attribute, hash|
	hash[attribute] = find_value_for(attribute)
	end
	end

then digs into the the params it received to find a value, and then sets that attribute value at the top level of a hash that is then passed to a call to update on your "user" model.

scim_rails/app/controllers/scim_rails/scim_users_controller.rb

Line 53 in 85dbd05

user.update!(permitted_user_params)

One thing you could try would be to create "setters" or aliases on your user model so that user.birthday = would set the birthday in the important dates relation. In your User model you could add delegate :birthday, to: :important_dates. I think that you might then be able to set those attributes through skim as if they existed directly on the user model. I'd haven't tried this myself, but I'd love to know if it works for ya!

With querying for users attributes through scim, I don't think this would work with relations even if you did the above for setting. The reason there is that in the scim gem, it's building a SQL query to query the attributes with the given operator.

scim_rails/app/controllers/scim_rails/scim_users_controller.rb

Line 10 in 85dbd05

"#{ScimRails.config.scim_users_model.connection.quote_column_name(query.attribute)} #{query.operator} ?",

In it's current implementation, that would only work for attributes directly on the user table I'm pretty sure.

That's some quick thoughts off the top of my head!

There may be a cool enhancement in there somewhere... for instance, what if we allow you to set a method on the User model that will be called if it exists when persisting user data? Something like User#scim_update that you can customize to persist relations, etc. The query side would be harder... I'm not sure what to do there to account for relations 🤔 Maybe there would be a way to shift that from building a SQL query to something that can be defined on the User model. I'm not sure!

[destinf]

Hi @rreinhardt9!

Sorry for the late reply, I've been prying into the code to see what I could do. So far, delegate that you mentioned doesn't seem to work with my code base, as the update! method in scim_rails does not handle delegating very well, so I've taken to modifying scim_rails instead.

I'd love to try and submit a PR for this, if you're accepting PRs. If so, what would configuration of this association in scim_rails_config.rb look like? So far, my thoughts are to use an array to indicate an association and keep the symbols to indicate a direct attribute, something like:

config.mutable_user_attributes = {
  :first_name,
  :last_name,
  [:important_dates, :birthday],
  [:important_dates, :anniversary]
}

What do you think?

[rreinhardt9]

Hi @destinf ! Arg, thanks for trying the delegate option and letting us know what happened; sorry it didn't work out.

We're definitely open to checking out PRs, thanks for considering that! @wernull I'd love to get your thoughts on if this seems like something we would want to try for supporting and potential hazards it might face?

Custom Persistence Method?

My first thought is... I wonder if we can give a way to allow your "user" model to define how it persists Scim information. Currently, we call update! directly on that model, but what if we allowed you to customize the name of the update method?

For example, you could configure scim to call scim_update! instead and then in your "user" model, you could define scim_update! and handle all the specifics of relations and persistence in there. Specifically, if it received a hash like:

{
  first_name: "Jane",
  last_name: "Doe",
  birthday: "11/13/1980",
  anniversary: "1/1/1990"
}

You could have a method like:

def scim_update!(attributes)
  update!(
    first_name: attributes[:first_name], 
    last_name: attributes[:last_name],
    important_dates: { birthday: attributes[:birthday], anniversary: attributes[:anniversary] }
  )
end

One thing that seems attractive with this option is it would keep the information about associations and persistence inside the model as opposed to codifying that structure in the initializer for the scim gem. You have the full ability in the model to process the attributes regardless of the underlying data structure.

Querying

So that would handle the "update" side of the house, I think the trickiest side would be handling "querying" for user attributes. It seems like if we were to add the ability to update these attributes, we would also want to support querying by them. The difficult part there is in that code I linked above we are building a SQL query string in order so search for specific attributes.

I'm less clear on what the best option would be here. It would be nice to follow the same pattern of allowing you to define a custom method on the user model, so maybe you could define a custom class method on your "user" that accepts an operator, attribute, and value like:

class < self
  def scim_query(operator, attribute, value)
    if important_date_attributes.include?(attribute)
      # join to important_dates association to find users by these attributes
    else
       # Fall back to the default behavior, not sure the best way to do that here.
       # Maybe this is `User.none` and then in the gem we can say
       # `where(ScimRails.config.scim_users_model.public_send(ScimRails.config.scim_query_method, operator, attribute, value).or(where(#normal_stuff))`
       # for when a custom query method is defined
    end
  end
end

But like like I mentioned, this is the part that I'm a little less sure on 🤔 The spirit of everything I'm trying for above is to find a way to keep the association structure and persistence information in the model rather than trying to codify and handle that structure by using a hash in the scim initializer (and then having to handle that data structure in the gem). I'm afraid that going down the initializer route could get complicated as we have to write code to transverse that data structure and build queries/ updates whereas allowing a method to be defined in the model would place that squarely back where business and persistence logic is at home for that specific app. I'd love your and @wernull 's thoughts on that!

[digerata]

@rreinhardt9 For what it's worth, the direction you describe makes sense to me.

[valerauko]

I'm working on implementing SCIM Groups for this gem (PR eventually), and this is a problem I've run into as well.

However unlike custom attributes like in the case above, SCIM Groups have a well-defined schema in the spec already which might make dealing with that a bit easier. For example there could be a config option like

  config.group_member_relation_attribute = :user_ids
  config.group_member_relation_schema = { value: :user_ids }

that would result in a process like group_object.update!(user_ids: group_hash[:members].map { |member| member[:value] } (after a translation much like the current path_for).

For my usecase Groups enjoy top priority so I'd be willing to make a Group-specific implementation first. Would this approach be acceptable, or would you require a more generic approach like described in the discussion above?