From 132f66fd06151e17b99bdd50416951dd491447e1 Mon Sep 17 00:00:00 2001 From: Krish Suchak Date: Tue, 30 Jun 2026 18:03:26 -0400 Subject: [PATCH 1/6] feat(sdk): resolve key splits via GetKeyMappingsByFqns Switch newGranterFromService to resolve effective KAS keys through the new GetKeyMappingsByFqns RPC (server-side value > definition > namespace precedence). Values that return no mapped keys (legacy-grant-only) fall back per-value to GetAttributeValuesByFqns, and older platforms that do not expose the RPC (Unimplemented) fall back entirely, so behavior is preserved for un-migrated policy and older servers. Adds fallback parity unit tests. Signed-off-by: Krish Suchak --- sdk/granter.go | 102 ++++++++++++++++++++++++++++++++++++-------- sdk/granter_test.go | 70 ++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 17 deletions(-) diff --git a/sdk/granter.go b/sdk/granter.go index 571616efa0..5b305202bd 100644 --- a/sdk/granter.go +++ b/sdk/granter.go @@ -10,6 +10,8 @@ import ( "sort" "strings" + "connectrpc.com/connect" + "github.com/opentdf/platform/lib/ocrypto" "github.com/opentdf/platform/protocol/go/policy" "github.com/opentdf/platform/protocol/go/policy/attributes" @@ -419,55 +421,121 @@ func (r granter) byAttribute(fqn AttributeValueFQN) *keyAccessGrant { return r.grantTable[fqn.key] } -// Gets a list of directory of KAS grants for a list of attribute FQNs +// Gets a list of directory of KAS grants for a list of attribute FQNs. +// +// Resolution uses GetKeyMappingsByFqns, which returns the effective mapped KAS +// keys per value (value > definition > namespace precedence, resolved +// server-side). Values that resolve to no mapped keys (e.g. configured only with +// legacy KAS grants) fall back to GetAttributeValuesByFqns so their grants still +// resolve. func newGranterFromService(ctx context.Context, logger *slog.Logger, keyCache *kasKeyCache, as sdkconnect.AttributesServiceClient, fqns ...AttributeValueFQN) (granter, error) { fqnsStr := make([]string, len(fqns)) for i, v := range fqns { fqnsStr[i] = v.String() } - av, err := as.GetAttributeValuesByFqns(ctx, &attributes.GetAttributeValuesByFqnsRequest{ - Fqns: fqnsStr, - }) - if err != nil { - return granter{}, err - } - grants := granter{ logger: logger, tags: fqns, grantTable: make(map[string]*keyAccessGrant), keyCache: &rlKeyCache{c: make(map[ResourceLocator]*policy.SimpleKasKey)}, } - for fqnstr, pair := range av.GetFqnAttributeValues() { + + km, err := as.GetKeyMappingsByFqns(ctx, &attributes.GetKeyMappingsByFqnsRequest{Fqns: fqnsStr}) + if err != nil { + // Older platforms do not expose GetKeyMappingsByFqns; fall back to the full + // attribute lookup for all values. + if connect.CodeOf(err) == connect.CodeUnimplemented { + if e := grants.addGrantsFromAttributeValues(ctx, logger, keyCache, as, fqnsStr); e != nil { + return grants, e + } + return grants, nil + } + return granter{}, err + } + + var fallback []string + for fqnstr, m := range km.GetFqnKeyMappings() { fqn, err := NewAttributeValueFQN(fqnstr) if err != nil { return grants, err } + keys := m.GetKeys() + if len(keys) == 0 { + // No mapped keys (legacy-grant-only or unconfigured); resolve via the + // full attribute lookup below. + fallback = append(fallback, fqnstr) + continue + } + // Only the rule and FQN are read downstream (constructAttributeBoolean / + // ruleToOperator), so a minimal attribute is sufficient. + attr := &policy.Attribute{Fqn: fqn.Prefix().String(), Rule: m.GetRule()} + for _, sk := range keys { + if err := grants.addMappedKey(fqn, sk); err != nil { + logger.Debug("failed to add mapped key", + slog.Any("fqn", fqn), + slog.Any("error", err), + ) + continue + } + grants.typ = mappedFound + if _, present := grants.grantTable[fqn.key]; !present { + grants.grantTable[fqn.key] = &keyAccessGrant{attr, []string{sk.GetKasUri()}} + } else { + grants.grantTable[fqn.key].kases = append(grants.grantTable[fqn.key].kases, sk.GetKasUri()) + } + // Populate the SDK key cache for wrapping (addMappedKey already filled + // the granter's rlKeyCache). + storeKeysToCache(logger, nil, []*policy.SimpleKasKey{sk}, keyCache, nil) + } + } + + if len(fallback) > 0 { + if err := grants.addGrantsFromAttributeValues(ctx, logger, keyCache, as, fallback); err != nil { + return grants, err + } + } + + return grants, nil +} + +// addGrantsFromAttributeValues resolves grants for the given value FQNs via the +// full GetAttributeValuesByFqns lookup (value > definition > namespace), used as +// the fallback for values with no mapped keys. +func (r *granter) addGrantsFromAttributeValues(ctx context.Context, logger *slog.Logger, keyCache *kasKeyCache, as sdkconnect.AttributesServiceClient, fqns []string) error { + av, err := as.GetAttributeValuesByFqns(ctx, &attributes.GetAttributeValuesByFqnsRequest{Fqns: fqns}) + if err != nil { + return err + } + for fqnstr, pair := range av.GetFqnAttributeValues() { + fqn, err := NewAttributeValueFQN(fqnstr) + if err != nil { + return err + } def := pair.GetAttribute() if def != nil { - storeKeysToCache(logger, def.GetGrants(), def.GetKasKeys(), keyCache, grants.keyCache) + storeKeysToCache(logger, def.GetGrants(), def.GetKasKeys(), keyCache, r.keyCache) } v := pair.GetValue() gType := noKeysFound if v != nil { - gType = grants.addAllGrants(fqn, v, def) - storeKeysToCache(logger, v.GetGrants(), v.GetKasKeys(), keyCache, grants.keyCache) + gType = r.addAllGrants(fqn, v, def) + storeKeysToCache(logger, v.GetGrants(), v.GetKasKeys(), keyCache, r.keyCache) } // If no more specific grant was found, then add the value grants if gType == noKeysFound && def != nil { - gType = grants.addAllGrants(fqn, def, def) - storeKeysToCache(logger, def.GetGrants(), def.GetKasKeys(), keyCache, grants.keyCache) + gType = r.addAllGrants(fqn, def, def) + storeKeysToCache(logger, def.GetGrants(), def.GetKasKeys(), keyCache, r.keyCache) } if gType == noKeysFound && def.GetNamespace() != nil { - grants.addAllGrants(fqn, def.GetNamespace(), def) - storeKeysToCache(logger, def.GetNamespace().GetGrants(), def.GetNamespace().GetKasKeys(), keyCache, grants.keyCache) + r.addAllGrants(fqn, def.GetNamespace(), def) + storeKeysToCache(logger, def.GetNamespace().GetGrants(), def.GetNamespace().GetKasKeys(), keyCache, r.keyCache) } } - return grants, nil + return nil } func algProto2String(e policy.KasPublicKeyAlgEnum) string { diff --git a/sdk/granter_test.go b/sdk/granter_test.go index bf29139347..fd59cde660 100644 --- a/sdk/granter_test.go +++ b/sdk/granter_test.go @@ -11,6 +11,8 @@ import ( "strings" "testing" + "connectrpc.com/connect" + "github.com/opentdf/platform/lib/ocrypto" "github.com/opentdf/platform/protocol/go/policy" "github.com/opentdf/platform/protocol/go/policy/attributes" @@ -812,6 +814,74 @@ func (*mockAttributesClient) GetAttributeValuesByFqns(_ context.Context, req *at }, nil } +// effectiveMockKasKeys mirrors the server-side resolveEffectiveKasKeys: the +// most-specific mapped keys with value > definition > namespace precedence. +func effectiveMockKasKeys(v *policy.Value, a *policy.Attribute) []*policy.SimpleKasKey { + if k := v.GetKasKeys(); len(k) > 0 { + return k + } + if k := a.GetKasKeys(); len(k) > 0 { + return k + } + if k := a.GetNamespace().GetKasKeys(); len(k) > 0 { + return k + } + return nil +} + +func (*mockAttributesClient) GetKeyMappingsByFqns(_ context.Context, req *attributes.GetKeyMappingsByFqnsRequest) (*attributes.GetKeyMappingsByFqnsResponse, error) { + out := make(map[string]*attributes.GetKeyMappingsByFqnsResponse_AttributeKeyMapping) + for _, v := range req.GetFqns() { + vfqn, err := NewAttributeValueFQN(v) + if err != nil { + return nil, err + } + val := mockValueFor(vfqn) + attr := val.GetAttribute() + out[v] = &attributes.GetKeyMappingsByFqnsResponse_AttributeKeyMapping{ + Rule: attr.GetRule(), + Keys: effectiveMockKasKeys(val, attr), + } + } + return &attributes.GetKeyMappingsByFqnsResponse{FqnKeyMappings: out}, nil +} + +// unimplementedKeyMappingsClient models an older platform that does not expose +// GetKeyMappingsByFqns; the granter must fall back to GetAttributeValuesByFqns. +type unimplementedKeyMappingsClient struct { + mockAttributesClient +} + +func (*unimplementedKeyMappingsClient) GetKeyMappingsByFqns(_ context.Context, _ *attributes.GetKeyMappingsByFqnsRequest) (*attributes.GetKeyMappingsByFqnsResponse, error) { + return nil, connect.NewError(connect.CodeUnimplemented, errors.New("not implemented")) +} + +// TestReasonerKeyMappingFallback verifies that grant-only values and older +// platforms (no GetKeyMappingsByFqns) resolve to the same plan as the key-mapping +// path, via the GetAttributeValuesByFqns fallback. +func TestReasonerKeyMappingFallback(t *testing.T) { + policy := []AttributeValueFQN{spk2spk} // grant-based value (no mapped keys) + defaults := []string{kasUs} + want := []keySplitStep{{evenMoreSpecificKas, ""}} + noSplit := func() string { return "" } + + for _, tc := range []struct { + name string + as sdkconnect.AttributesServiceClient + }{ + {"per-value fallback (empty key set)", &mockAttributesClient{}}, + {"full fallback (RPC unimplemented)", &unimplementedKeyMappingsClient{}}, + } { + t.Run(tc.name, func(t *testing.T) { + reasoner, err := newGranterFromService(t.Context(), slog.Default(), newKasKeyCache(), tc.as, policy...) + require.NoError(t, err) + plan, err := reasoner.plan(defaults, noSplit) + require.NoError(t, err) + assert.ElementsMatch(t, want, plan) + }) + } +} + // Tests titles are written in the form [{attr}.{value}] => [{resulting kas boolean exp}] // where the left hand side is the list of attributes passed in and the right // is the resulting split steps From a1b863683aecf98695915c0a5105f17ff06ae383 Mon Sep 17 00:00:00 2001 From: Krish Suchak Date: Thu, 2 Jul 2026 10:12:21 -0400 Subject: [PATCH 2/6] fix(sdk): address PR review comments - iterate requested FQNs (not the response map) in newGranterFromService so a value omitted from GetKeyMappingsByFqns falls back instead of being dropped; rely on nil-safe protobuf getters (no redundant present/nil guard) - return granter{} on error paths - test the FQN-omitted-from-response fallback path Signed-off-by: Krish Suchak --- sdk/granter.go | 22 ++++++++++++---------- sdk/granter_test.go | 12 ++++++++++++ 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/sdk/granter.go b/sdk/granter.go index 5b305202bd..64cd6c7ce5 100644 --- a/sdk/granter.go +++ b/sdk/granter.go @@ -447,24 +447,26 @@ func newGranterFromService(ctx context.Context, logger *slog.Logger, keyCache *k // attribute lookup for all values. if connect.CodeOf(err) == connect.CodeUnimplemented { if e := grants.addGrantsFromAttributeValues(ctx, logger, keyCache, as, fqnsStr); e != nil { - return grants, e + return granter{}, e } return grants, nil } return granter{}, err } + // Iterate the requested FQNs (not the response map) so a value the server + // omits still falls back rather than being silently dropped. The response is + // keyed by normalized FQN, and the protobuf getters are nil-safe, so an absent + // or nil entry yields empty keys and resolves via the fallback below. + mappings := km.GetFqnKeyMappings() var fallback []string - for fqnstr, m := range km.GetFqnKeyMappings() { - fqn, err := NewAttributeValueFQN(fqnstr) - if err != nil { - return grants, err - } + for _, fqn := range fqns { + m := mappings[fqn.key] keys := m.GetKeys() if len(keys) == 0 { - // No mapped keys (legacy-grant-only or unconfigured); resolve via the - // full attribute lookup below. - fallback = append(fallback, fqnstr) + // No mapped keys (legacy-grant-only, unconfigured, or omitted); resolve + // via the full attribute lookup below. + fallback = append(fallback, fqn.key) continue } // Only the rule and FQN are read downstream (constructAttributeBoolean / @@ -492,7 +494,7 @@ func newGranterFromService(ctx context.Context, logger *slog.Logger, keyCache *k if len(fallback) > 0 { if err := grants.addGrantsFromAttributeValues(ctx, logger, keyCache, as, fallback); err != nil { - return grants, err + return granter{}, err } } diff --git a/sdk/granter_test.go b/sdk/granter_test.go index fd59cde660..63acae6ea9 100644 --- a/sdk/granter_test.go +++ b/sdk/granter_test.go @@ -856,6 +856,17 @@ func (*unimplementedKeyMappingsClient) GetKeyMappingsByFqns(_ context.Context, _ return nil, connect.NewError(connect.CodeUnimplemented, errors.New("not implemented")) } +// emptyKeyMappingsClient models a server that succeeds but omits the requested +// FQNs from the response map; the granter must fall back per value rather than +// silently drop them. +type emptyKeyMappingsClient struct { + mockAttributesClient +} + +func (*emptyKeyMappingsClient) GetKeyMappingsByFqns(_ context.Context, _ *attributes.GetKeyMappingsByFqnsRequest) (*attributes.GetKeyMappingsByFqnsResponse, error) { + return &attributes.GetKeyMappingsByFqnsResponse{}, nil +} + // TestReasonerKeyMappingFallback verifies that grant-only values and older // platforms (no GetKeyMappingsByFqns) resolve to the same plan as the key-mapping // path, via the GetAttributeValuesByFqns fallback. @@ -871,6 +882,7 @@ func TestReasonerKeyMappingFallback(t *testing.T) { }{ {"per-value fallback (empty key set)", &mockAttributesClient{}}, {"full fallback (RPC unimplemented)", &unimplementedKeyMappingsClient{}}, + {"per-value fallback (FQN omitted from response)", &emptyKeyMappingsClient{}}, } { t.Run(tc.name, func(t *testing.T) { reasoner, err := newGranterFromService(t.Context(), slog.Default(), newKasKeyCache(), tc.as, policy...) From e206b3280b3bd0741c23ee569c09a9e067fc1b54 Mon Sep 17 00:00:00 2001 From: Krish Suchak Date: Thu, 2 Jul 2026 12:54:07 -0400 Subject: [PATCH 3/6] test(sdk): cover mixed mappings and grants in granter Signed-off-by: Krish Suchak --- sdk/granter_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/sdk/granter_test.go b/sdk/granter_test.go index 63acae6ea9..15a04b2da4 100644 --- a/sdk/granter_test.go +++ b/sdk/granter_test.go @@ -894,6 +894,30 @@ func TestReasonerKeyMappingFallback(t *testing.T) { } } +// A single request that mixes mapped-key values (MP: mapped kas_keys) with +// grant-configured values (REL: legacy grants) must resolve every attribute and +// combine them into one plan. The server returns keys for both (cached-key grants +// are converted alongside mapped keys), so the plan should carry the mapped KAS +// and both grant KASes. +func TestReasonerMixedMappingsAndGrants(t *testing.T) { + policy := []AttributeValueFQN{mpa, mpb, rel2aus, rel2can} + reasoner, err := newGranterFromService(t.Context(), slog.Default(), newKasKeyCache(), &mockAttributesClient{}, policy...) + require.NoError(t, err) + + i := 0 + plan, err := reasoner.plan([]string{kasUs}, func() string { + i++ + return strconv.Itoa(i) + }) + require.NoError(t, err) + + got := make(map[string]bool) + for _, s := range plan { + got[s.KAS] = true + } + assert.ElementsMatch(t, []string{evenMoreSpecificKas, kasAu, kasCa}, slices.Collect(maps.Keys(got))) +} + // Tests titles are written in the form [{attr}.{value}] => [{resulting kas boolean exp}] // where the left hand side is the list of attributes passed in and the right // is the resulting split steps From ceec587ef02466e8c4b2ca192174901f965cda70 Mon Sep 17 00:00:00 2001 From: Krish Suchak Date: Thu, 2 Jul 2026 13:16:54 -0400 Subject: [PATCH 4/6] test(sdk): use empty struct for kas set in granter test Signed-off-by: Krish Suchak --- sdk/granter_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/granter_test.go b/sdk/granter_test.go index 15a04b2da4..2c3776937f 100644 --- a/sdk/granter_test.go +++ b/sdk/granter_test.go @@ -911,9 +911,9 @@ func TestReasonerMixedMappingsAndGrants(t *testing.T) { }) require.NoError(t, err) - got := make(map[string]bool) + got := make(map[string]struct{}) for _, s := range plan { - got[s.KAS] = true + got[s.KAS] = struct{}{} } assert.ElementsMatch(t, []string{evenMoreSpecificKas, kasAu, kasCa}, slices.Collect(maps.Keys(got))) } From daceec523edb6224128d2f3009fe6ed5ae3691e9 Mon Sep 17 00:00:00 2001 From: Krish Suchak Date: Thu, 2 Jul 2026 14:19:33 -0400 Subject: [PATCH 5/6] fix(sdk): warn on failed mapped key add in granter Signed-off-by: Krish Suchak --- sdk/granter.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/granter.go b/sdk/granter.go index 64cd6c7ce5..b0d257284d 100644 --- a/sdk/granter.go +++ b/sdk/granter.go @@ -474,8 +474,9 @@ func newGranterFromService(ctx context.Context, logger *slog.Logger, keyCache *k attr := &policy.Attribute{Fqn: fqn.Prefix().String(), Rule: m.GetRule()} for _, sk := range keys { if err := grants.addMappedKey(fqn, sk); err != nil { - logger.Debug("failed to add mapped key", + logger.Warn("failed to add mapped key", slog.Any("fqn", fqn), + slog.String("kas", sk.GetKasUri()), slog.Any("error", err), ) continue From 673d69d77398e1064546f52ee8156d0281e78dc8 Mon Sep 17 00:00:00 2001 From: Krish Suchak Date: Thu, 2 Jul 2026 14:26:51 -0400 Subject: [PATCH 6/6] test(sdk): assert full plan shape in mixed granter test Signed-off-by: Krish Suchak --- sdk/granter_test.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/sdk/granter_test.go b/sdk/granter_test.go index 2c3776937f..170d788651 100644 --- a/sdk/granter_test.go +++ b/sdk/granter_test.go @@ -911,11 +911,16 @@ func TestReasonerMixedMappingsAndGrants(t *testing.T) { }) require.NoError(t, err) - got := make(map[string]struct{}) - for _, s := range plan { - got[s.KAS] = struct{}{} + // The mapped attribute (MP) forms its own split; the grant attribute (REL, + // ANY_OF) shares a split across its two KASes. Assert the full plan (KAS plus + // split id) rather than a KAS set, so mapped-key splits stay distinct instead + // of collapsing by KAS. + want := []keySplitStep{ + {evenMoreSpecificKas, "1"}, + {kasAu, "2"}, + {kasCa, "2"}, } - assert.ElementsMatch(t, []string{evenMoreSpecificKas, kasAu, kasCa}, slices.Collect(maps.Keys(got))) + assert.ElementsMatch(t, want, plan) } // Tests titles are written in the form [{attr}.{value}] => [{resulting kas boolean exp}]