ExternalID for Groups (Create our onw table) #2

Open
opened 2022-07-28 10:04:35 +00:00 by pierre · 9 comments
pierre commented 2022-07-28 10:04:35 +00:00 (Migrated from lab.libreho.st)

Store scim groups external id in a table.

Store scim groups external id in a table.
pierre commented 2022-07-28 10:05:03 +00:00 (Migrated from lab.libreho.st)

changed time estimate to 3d

changed time estimate to 3d
pierre commented 2022-07-28 10:07:33 +00:00 (Migrated from lab.libreho.st)

changed time estimate to 1w

changed time estimate to 1w
rawtaz commented 2022-11-08 13:32:03 +00:00 (Migrated from lab.libreho.st)

mentioned in issue #5

mentioned in issue #5
rawtaz commented 2022-11-23 21:24:09 +00:00 (Migrated from lab.libreho.st)

mentioned in issue #6

mentioned in issue #6
rawtaz commented 2023-03-04 20:27:28 +00:00 (Migrated from lab.libreho.st)

I noticed that for users, their externalId is currently stored in the preferences table by using IConfig::setUserValue().

Can we not do the same thing with externalId for groups (disregarding the name of the function used, which is in a way "fine" because there is already so much inconsistency and misnamed stuff in Nextcloud anyway)? Instead of a user ID, we'd put the group ID in there. In other words, why do we need our own table to store groups' externalId?

I noticed that for users, their `externalId` is currently stored in the `preferences` table by using [`IConfig::setUserValue()`](https://nextcloud-server.netlify.app/classes/ocp-iconfig#method_setUserValue). Can we not do the same thing with `externalId` for groups (disregarding the name of the function used, which is in a way "fine" because there is already so much inconsistency and misnamed stuff in Nextcloud anyway)? Instead of a user ID, we'd put the group ID in there. In other words, why do we need our own table to store groups' `externalId`?
pierre commented 2023-03-06 11:47:10 +00:00 (Migrated from lab.libreho.st)

which is in a way "fine" because there is already so much inconsistency and misnamed stuff in Nextcloud anyway

😅

true story :)

You propose we store a group attribute in all users that belong to that group? I don't think it is a good idea. And having a table is not that terrible.
Because, from what I remember, it is a user preferences table, so each preference has to be attached to a user.

> which is in a way "fine" because there is already so much inconsistency and misnamed stuff in Nextcloud anyway :sweat_smile: true story :) You propose we store a group attribute in all users that belong to that group? I don't think it is a good idea. And having a table is not that terrible. Because, from what I remember, it is a user `preferences` table, so each preference has to be attached to a user.
rawtaz commented 2023-03-06 12:12:13 +00:00 (Migrated from lab.libreho.st)

You propose we store a group attribute in all users that belong to that group?

No, "Instead of a user ID, we'd put the group ID in there".

The preferences table has columns for userid, appid, key and value. The appid one should naturally be scimserviceprovider. The userid one can technically be anything - it is originally intended to be a user ID, but there is really nothing stopping us from putting a group ID there instead, and hence use the preferences table for storing "group preferences", or rather "group settings" just like we store "user settings". In a sense a group is a user-related entity, so meh.

Given the amount of inconsistency in Nextcloud as a whole, I think pragmatism is somewhat reasonable even in this case. I mean, creating another table just to store the externalId of groups seems rather overkill, even if it is the more correct way of doing it. We have already deviated from what is "right" when we store an externalId for users in the preferences table, as an externalId is not a preference (nor config, referring to IConfig), it is data. It doesn't really belong in this table in the first place.

That said, it was just food for thought :)

> You propose we store a group attribute in all users that belong to that group? No, "Instead of a user ID, we'd put the group ID in there". The `preferences` table has columns for `userid`, `appid`, key and value. The `appid` one should naturally be `scimserviceprovider`. The `userid` one can technically be anything - it is originally intended to be a user ID, but there is really nothing stopping us from putting a group ID there instead, and hence use the `preferences` table for storing "group preferences", or rather "group settings" just like we store "user settings". In a sense a group is a user-related entity, so meh. Given the amount of inconsistency in Nextcloud as a whole, I think pragmatism is somewhat reasonable even in this case. I mean, creating another table just to store the externalId of groups seems rather overkill, even if it is the more correct way of doing it. We have already deviated from what is "right" when we store an externalId for users in the preferences table, as an externalId is not a preference (nor config, referring to IConfig), it is *data*. It doesn't really belong in this table in the first place. That said, it was just food for thought :)
hugo.renard commented 2023-03-06 13:23:05 +00:00 (Migrated from lab.libreho.st)

I'm not sure of what could happen in the shit show that is IDs in Nextcloud, but doesn't it add a risk of collision between user and group ID ?

It seems reasonable to have a table scim_resource with :

  • id of the internal resource
  • type of the internal resource (user or group)
  • externalId, ID of the external resource
  • clientIdto identify a SCIM client in case there is more than one (requires a way to identify clients by path or header).
I'm not sure of what could happen in the shit show that is IDs in Nextcloud, but doesn't it add a risk of collision between user and group ID ? It seems reasonable to have a table `scim_resource` with : - `id` of the internal resource - `type` of the internal resource (user or group) - `externalId`, ID of the external resource - `clientId`to identify a SCIM client in case there is more than one (requires a way to identify clients by path or header).
rawtaz commented 2023-03-06 17:49:39 +00:00 (Migrated from lab.libreho.st)

I'm not sure of what could happen in the shit show that is IDs in Nextcloud, but doesn't it add a risk of collision between user and group ID?

I can't speak for others, but all of the users that we SCIM from Azure AD into Nextcloud will get a user ID that is their e-mail address. I cannot imagine that we will ever create a group that has a name or ID that is formatted like an e-mail address.

The table name should probably have the app ID as the base prefix, for consistency and to avoid any confusion down the road. Other than that, the schema seems to make sense. A related question though; do you have any thoughts on where/how to store enterprise extension data, e.g. organization?

> I'm not sure of what could happen in the shit show that is IDs in Nextcloud, but doesn't it add a risk of collision between user and group ID? I can't speak for others, but all of the users that we SCIM from Azure AD into Nextcloud will get a user ID that is their e-mail address. I cannot imagine that we will ever create a group that has a name or ID that is formatted like an e-mail address. The table name should probably have the app ID as the base prefix, for consistency and to avoid any confusion down the road. Other than that, the schema seems to make sense. A related question though; do you have any thoughts on where/how to store enterprise extension data, e.g. `organization`?
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: libre.sh/scimserviceprovider#2
No description provided.