Filter available authentication providers for FICAM setting#7857
Filter available authentication providers for FICAM setting#7857labkey-adam wants to merge 7 commits into
Conversation
|
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?.():
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. |
…rn if providers aren't found because of this setting.
cnathe
left a comment
There was a problem hiding this comment.
Code changes make sense. Looking at this in the UI, it is a little odd to get totally empty dropdowns and to hide things that are already configured for filtered out cases when this compliance setting is enabled, but it seems very edge case that a server would actually have this setting enabled without having valid auth options. We could definitely clean some of that UI part up in a future story / change if needed.
| this.onToggleModal('deleteModalOpen', this.state.deleteModalOpen); | ||
| toggleModalOpen(false); |
There was a problem hiding this comment.
minor: these two lines are shared with the onCancel above. Could factor out a closeModal function that they both use/call, but up to you.
There was a problem hiding this comment.
I factored these out. Take a look, if you'd like.
Agreed that the UI is a bit goofy in this mode. Better would be to hide these drop-down buttons if empty. Also agree that this is a fairly obscure setting; I don't know of anyone using it currently. Note that we have no true authentication providers that are "FICAM approved" (other than database auth, but that's never in the drop-downs), so these lists will always be empty when the setting is on. |
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