Security API Change Proposal: If configured settler is not available, drop back to XWiki default behaviour

Development
,
#1

Hi,

I have a proposal regarding the Security Module that I would appreciate being considered. I am more than happy to implement this change given the go ahead from the XWiki team. If the stretch goal mentioned below is possible within the XWiki framework, then I am happy to tackle that also but I would need guidance from the XWiki team on how best to approach it.

Proposal

Change the Security Module implementation to drop back to default behaviour if the class/component referenced by security.authorization.settler is not present.

Stretch Goal

A stretch goal to this proposal would be for the Security Module to (efficiently) react to the deployment and undeployment of the extension containing the class/component that represents the value defined by security.authorization.settler to avoid having to restart XWiki.

Background / Reasoning

As per the Security Module documentation, it is possible to set a security.authorization.settler property within xwiki.properties. Doing so will cause the Security Module to consult that “settler” when making Rights decisions.

The recommended approach for adding/customising capabilities to XWiki is to bundle them in an extension and to deploy that extension using the Extension Manager. As such, it is quite likely that the implementation behind the value of security.authorization.settler is going to be in an extension. This distinction is important because the presence of an extension can be considered more transient than components/libraries contained in the WEB-INF/lib folder of XWiki. Using the Extension Manager to install the code behind a custom “settler” is attractive due to all of the benefits using the Extension Manager provides (dependency management/checking, etc.)

If the security.authorization.settler property is set but the class/component in an extension referred to by that property is not present/deployed, XWiki fails to start properly. This makes building a new XWiki environment more challenging from an operations standpoint as it requires an ordered multi-step process as follows:

  1. Start XWiki with no security.authorization.settler set.
  2. Deploy the extension containing the custom “settler”.
  3. Set the security.authorization.settler property in xwiki.properties
  4. Restart XWiki.

It also means that uninstalling the extension has greater impact than your average extension. Rather than defaulting back to XWiki default behaviour, restarting XWiki without also resetting the security.authorization.settler property will consequently lead to a failed XWiki. The person managing that process would need to be aware of the “special” behaviour of this extension and the configuration value.

Cheers,
Alex

#2

This generally involves writing an EventListener following ComponentDescriptorAddedEvent (and ComponentDescriptorRemovedEvent) and, if the event is about a component with role AuthorizationSettler and the configured hint (and for this specific use case I would probably add making sure that the component is registered on the root namespace to be safe), switch the AuthorizationSettlerProvider#authorizationSettler accordingly. By the way, since this change could have an impact on the cached right checking decisions (that’s kind of the point of changing the AuthorizationSettler :slight_smile: ), it also means that the security cache need to be flushed when that happen.

Fallbacks which don’t make obvious enough that there is something wrong and “we are just fallbacking but this should be fixed ASAP” (and, by experience, logging a warning is not) can cause important problems before you finally realize it. On the other hand we are doing exactly that already with the authenticator so it would be consistent (but it tends to be a little more obvious for the authenticator that the default one is used instead of the expected one).

#3

Thanks @tmortagne

Thanks for the stretch goal guidance, it makes sense. I’d wondered whether there would be an event to listen for but didn’t know the specifics.

Thinking about this further, if the Security Module is changed to automatically pick up a settler provided in an extension, maybe the proposal should be changed to remove the settler config property entirely? The Security Module is either using it’s built-in default or, it finds a component with the AuthorizationSettler role and uses that instead.

and for this specific use case I would probably add making sure that the component is registered on the root namespace to be safe

Does that mean added to the WEB-INF/lib folder or am I misunderstanding?

I agree with your comments regarding fallback behaviour. This particular area is in my opinion between a rock and a hard place where either outcome is not ideal; however, I lean slightly more towards reverting to defaults because:

  • If you have a running setup, you’re in a position to rectify the problem by (re)installing the extension. If XWiki fails to start, you are unable to do that. The person performing the maintenance has to know about the specific property in order to disable it and rectify the problem. Even then, you need to get XWiki back into a running state where you can install the extension, re-enable the property and restart XWiki. It’s generally more complicated and time consuming.
  • On the whole, this scenario is less likely to happen in a production environment as it will typically be going through upgrades, and the process to apply that upgrade has (hopefully) been tested/verified, planned and the necessary steps are known. As such, the proposed change has little impact either way. Of course, things can always go wrong even with best laid plans but, in those circumstances I think I’d rather have an XWiki that comes up so it’s quicker to remediate the issue. Having said that, it still does rely on you knowing there is a problem and that’s where failing hard/fast does make it obvious that something is wrong.
  • The proposed behaviour sees a greater benefit when building new environments and especially for automation. It means you can have a simpler provisioning process that applies the necessary xwiki.cfg and xwiki.properties files without modification, install xwiki, install the extensions and finish with an XWiki restart, if necessary. If the stretch goal were implemented, the need for a restart in this particular case would also be removed. With the settler property as it is, you somewhat need “before” and “after” properties file(s). This makes deployment more complicated and significantly more difficult to automate.

Is the proposal acceptable in principle with the only the issue being one of awareness in the failure case? If so, I’d like to think (but maybe I’m being too optimistic :slightly_smiling_face:) that install/upgrade processes would involve some sanity testing of functionality to help mitigate issues but, are there any ideas on how it could be made more apparent?

If the property were removed entirely as I mentioned above, then I don’t think there is anything that can be done to indicate the mismatch in state between the properties file and the deployed extensions; the custom settler is either deployed and in use or, it isn’t.

Cheers

#4

No. The root namespace is what is called “On farm” in the Extension Manager UI, it means making sure that this component is shared among all wikis.

#5

Let’s say I’m OK with that on the basis that it’s a pretty rare use case and customizing the AuthorizationSettler involves people who really know what they are doing.

Still, would be nice to have other inputs :slight_smile:

I don’t think it’s really related to the installation process, but I guess a possibility would to be to insert some persistent warning banner in the UI when the configured AuthorizationSettler is not the one currently returned by the provider and which would disappear only when the situation is resolved.

#6

No. The root namespace is what is called “On farm” in the Extension Manager UI, it means making sure that this component is shared among all wikis.

Thanks, I now understand.

Given your earlier comment…

…that would mean that for the settler to be picked up the user would either have to use the “Install on farm” option when installing the extension or (not 100% confident I have this right…) add the <xwiki.extension.namespaces>{root}</xwiki.extension.namespaces> property in the extension’s pom.xml to “force” the “Install on farm” setting. Is that right?

If so, that means the Security Manager would use the injected default ComponentManager which I believe is for the root namespace and try to load the setter from there? Or, are there extra steps required to meet/enforce that constraint?

Of course, I’ll wait until there is further input from others before going any further than just this discussion.

Would you expect that to be displayed to just the Admin or all users? I think I’d lean towards only showing to Admins. It’s fairly useless to non-admins apart from possibly causing a normal user to notify the admin, and again that feels “too late”. If there is a banner message provided by XWiki does that require translations?

Does that also mean you would still be in favour of retaining the settler property as it might not necessarily be required if the Security Manager is automatically looking for an AuthorizationSettler component?

Cheers

#7

Yes.

That’s already what AuthorizationSettlerProvider is doing. The problem is that, according to your tests, AuthorizationSettlerProvider is doing that too early (before the installed extensions are loaded) and the reason is just that it’s probably injected directly or indirectly by some listener somewhere since we don’t really do any right check before loading the installed extensions. The ways to fix that is either tracking what in XWiki Standard cause this early initialization of AuthorizationSettlerProvider and make it lazier or make AuthorizationSettlerProvider itself lazier (load the AuthorizationSettler the first time AuthorizationSettlerProvider#get() is called instead of when initializing AuthorizationSettlerProvider).

I would expect it to show up everywhere but only to users with admin right on main wiki. I would also make this banner a bit more generic than just this use case (for example, the permanent dir suffer from the same problem: the configured path is not used before it does not exist or XWiki cannot write in it).

Ideally, but what this mean for the author of this code is just to make that banner translatable (define translation keys and provide the English version) and let l10n.xwiki.org contributors translate it when they can.

You still need to indicate somewhere which AuthorizationSettler component you want to use. Just in XWiki Standard there is 2 (“default” and “priority”).

#8

Hi,

@tmortagne was there anyone else in the xwiki team that had any opinions on this proposal that you are aware of?

Cheers

#9

Just sent a little push for inputs on the chat.

#10

Great, thank you!

#11

Coming from that push :slight_smile: (I’ve seen the topic but didn’t took time yet to answer it, sorry for the delay)

So all in all I agree with the global idea of being able to easily switch the AuthorizationSettler component whenever an extension provides another implementation for it.

Now IMO this component is a sensitive part of XWiki and I think we should be very careful for that one.
So first, concerning the fallback, I agree with @alewis001 to fallback on the default implementation in case of any issue, with a proper log for the arguments exposed below

Now regarding:

I’m really not a big fan of that idea.
First we should support backward compatibility of this property, so it will be hard to remove it, we could only deprecate it basically. But even without talking about backward compatibility, I do think that we should preserve this property and allow to override it at higher level (e.g. in a configuration page of the farm).

Which leads us to my final point: from what I understood you want Admins of a farm to be able to switch the Authorization settler of the wiki when an extensions is installed that provides a new one. I think that should never be an automated process when installing an extension: IMO it should be a dedicated Administration UI to list the available settlers and to allow change them, and it should be made extra clear that this can have a big impact on the security of the wiki, since basically you can do whatever you want with a specific implementation of the Security settler, such as giving programming rights to everyone. So this UI would be a manual activation to perform when the extension is installed, but would not require the restart of the wiki or an operation to do on the configuration files.

#12

Thank you @surli for your response!

Yes, good point.

Yes and no :slight_smile: What prompted me to raise this proposal was that the settler property could cause XWiki to fail to start (the property is set but the extension not deployed). This is problematic for new installs when you may already have a “template” config that you could reuse. I.e. you need to know about the specific behaviour of the property, known when to (un)set it, restart XWiki, etc. and that felt like an unnecessary operational complication. The higher the number of steps and the required knowledge introduces risk. Attempting to automate that process also becomes far more complicated.

Rather than “to switch the Authorization settler of the wiki when an extensions is installed that provides a new one.” suggesting “any” extension and any “any” settler therein, my proposal was only for an extension that contains the component with the right settler hint as set by the settler property. As configuring the property and installing extensions is (hopefully) done by an authorised User, that would provide enough protection from a badly behaving extension being deployed.

As per my comment above, it would only be “automatic” in the sense it’s loading the settler as defined by the hint in the settler config property.

Although that would somewhat complicate the automation aspect compared to what I’ve proposed, it would fit in with other post-install operations and I assume could still be automated via the REST API. Having said that, this feels like an extension or follow up to the proposal rather than gating the efficacy of it. If I’ve understood, what you’re proposing is a more dynamic means of managing the Authorization Settler through the UI rather than having to use the xwiki.properties file which may or may not be seen as less “user-friendly”.

On a point of backwards compatibility, I suspect this new UI could only be active when the settler property is not set, otherwise the behaviour of that property is somewhat changed because it could be ignored by this additional layer of configuration. Maybe the UI would be visible but disabled with a prompt to say that the the settler has already been “statically” configured in the properties file. If that property is not set, then the UI would be enabled and you could pick from any available settler and switch between them at will.

On the face of it seems like a nice additional enhancement if creating “settlers” is something done often, as is switching between them.

Cheers

#13

Ok I read back the very first proposal and I get it better. This looks ok to me, it should probably be safe to rely on the hint written in a file. It’s even probably more secure than what I propose. So I’m +1 for this.

Now just to clarify I don’t manage to understand what you proposed there:

I mainly relied on that one for my previous answer, and I fail to see how it fits the first proposal you made. If you remove the config property, where do you put the hint you were mentioning?

yep that’s it. Note that it’s already a common practice for our configuration, to first be checked in some wiki pages, and then to fallbacks on properties defined in the configuration file, so I was just reusing this pattern. If I’m correct we already have some generic components to ease this job.
Now I completely agree that it could be done afterwards.

#14

Sorry, my bad for the confusion. My original proposal didn’t suggest removing the settler property from config. As I thought it about it more, I wondered whether the property would still be needed and voiced that idea above. I’d forgotten about the built-in experimental “priority” settler as mentioned by @tmortagne, and I think the general feeling is that having the property makes it more explicit about what the user/admin intends. I.e. Without the property, you could deploy two extensions that contained a settler and depending on extension loading you’d maybe get a different settler on each XWiki start so, I think there’s good reason to keep the property (apart from backwards compatibility).

Ah I see, then the behaviour of your suggestion would make sense.

Cheers