Skip to content

Filter available authentication providers for FICAM setting#7857

Open
labkey-adam wants to merge 3 commits into
developfrom
fb_ficam
Open

Filter available authentication providers for FICAM setting#7857
labkey-adam wants to merge 3 commits into
developfrom
fb_ficam

Conversation

@labkey-adam

@labkey-adam labkey-adam commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Rationale

When the "Accept only FICAM-approved third-party identity service providers" compliance setting was turned on, the authentication configuration page was correctly filtering out non-FICAM configurations (everything but DB login), but was not filtering the provider lists. As a result, one could attempt to add a new configuration in this mode, which would then result in an NPE. This has likely been broken since the authentication configuration page was redesigned. https://github.com/LabKey/internal-issues/issues/1145

Changes

  • Mark ResetPasswordProvider, DisableLoginProvider, and ExpireAccountProvider as "FICAM-approved"
  • Filter the cached AuthenticationProviders based on FICAM setting. If FICAM-only is checked, non-FICAM providers disappear. This is equivalent to how AuthenticationConfigurations have been handled.
  • Remove explicit FICAM filtering of configurations since provider filtering effectively does this now
  • Clear the AuthenticationProviderCache whenever the FICAM setting changes, same as AuthenticationConfigurationCache
  • Fix failure mode handling in AuthRow.tsx so the delete confirmation dialog disappears in case of an error. See below for details on this change.

@labkey-adam

Copy link
Copy Markdown
Contributor Author

Claude helped me fix an issue with client-side error handling in AuthRow.tsx. In the corner case where the authentication configuration page is opened (by User A) and then the FICAM setting is turned on in a different tab or session, User A can still attempt to delete and add configurations, since they still appear on that page. If delete is attempted, the confirmation dialog appears, "Yes, Delete" is clicked, and a reasonable error message now appears on the main panel, but the dialog remains open.

Here's Claude's summary of the fix for this:

Root cause: The delete confirmation Modal's onConfirm was wired directly to the fire-and-forget onDelete Ajax call (AuthenticationConfiguration.onDelete). The confirmation modal's open/closed state lives locally in AuthRow (deleteModalOpen), but nothing ever closed it on the delete request's failure path — the failure callback only sets error state on the top-level component, which renders in the main window. On success the modal disappeared only incidentally, because the deleted row's config gets filtered out of state and the whole row (modal included) unmounts.

Your FICAM server change makes the failure path reachable in normal use: when FICAM-only mode is toggled on in another tab/session, AuthenticationConfigurationCache.clear() drops the now-disallowed provider's configuration, so deleteConfiguration throws the new NotFoundException — leaving the modal stranded open.

Fix (AuthRow.tsx, the delete modal's onConfirm): close the confirmation modal as soon as the user confirms, regardless of outcome. It now mirrors the existing onCancel handler — toggling deleteModalOpen off and calling toggleModalOpen(false) — then invokes onDelete?.():

  • Success: row unmounts as before; closing early is harmless.
  • Failure: modal closes, and the error surfaces in the main window as intended.

As a bonus, routing through toggleModalOpen(false) on confirm also resets the parent panel's modalOpen/isDragDisabled flag, which the old onConfirm={onDelete} never did — so drag-and-drop is correctly re-enabled after a delete instead of staying disabled.

One thing worth confirming on your end: this closes the modal immediately on confirm rather than waiting for the server round-trip. Given onDelete is fire-and-forget (no promise to await) and the request is fast, that's the pragmatic fix. If you'd prefer the modal to stay open with a spinner until the request resolves (using the Modal's isConfirming/confirmingText props), that would require threading a promise back from onDelete — happy to do that instead if you want the more elaborate UX.

@labkey-adam
labkey-adam requested a review from cnathe July 18, 2026 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant