New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User account table and user locking/blacklist #23558

Closed
karlitschek opened this Issue Mar 24, 2016 · 27 comments

Comments

Projects
None yet
8 participants
@karlitschek
Member

karlitschek commented Mar 24, 2016

User Account Table

Requirements

More and more features within ownCloud (either during migration or in live operations) require to list all users which are available in ownCloud. Especially during migration we have been heavily facing issues with user backend apps not being present and with bad performance impacts since the user information has to be queried from an external system which in addition leads to slower processing and latency. Especially the latency issue in the area of LDAP has been targeted by some bigger installations by heavy master slave ldap setups.
Futhermore any user related api we use requires pagination to allow proper low memory and low latency processing – nobody wants to load half a million user objects into ram before further processing. Proper pagination can only be implemented if all users are kept in one place.

Feature description

There will be a new table in the database oc_user_accounts. All user backends will be 'degraded' to authentication backends – we basically only need them to perform authentication.

1

2

The database design is influenced by one design decision we have to take: do we allow multiple authentications for one user? This would allow one user to login via ldap as well as shibboleth as well as user/password. As of today this is only possible for ldap and shibboleth because of we make sure that login names match up magically. Having multiple authentication mechanisms assigned to one account would leverage this to a more general approach.

Migration

Migration of the regular database user backend will happen during upgrade. Any other backend will need to use an occ command – this is necessary since this might take a very long time.
We can think about preparing a pre-script to be executed so that oc_user_accounts can apready be populated upfront.
The occ command will basically grab all users from a given user backend and move the information into the table oc_user_accounts.

Vcard handling and related functionalities

The user will maintain his vcard in the personal page. Upon change we will sync the vcard down to the system addressbook which is used for fed syncing of contacts.
The vcard will hold the avatar as well – for now we shall not touch the avatar handling, keep the current storage location and import the avatar into the vcard upon change.

locking/blacklisting

A new occ command will be added to lock and unlock a specific user. This can be used with a user account is locked in the backend LDAP but there are still valid PHP session open. A user who is blakclisted/locked/disabled can't do any action in ownCloud even if authenticated.

Design goal

We need to make sure that the IUserManager interface and all related interfaces stay untouched.
We will deprecate any interface which is related to the user backend.
We need to make sure that existing user backend implementation continue to work.
We should provide a general purpose tool which will migrate user accounts from any given user backend to the accounts table.

Open Questions

User management UI changes to deflect multiple auths mechanisms per user
touch group handling as well?
Arthur will provide a list of use cases about LDAP which need to be taken into account
How to sync heavy backends like LDAP with the user accounts table?
How do we perform the mapping of authentications to users?
Do we autoprovision a user as soon as it logs in?
Shall the migration touch sharing and other components already? As in move from legacy_login to the real it?

@karlitschek

This comment has been minimized.

Show comment
Hide comment
@karlitschek

karlitschek Mar 24, 2016

Member

This should solve the invalidate session issue: #18410

Member

karlitschek commented Mar 24, 2016

This should solve the invalidate session issue: #18410

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Mar 24, 2016

Member

All user backends will be 'degraded' to authentication backends

I think we also need the user backends to deliver the list of know users, if supported, so we can prepopulate the table (ex: LDAP).

Some backends might not support browsing/searching users, something to keep in mind (and retest).

Member

PVince81 commented Mar 24, 2016

All user backends will be 'degraded' to authentication backends

I think we also need the user backends to deliver the list of know users, if supported, so we can prepopulate the table (ex: LDAP).

Some backends might not support browsing/searching users, something to keep in mind (and retest).

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Mar 24, 2016

Member

Ref other ticket about the account table: #21282

Member

PVince81 commented Mar 24, 2016

Ref other ticket about the account table: #21282

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Mar 24, 2016

Member

In case this wasn't considered yet, here were some other points from @blizzz #21282 (comment)

Member

PVince81 commented Mar 24, 2016

In case this wasn't considered yet, here were some other points from @blizzz #21282 (comment)

@blizzz

This comment has been minimized.

Show comment
Hide comment
@blizzz

blizzz Mar 24, 2016

Contributor

In case this wasn't considered yet, here were some other points from @blizzz #21282 (comment)

Thanks for pointing at it @PVince81

All user backends will be 'degraded' to authentication backends – we basically only need them to perform authentication.

This sentence opens a nice box of topics to discuss, more than I have expected when I was doing #21282 (comment)

Contributor

blizzz commented Mar 24, 2016

In case this wasn't considered yet, here were some other points from @blizzz #21282 (comment)

Thanks for pointing at it @PVince81

All user backends will be 'degraded' to authentication backends – we basically only need them to perform authentication.

This sentence opens a nice box of topics to discuss, more than I have expected when I was doing #21282 (comment)

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Mar 29, 2016

Member

@schiesbn also suggested using the carddav/system address book table which already contains all known users (that's the table used for syncing accounts between federated servers)

Member

PVince81 commented Mar 29, 2016

@schiesbn also suggested using the carddav/system address book table which already contains all known users (that's the table used for syncing accounts between federated servers)

@MTRichards MTRichards added this to the 9.1-current milestone Mar 30, 2016

@MTRichards

This comment has been minimized.

Show comment
Hide comment
@MTRichards

MTRichards Mar 30, 2016

Contributor

Out of laziness reasons, these are screenshots.
SO, we need to change the behavior of the disabled user flag so that it disables logins for the user. In addition, link shares, regular and federated shares become unavailable with a 503 error returned so the desktop clients don't delete all the files while it is disabled.

Contributor

MTRichards commented Mar 30, 2016

Out of laziness reasons, these are screenshots.
SO, we need to change the behavior of the disabled user flag so that it disables logins for the user. In addition, link shares, regular and federated shares become unavailable with a 503 error returned so the desktop clients don't delete all the files while it is disabled.

@MTRichards

This comment has been minimized.

Show comment
Hide comment
@MTRichards

MTRichards Mar 30, 2016

Contributor

We would change the boolean to an int to cover more than two possible scenarios: Disabled, enabled, and disabled (shares available)

Contributor

MTRichards commented Mar 30, 2016

We would change the boolean to an int to cover more than two possible scenarios: Disabled, enabled, and disabled (shares available)

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Mar 30, 2016

Member
  • use mount id instead of home_dir
Member

PVince81 commented Mar 30, 2016

  • use mount id instead of home_dir
@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Mar 30, 2016

Member
  • rename "legacy_login" to "user_id"
Member

PVince81 commented Mar 30, 2016

  • rename "legacy_login" to "user_id"
@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Mar 30, 2016

Member
  • use "user_id" as primary key (see previous item), that one is not changeable
Member

PVince81 commented Mar 30, 2016

  • use "user_id" as primary key (see previous item), that one is not changeable
@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Mar 30, 2016

Member
  • remove "password_hash" and use the one from the "oc_users" table, since it contains the password hash for local auth
Member

PVince81 commented Mar 30, 2016

  • remove "password_hash" and use the one from the "oc_users" table, since it contains the password hash for local auth
@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Mar 30, 2016

Member
  • add "display_name" column
Member

PVince81 commented Mar 30, 2016

  • add "display_name" column
@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Mar 30, 2016

Member
  • add column "oc_auth.status": 1 = disabled, other states for "wipe this device" in the future (which is one reason to keep the entries)
Member

PVince81 commented Mar 30, 2016

  • add column "oc_auth.status": 1 = disabled, other states for "wipe this device" in the future (which is one reason to keep the entries)
@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Mar 30, 2016

Member
  • provisioning API option/attribute for disabling users
Member

PVince81 commented Mar 30, 2016

  • provisioning API option/attribute for disabling users
@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Mar 30, 2016

Member

User management UI changes to deflect multiple auths mechanisms per user
touch group handling as well?

Discussed => will be responsibility of user backends / app that implements the backend

Member

PVince81 commented Mar 30, 2016

User management UI changes to deflect multiple auths mechanisms per user
touch group handling as well?

Discussed => will be responsibility of user backends / app that implements the backend

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Mar 30, 2016

Member

Group handling left as is.

Member

PVince81 commented Mar 30, 2016

Group handling left as is.

@MorrisJobke

This comment has been minimized.

Show comment
Hide comment
@MorrisJobke

MorrisJobke Mar 30, 2016

Member

Shall the migration touch sharing and other components already? As in move from legacy_login to the real it?

We will NOT change any username - it's our internal ID and will stay.

Member

MorrisJobke commented Mar 30, 2016

Shall the migration touch sharing and other components already? As in move from legacy_login to the real it?

We will NOT change any username - it's our internal ID and will stay.

@MTRichards

This comment has been minimized.

Show comment
Hide comment
@MTRichards

MTRichards Apr 1, 2016

Contributor

This is what is being covered in 9.1: #18410

Contributor

MTRichards commented Apr 1, 2016

This is what is being covered in 9.1: #18410

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Nov 30, 2016

Member

Do we also need a central groups table ? From what I see every backend provides a dynamic list of their groups.

Furthermore the groups logic is a bit worrying: the code seems to allow having the same group id coming from multiple backends, but so far it seems backends like LDAP does extra work to prevent duplicate ids. See #7918 (comment) for my research on groups.

Member

PVince81 commented Nov 30, 2016

Do we also need a central groups table ? From what I see every backend provides a dynamic list of their groups.

Furthermore the groups logic is a bit worrying: the code seems to allow having the same group id coming from multiple backends, but so far it seems backends like LDAP does extra work to prevent duplicate ids. See #7918 (comment) for my research on groups.

@cdamken

This comment has been minimized.

Show comment
Hide comment
@cdamken

cdamken Jan 3, 2017

Contributor

Do we also need a central groups table ? From what I see every backend provides a dynamic list of their groups.

could we try to have an occ ldap:update-group before the upgrade and while upgrading avoid to check the groups again?

Contributor

cdamken commented Jan 3, 2017

Do we also need a central groups table ? From what I see every backend provides a dynamic list of their groups.

could we try to have an occ ldap:update-group before the upgrade and while upgrading avoid to check the groups again?

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Feb 10, 2017

Member
  • "oc_user_accounts" needs a "backend_id" column to track from which backend the user was retrieved (ex: user_ldap)

  • rename "oc_user_accounts.legacy_login" to "oc_user_accounts.backend_user_id" which is the user id as delivered by the user backend

  • also add a table "oc_all_groups" that contains the groups imported from the different backends

    • id => primary key
    • backend_id => id of the backend from which the group was retrieved (ex: user_ldap)
    • backend_group_id => group id as delivered by the backend
    • display_name => group display name (display name is new in 10.0)
  • "oc_all_group_members" that contains group memberships

    • id => primary key
    • backend_group_id => id of the backend from which the group was retrieved (ex: user_ldap)
    • backend_user_id => user id as delivered by the backend
    • role (?) => for future subadmin role

Goal is to be able to do quick search/lookup on the local table instead of contacting every backend every time we need such information.

  • discuss whether to keep oc_users and oc_groups which is the default local database backend manageable of users or merge it with the new table. I'd vote for keeping these for now. A bit of duplicate data though.
Member

PVince81 commented Feb 10, 2017

  • "oc_user_accounts" needs a "backend_id" column to track from which backend the user was retrieved (ex: user_ldap)

  • rename "oc_user_accounts.legacy_login" to "oc_user_accounts.backend_user_id" which is the user id as delivered by the user backend

  • also add a table "oc_all_groups" that contains the groups imported from the different backends

    • id => primary key
    • backend_id => id of the backend from which the group was retrieved (ex: user_ldap)
    • backend_group_id => group id as delivered by the backend
    • display_name => group display name (display name is new in 10.0)
  • "oc_all_group_members" that contains group memberships

    • id => primary key
    • backend_group_id => id of the backend from which the group was retrieved (ex: user_ldap)
    • backend_user_id => user id as delivered by the backend
    • role (?) => for future subadmin role

Goal is to be able to do quick search/lookup on the local table instead of contacting every backend every time we need such information.

  • discuss whether to keep oc_users and oc_groups which is the default local database backend manageable of users or merge it with the new table. I'd vote for keeping these for now. A bit of duplicate data though.
@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Feb 14, 2017

Member

use mount id instead of home_dir

I mean the mount id from the "oc_mounts" table. But I don't think we can use it until we have oc_mounts as fstab: #26190

Would still be good to have a direct connection to the oc_storages and/or oc_filecache of the user for easier joining. Could be possible over the oc_mounts table. Note that the table only populates after the first setupFS, so that column should be nullable.

Member

PVince81 commented Feb 14, 2017

use mount id instead of home_dir

I mean the mount id from the "oc_mounts" table. But I don't think we can use it until we have oc_mounts as fstab: #26190

Would still be good to have a direct connection to the oc_storages and/or oc_filecache of the user for easier joining. Could be possible over the oc_mounts table. Note that the table only populates after the first setupFS, so that column should be nullable.

@DeepDiver1975

This comment has been minimized.

Show comment
Hide comment
@DeepDiver1975

DeepDiver1975 Feb 14, 2017

Member

hmmm - but the mount_id is only available once the storage is initialized - which is at first login.
But I'd like to set the home folder path at creation of the user. hmmmm

Member

DeepDiver1975 commented Feb 14, 2017

hmmm - but the mount_id is only available once the storage is initialized - which is at first login.
But I'd like to set the home folder path at creation of the user. hmmmm

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Feb 14, 2017

Member

Actually, by extension the oc_storages entry is also only created after the first login.
So even the home folder path should be nullable until the user logs in.

Because the home folder path might be computed based on LDAP attributes 🙈

Member

PVince81 commented Feb 14, 2017

Actually, by extension the oc_storages entry is also only created after the first login.
So even the home folder path should be nullable until the user logs in.

Because the home folder path might be computed based on LDAP attributes 🙈

@DeepDiver1975 DeepDiver1975 referenced this issue Feb 15, 2017

Closed

Central user account table arriving ..... #27165

8 of 16 tasks complete
@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Feb 20, 2017

Member

I suggest that for now we keep the "database user backend" as is and let the users page still drive that one. The account table logic will then sync users down from that backend.

Maybe one different would be that we should fire events from backends whenever user/groups change to tell the account table to directly resync a specific user/group for immediate result. Such events might not apply for all backends. This would also be useful for the customgroups app when creating groups.

Side note: assigning LDAP users to local groups must still be possible as it's a supported feature.

Member

PVince81 commented Feb 20, 2017

I suggest that for now we keep the "database user backend" as is and let the users page still drive that one. The account table logic will then sync users down from that backend.

Maybe one different would be that we should fire events from backends whenever user/groups change to tell the account table to directly resync a specific user/group for immediate result. Such events might not apply for all backends. This would also be useful for the customgroups app when creating groups.

Side note: assigning LDAP users to local groups must still be possible as it's a supported feature.

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Apr 11, 2017

Member

Raised #27623 for the groups and gorup memberships part

Member

PVince81 commented Apr 11, 2017

Raised #27623 for the groups and gorup memberships part

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment