user whitelisting not consistent

nicolasfranck commented 5 years ago

The way whitelisting of users is done in librecat is not consistent: it makes promises that it cannot fulfil.

In the configuration you can have

user:
  sources:
    - store: store1
      bag: bag1
      username_attr: login
    - store: store2
      bag: bag2
      username_attr: orcid

This way you can:

match "login" with attribute "login" from bag1
match "login" with attribute "orcid" from bag2

That's all fine, but is does not force you to look in the main user database. The user that is returned by methods LibreCat::Model::User::find_by_username and LibreCat::Model::User::get is not necessarily contained in the internal user database.

Proposal:

deprecate current use of configuration option "user.sources":
force "store" to "search" or "main". Default is "main".
force "bag" to "user", meaning you ignore the configuration option

Problems:

what to do with the store "builtin_users"?

vpeil commented 5 years ago

I understand the store builtin_users as for testing or development. This can be done differently, though. I can't imagine someone uses it in production

nics commented 5 years ago

We use it in production to store the superuser

nics commented 5 years ago

But @nicolasfranck is right, the feature is problematic, not one of my best ideas ;-)

phochste commented 5 years ago

@nicolasfranck what is the relation of this ticket to https://github.com/LibreCat/LibreCat/pull/747 ? Does this PR close this issue , is it unrelated to this issue , or something else?

nicolasfranck commented 5 years ago

@phochste issue #747 is about blocking the user that was selected. This is about how we should select a user.

nicolasfranck commented 4 years ago

The method get and add of this model are also contradicting each other: while add updates data in the underlying bag and search_bag, the get retrieves data from the first bag in a list.

For example, put this in config/catmandu.local.yml ..

user:
  sources:
    - store: builtin_users
      username_attr: login
    - store: search
      bag: user
      username_attr: login

..and you will strange behaviour:

user "einstein" (id 1234) appears in the list of users in /librecat/admin/account
if you try to update that record using the form, the changes will appear in /librecat/admin/account, but if you return to the form nothing of the like will be visible.
reason: the form uses user->get to retrieve, which should be correct, were it not that the get is overwritten in LibreCat::Model::User..

nicolasfranck commented 4 years ago

Anyway, the record that is retrieved in /librecat/admin/account/edit/:id should be the one from the bag of the model. See my PR https://github.com/LibreCat/LibreCat/pull/912

vpeil commented 4 years ago

@nicolasfranck is this one solved by PR #912 ?

nicolasfranck commented 4 years ago

@vpeil only partially. I made sure /librecat/admin/account and others use the same underlying store, but this method is also used in other places.

nicolasfranck commented 3 years ago

@nics @phochste @vpeil @petrakohorst would it be an option to

enforce attribute store to be either search or main
ignore attribute bag, and set it to bag user always

One can always use the builtin_users from the authentication layer (password check), but not as a whitelist.

LibreCat / LibreCat

user whitelisting not consistent #748