removing the oidc.rp plugin removes too many configuration files

Description

I’ve removed the oidc.rp plugin, intending to still let the IDP keep as an OP. However, the removal deleted files from net.shibboleth.idp.plugin.oidc.op:

       conf/attributes/oidc-claim-rules.xml removed
       conf/oidc-credentials.xml renamed to, conf/oidc-credentials.xml.idpsave

Which is actually not correct, I need to keep these for the OP functionality.

Environment

None

Activity

Show:

Philip SmartJanuary 13, 2024 at 10:30 AM

Good point. Because it was not in the first release of the config module, it will need some thought on how to add it now.

Rod WiddowsonJanuary 13, 2024 at 10:18 AM

I think that’s you’d need to think very carefully about the impact of this. It seems to me (at first blush) that the possibilities for the file to go AWOL during the change over are massive.

  • Install new commons, file is there no update

  • Update op, old version removes file, new version does not add it.

but that is speculation, my brain hurts even thinking about it

Philip SmartJanuary 13, 2024 at 10:12 AM
Edited

Yeah. I’m not sure why I did not do this. The credentials file was different between the two, but I moved out those differences to a separate file, so now the base file is the same. The config plugin does not install any user config at the moment (although it could of course), it just provides shared system config. I must have overlooked this and we should move this in.

Scott CantorJanuary 13, 2024 at 12:33 AM

I intended I think that they’d have been moved to the config plugin, that was kind of the point of it. I’m not sure what it’s managing now, but these are definitely candidates unless there’s some clear reason they’d be different between the two, and then we’d want to use different files for them I think.

Rod WiddowsonJanuary 12, 2024 at 10:48 AM

The customer work-around is to re-enable the module (OP?) installation not needed.

I looked and there isn’t a persistent property for installing files. It would be achievable, to add it but it would (AFAICS) need a extension to the Module#ModuleResource interface (which could be defaulted) and some wiring from there up. So it would be 5.1 only.

I think the architectural changes to where the file goes sound better but

(a) that ship has sailed
{b) because it is “someone else’s problem” I would obviously prefer it

Details

Assignee

Reporter

Affects versions

Created January 12, 2024 at 7:36 AM
Updated January 19, 2024 at 2:50 PM