Skip to content

Conversation

@usavkov-epam
Copy link
Contributor

@usavkov-epam usavkov-epam commented Jan 27, 2026

Purpose

https://folio-org.atlassian.net/browse/STSMACOM-938

  • Prevent accidental deletion or modification of the system-created originaltenantid custom field (which breaks identity-provider links and prevents logins).
  • Hide system custom fields from Settings → Users → Custom fields so admins cannot remove or edit them via the UI.

Approach

  • UI safeguard: filter system fields out of the Custom Fields Settings views and edit form so they are not presented to admins.
  • Detection:
    • Primary mechanism: hide fields whose refId matches known system ref ids (CUSTOM_FIELDS_SYSTEM_REF_IDS includes originaltenantid).
    • Additional system refIds can be passed via the systemFields prop to the settings pages for tenant-specific or future system fields.
  • Implementation summary:
    • Add CUSTOM_FIELDS_SYSTEM_REF_IDS constant with ['originaltenantid'].
    • Add excludeSystemCustomFields(systemFields, customFields) utility that removes fields whose refId is in the union of CUSTOM_FIELDS_SYSTEM_REF_IDS and the systemFields prop.
    • Handling form initialization, taking into account system fields intended to be hidden from view.

Screen recording

Screen.Recording.2026-01-27.at.22.16.45.mov

@usavkov-epam usavkov-epam self-assigned this Jan 27, 2026
@github-actions
Copy link

github-actions bot commented Jan 27, 2026

Jest Unit Test Results

31 tests  +9   31 ✅ +9   32s ⏱️ -1s
 7 suites +1    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit e9676b2. ± Comparison against base commit 759b487.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Jan 27, 2026

Bigtest Unit Test Results

  1 files  ±0    1 suites  ±0   23s ⏱️ ±0s
515 tests ±0  500 ✅ ±0  15 💤 ±0  0 ❌ ±0 
518 runs  ±0  503 ✅ ±0  15 💤 ±0  0 ❌ ±0 

Results for commit e9676b2. ± Comparison against base commit 759b487.

♻️ This comment has been updated with latest results.

@sonarqubecloud
Copy link

@usavkov-epam usavkov-epam marked this pull request as ready for review January 27, 2026 18:30
@usavkov-epam usavkov-epam requested a review from a team as a code owner January 27, 2026 18:30
Copy link
Member

@zburke zburke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like the right solution to the wrong problem, like giving somebody a lead-lined wallet to prevent their uranium credit card from poisoning them. Sure, it works, but a better solution is not to manufacture uranium credit cards in the first place. I get it; I wrote UIU-2499 (folio-org/ui-users#2013) and here I will add the same caution: a UI-only fix isn't really a fix at all because a determined user can still get at this data via APIs.

Additionally, the cognitive dissonance of a "system" custom-field is extraordinary. What is "custom" about it if it's being provided by the system itself? And if originaltenantid is necessary for authentication, why is that data being stored in a custom field?!?

Did we bump this up a level to find out from an architect if this is a good solution? Maybe the boss/architect says, "Yeah, I understand that manufacturing radioactive credit cards is problematic, but that's not your problem to solve today. Today I need you to focus on lead-lined wallets." then I grudgingly approved this nicely-implemented PR. Still, my hope is the boss says, "Whoops, you're right, adding lead will cause its own problems. Instead of doing that, we should back up and see if this whole situation can be avoided."

@usavkov-epam
Copy link
Contributor Author

Did we bump this up a level to find out from an architect if this is a good solution?

@SerhiiNosko, Maybe you can better describe the architectural approach with custom fields?

@zburke
Copy link
Member

zburke commented Jan 28, 2026

@usavkov-epam, @SerhiiNosko: It's less "Is there somebody who understands the big-picture architecture who can explain it to Captain Crankypants @zburke?" than "Is there somebody who understands the big-picture who can look at the side-effects this PR has to deal with and confirm, 'Yep, we understand the side-effects. Every approach has tradeoffs, and we determined this solution is the best one.'"

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.

5 participants