Skip to content

identity: areMe converts storage errors into false-negative ownership answers #2066

Description

@adecaro

Summary

Provider.areMe (backing both AreMe and IsMe) returns whatever partial result it has accumulated
when the storage lookup for signer existence errors, rather than propagating the error. This means a
transient storage failure makes IsMe report false ("not mine") for an identity that actually is
ours, which is on the token-ownership decision path.

Where

token/services/identity/provider.go:270-279:

// check Storage
found, err := p.storage.GetExistingSignerInfo(ctx, notFound...)
if err != nil {
	p.Logger.Errorf("failed checking if a signer exists [%s]", err)
	return result.ToSlice()
}
result.Add(found...)
return result.ToSlice()

On error, result contains only whatever was already resolved from the in-memory cache
(provider.go:257-264) before the storage call — everything not in cache is silently reported as
"not mine," logged only at Errorf with no error propagated to the caller.

Related: token/services/identity/wallet/service.go:138-149 Wallet() similarly discards two errors
from OwnerWallet/IssuerWallet and returns nil (a caller cannot distinguish "no wallet" from
"lookup failed"):

func (s *Service) Wallet(ctx context.Context, identity tdriver.Identity) tdriver.Wallet {
	w, _ := s.OwnerWallet(ctx, identity)
	if w != nil {
		return w
	}
	iw, _ := s.IssuerWallet(ctx, identity)
	if iw != nil {
		return iw
	}
	return nil
}

Impact

IsMe is used to decide ownership-related behavior (e.g. whether to react to a token as an owned
token). A transient storage error causing a false negative means an owned token can be treated as
not-owned for that call, with no visible error — the caller has no signal that the answer is
unreliable rather than authoritative.

Reproduction

Not yet committed. Mock Storage.GetExistingSignerInfo to return an error for a set of identities
that includes at least one genuinely-owned identity not already warm in the in-memory cache; assert
IsMe/AreMe either propagates the error or is documented as best-effort at the call sites that rely
on it for correctness-sensitive decisions.

Suggested fix

AreMe/IsMe's current signatures ([]string / bool, no error return) make it structurally unable
to distinguish "confirmed not mine" from "couldn't check." Changing the signature ripples through the
driver.IdentityProvider interface and every caller, so the minimal fix here is: keep the signatures,
but do not silently swallow storage errors into a negative result for the specific identities the
storage call was about
— instead of returning early, consider retrying once, or making the
error-swallowing explicit and loud enough (metric increment, not just a log line) that an operator can
detect ownership answers were degraded. If callers need a hard guarantee, a follow-up to thread an
error return through AreMe is worth considering separately.

Severity

MEDIUM — requires a transient storage failure to trigger, and produces an unsignaled false negative
on an ownership decision.

Metadata

Metadata

Assignees

Type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions