Revise deactivation of doAddUser for DOSIS

[cmeier76]

@drfho What objective has if false and ... to Deactivate adding (DOSIS) in zms-publishing/Products.zmsPluggableAuthService@080c626

Good point! Saving of constantly saving/refreshing tokens via Plugin can result in huge ZODB growth, especially when every access is running through the OIDC pipeline (like in the DOSIS use-case). This was fundamental drawback, we did not see at the beginnning. Here this was just covered by the fact, that just a few users get access via login. That is why the UniBE-ZODB did not grow so much.
I suppose we need to refactor the feature "saving the userid" as configurable.
Or maybe we get alternative ideas about user registration ?

Originally posted by @drfho in https://github.com/idasm-unibe-ch/unibe-cms/pull/1273#issuecomment-4731002735

[cmeier76]

IDEA: Perform the check using attributes that can be configured in ZMI, since users.get(user_id,{}) != decoded_token is too broad (it always changes). If no attributes are set, no saving must be performed (to avoid rapidly increasing ZODB size due Undo history).

[drfho]

Today, we analysed the issue and look forward to a feedback for the proposed solution:

Problem

The reason that we run into a probem here with the doAddUser()-function

Products.zmsPluggableAuthService/Products/zmsPluggableAuthService/plugins/ZMSPASSsoPlugin.py

Lines 425 to 443 in 080c626

	def doAddUser(self, login, password):
	""" Add a user record to a User Manager, with the given login
	and password. It is up to the implementation to determine
	if the login is used as user id as well.
	o Return a Boolean indicating whether a user was added or not
	"""
	request = self.REQUEST
	token = request.get(self.header_name, '')
	decoded_token = self.decryptToken(token)
	if decoded_token:
	user_id = decoded_token['user_id']
	users = getattr(self,'_users',{})
	# Deactivate adding (DOSIS)
	if False and users.get(user_id,{}) != decoded_token:
	users[user_id] = decoded_token
	self._users = users
	return True
	return False

is that

1. .the doAddUser-function is called in many use-contextes
2. the OIDC payload has volatile, date-depending values

So we need to differ in general different uses-cases of handling the ZMSPASSsoPlugin-attribute _users:

1. we want to save user data because we need it for ZMS/editors
2. we do not need the user-data for ZMS (because these users are just subscribers)

Solution

New ZMSPASSsoPlugin-property "Crucial keys for doAddUser"

We may introduce a new property-field Crucial keys for doAddUser for conforming the saving of user-data based on the given (comma-separated) key-names. This has two effects:

1. if key-names are given, the doAddUser()-function will create a new user-dataset to ZMSPASSsoPlugin._users
2. if key-names are given, the doAddUser()-function will change the formerly added dataset if current token/payload-values differ from the saved ones

So the DOSIS-case is simply covered by giving no keys (no keys = no save)

Alternative User_Adder-Option

The PASPlugin-interface has a general functionality-property "User_Adder"; if this actually will will neutralize the execution of the doAddUser-function, the checkbox may be a quick solution. Hello @zmsdev, can you confirm that IUserAdderPlugin is accomplishing this?

Products.zmsPluggableAuthService/Products/zmsPluggableAuthService/plugins/ZMSPASSsoPlugin.py

Lines 544 to 553 in 0b06c2d

	classImplements( ZMSPASSsoPlugin
	, IZMSPASSsoPlugin
	, ILoginPasswordHostExtractionPlugin
	, IAuthenticationPlugin
	, IChallengePlugin
	, ICredentialsResetPlugin
	, IUserAdderPlugin
	, IUserEnumerationPlugin
	, IRolesPlugin
	)

[drfho]

Historical Reasons of the Problem

After building a testing env with a OIDC-server I could reconstruct the reason for the extensive use of doAddUser():
when UnBE shifts to OIDC-Auth there was a requirement to sync the user data, even changes of the data-model. As consequence the payload was save as fresh copy working as a source for the ZMS-User-Manager.

At any authentication applying zmsPluggableAuthService.authenticateCredentials():

Products.zmsPluggableAuthService/Products/zmsPluggableAuthService/plugins/ZMSPASSsoPlugin.py

Line 378 in 080c626

def authenticateCredentials( self, credentials, request=None ):

the PAS-user-dataset is extracted from the nginx-http-heaser and saved into the Zope-object:

Products.zmsPluggableAuthService/Products/zmsPluggableAuthService/plugins/ZMSPASSsoPlugin.py

Lines 396 to 401 in 080c626

	token = request.get(self.header_name, '')
	user_id_attrs = self.get_user_id_attrs()
	decoded_token = self.decryptToken(token)
	if decoded_token:
	# refresh users
	self.doAddUser(None, None)

Because the OIDC-datamodel may get creativly customized und there are a lot of time-based values, the payload will change at any auth-event and thus cause a data-refresh - and thus multiply the ZODB-data-amount.
Besides the lack of a binding data-model that allows a more selective check of "data have changed " there is second fundamental problem:
as consequence of calling doAddUser() in authenticateCredentials() the call happens extremly often - and maybe in 99.99% for no reason!

Solution: Strategic Goals

So, if we need to sync the user-data, we should

1. minimize/avoid unneccessary calls of doAddUser()
2. make the checking wether data have changed more specific- not just if users.get(user_id,{}) != decoded_token: (what are the oidc-keys? how do we define/save them?)
3. ZMS does not map the Zope-username to the OIDC-user-id if the key is unusual. Then ZMS shows the user-icon as "broken". Actually this is misleading because in general we do need NO user-data saved in the zmsPluggableAuthService-object for a usual OIDC dataflow (its a UniBE special case); ZMS shall the user-icon not show as "broken", maybe as muted, cloudy etc. (because the user-data are actually somewhere else)
4. We should evaluate whether we can avoid the whole data-saving flow if the plugin is declared as a non-active user_adder (see above)