Declare clientId a string, not a foreign key - #13
Merged
Conversation
An OpenID Connect client id is a string the provider chooses -- "fog-web",
or a GUID on Entra -- but it ends in "id", and FOGController::save() read
any key ending in "id" as an integer foreign key. clientId is required, so
the mismatch did not degrade anything: it made creating a provider
impossible, reported as
Required database field is empty: clientId
about a field the admin had filled in, surfacing in the UI as the generic
"Add provider failed!". No OIDC provider could be created at all, which
also means no provider group, and so the Role and User Group provider-group
tabs from #10 had nothing to associate.
fogproject#1153 stops the base class inferring a column's type from its
name and lets the model say so instead. This declares it.
The gate lives in the existing provider-safety test because this belongs
with the other silent ways a provider row can be wrong: dropping the
declaration does not make providers slightly worse, it switches provider
creation off entirely.
Requires fogproject#1153; inert without it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017aBSWrDArXHTpKWkkN27LR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An OpenID Connect client id is a string the provider chooses —
fog-web, or a GUID on Entra. It also ends in"id", andFOGController::save()read any key ending in"id"as an integer foreign key.Because
clientIdis required, that did not make provider rows slightly worse — it made creating one impossible:…about a field the admin had filled in, surfacing in the UI as the generic "Add provider failed!". And since no provider could exist, no provider group could exist either — so the Role and User Group provider-group tabs from #10 had nothing to associate. That is how it was found.
FOGProject/fogproject#1153 stops the base class inferring a column's type from its name and lets the model declare it instead. This is the declaration.
Verified against a live 1.6 database from a shadow tree carrying both changes:
clientId => 'fog-web'saves and reads back intact (opClientID = fog-web). Before the pair, the same call returnedfalse.The gate goes in the existing
oidc-provider-safety.test.phprather than a new file, because it belongs with the other silent ways a provider row can be wrong — dropping the declaration switches provider creation off entirely. Mutation-verified: removing the property fails the test with a message naming the cause.Requires FOGProject/fogproject#1153 — inert without it, and harmless: on an older core the property is simply an unread declaration.
🤖 Generated with Claude Code
https://claude.ai/code/session_017aBSWrDArXHTpKWkkN27LR