Updating identity on login

[robinknaapen]

I followed #3922 for the better part of the issue. Recently the pull got closed because another way was implemented outside this pull request 4fcb299

When I review the code + the understanding I have of Kratos' internal workings; I noticed that the Metatadata and Traits are being merged instead of updated. The merging is done using Mergo, the Go library I see being used in the code. But this seems to do a deep merge, which in some scenarios can cause unwanted behaviour. Below I've sketched some example I can think of.

Example:

// Old
{
  "a": "old value"
}

// New
{
  "a": "new value"
}

// Yields
{
  "a": "new value"
}

But in other cases, for example when omitting values in Jsonnet mappers (which I suppose is a general practice to reduce bloat in identities)

// Old
{
  "a": "old value"
}

// New
{
  // pretend that `a` is omitted in the Jsonnet mapper
}

// Yields
{
  "a": "old value"
}

In this case "a" is not touched and keeps existing

This forces you to always explicitly set a property to null, which could be a breaking change for people relying on the value being omitted

For example in Zitadel I know that the user's roles is a nested map
They will omit the roles when you are removed from them

If someone just maps the above into the Identity metadata, then the result will be a that all the roles will accumulate over time

Here is a more concrete example

package main

import (
	"encoding/json"
	"os"

	"dario.cat/mergo" // The package that is used to merge the identity
)

func main() {
	a := map[string]any{
		"urn:zitadel:iam:org:project:roles": map[string]any{
			"admin": map[string]any{
				"222222222222222222": "my-org-name",
			},
		},
	}

	b := map[string]any{
		"urn:zitadel:iam:org:project:roles": map[string]any{
			"editor": map[string]any{
				"222222222222222222": "my-org-name",
			},
		},
	}

	mergo.Merge(&b, a, mergo.WithOverride)

	e := json.NewEncoder(os.Stdout)
	e.SetIndent("", "  ")

	e.Encode(b)
	// output:
	// {
	//   "urn:zitadel:iam:org:project:roles": {
	//     "admin": {
	//       "222222222222222222": "my-org-name"
	//     },
	//     "editor": {
	//       "222222222222222222": "my-org-name"
	//     }
	//   }
	// }
}

Have these cases been considered while implementing the feature, or am I just being paranoid 😓?

[dfbustosus]

Your reading of the merge behavior is correct, and it's worth pinning to the code that landed — plus there's one edge in it that's sharper than your examples suggest.

The merge is in selfservice/strategy/oidc/form.go:

// merge merges the JSON messages in a and b. If a value is defined in both a and b, a wins.
func merge(a, b json.RawMessage) (identity.Traits, error) {
    if len(a) == 0 || string(a) == "{}" {
        return identity.Traits(b), nil   // empty mapper output -> existing traits kept entirely
    }
    ...
    // values in aMap (src) win.
    if err := mergo.Merge(&bMap, &aMap, mergo.Withoverride); err != nil { ... }

where a is the freshly mapped claims and b the existing traits. So: omitted keys survive, exactly as you sketched — and the early return means an empty Jsonnet result retains the full previous traits.

The sharper edge: your proposed workaround (explicitly emitting null to remove a key) doesn't actually work here. mergo.Merge with Withoverride only overrides destination values with non-empty source values — a JSON null unmarshals to a nil interface, which mergo treats as empty and skips. So with this implementation there is no mapper-level way to delete a trait key at all: omitted → retained, explicit null → also retained. Removal only exists outside the login path (Admin API / identity import). That's worth raising in the issue you'd open, because "set it to null" is the natural expectation and it silently does nothing.

On whether merge-instead-of-replace is defensible: it's the conservative default for claims-derived traits. OIDC claims are situational — scopes can be dropped between logins, consent can lapse, a field that was present at registration can be absent at the next login. Replace-semantics would turn any such omission into data loss on a routine sign-in, which is destructive and not reviewable by the user. Merge makes every login a no-op-or-narrowing update instead. The cost is exactly your case: staleness for keys your IdP stops asserting.

Practical patterns that work today:

1. Make your Jsonnet mapper always emit the complete desired trait set for the keys it owns — treat a family of traits as mapper-owned (IdP-managed) or user-owned (never mapper-touched), never mixed. Omission then only ever affects mapper-owned keys you've decided to keep asserting.
2. Do trait pruning in a web hook (after login) or a scheduled reconciliation via the Admin API, where you have full replace semantics and can log/audit the change — arguably the right home for "IdP said this user left group X" anyway, since it's a governance event rather than a sign-in side-effect.

If you do open the discussion-to-issue, the two concrete asks would be: (a) document that empty/omitted/null mapper output retains existing traits, and (b) decide whether WithOverwriteWithEmptyValue (or an explicit null-removes-key rule) is wanted — that single mergo option is the difference between your two examples.

[robinknaapen]

The sharper edge ... does nothing

This makes the merging even less intuitive, so even an explicit "null" value will be ignored. I've confirmed this in the code of the original code.

1:
Even when I map { "a": null } the value will remain the old value.
This could be a legit case for a service that has a nullable enum or something. So that would mean whole infra should be updated to accommodate for a new value in an enum that is not "null".

---

2:
The webhook could work, but that's not an easy feat to implement. It would require infra changes which are involved tasks and cost additional development time

---

It seems like there is no ergonomic way to update the identity after a login for people who are susceptible to the above mentioned issues.

In case of point 1; The way that for example Kratos is used in a legacy infra in our case:
We use Oathkeeper to create a JWT of the Identity and feed that to a number of services. In this case the impact of the updated example "enum" would hit all those services

Just as a disclaimer, I am not asking for practical solutions, there are a ton of things I can do to mitigate the issue, but I was wondering why the "merge" approach was chosen and if an "overwrite" flow was ever considered. In this scenario, it seems like low hanging fruit to support it, and let people just update their identity in a deterministic way via the mappers

[dfbustosus]

Since a webhook is out of scope for you right now, one middle path worth considering: the admin API (PUT /admin/identities/{id}) replaces traits wholesale rather than merging them, so a small sync job that recomputes the desired trait set and writes it via the admin endpoint sidesteps the Mergo semantics entirely — no webhook infra, and { "a": null } becomes expressible as "absent in the replacement payload". It's still an extra moving piece, but it's usually a ~20-line script on a timer instead of new infrastructure.

If the merge-semantics analysis plus this route answers the question, marking it would help others landing here from PR #3922.