Skip to content

Commit f40d05f

Browse files
authored
feat(sdk): sdk should optionally take in a logger (#2754)
If we want to move forward with this, I'll need to split the SDK and service changes out (stacked PRs) to avoid breaking `service` with unreleased `sdk` api changes. ### Key Benefits - User Control: Users decide where/how SDK logs appear - Integration: Works with existing logging infrastructure - Debugging: Users can see what SDK is doing internally - Production Ready: Different log levels for different environments - Compliance: Meets audit/regulatory requirements - Testing: Makes SDK behavior verifiable - No Surprises: SDK doesn't spam logs they can't control If stdout is used for SDK/tool result output (like in the CLI context of [otdfctl](https://github.com/opentdf/otdfctl)), logs can break tool utility.
1 parent 0f77246 commit f40d05f

13 files changed

+128
-61
lines changed

sdk/auth/oauth/oauth.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,6 @@ func processResponse(resp *http.Response) (*Token, error) {
196196
}
197197

198198
func getDPoPAssertion(dpopJWK jwk.Key, method string, endpoint string, nonce string) (string, error) {
199-
slog.Debug("building DPoP Proof")
200199
publicKey, err := jwk.PublicKeyOf(dpopJWK)
201200
const expirationTime = 5 * time.Minute
202201

sdk/bulk.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,15 +133,15 @@ func (s SDK) BulkDecrypt(ctx context.Context, opts ...BulkDecryptOption) error {
133133
return fmt.Errorf("retrieving platformEndpoint failed: %w", err)
134134
}
135135
// if no kasAllowlist is set, we get the allowlist from the registry
136-
allowlist, err := allowListFromKASRegistry(ctx, s.KeyAccessServerRegistry, platformEndpoint)
136+
allowlist, err := allowListFromKASRegistry(ctx, s.logger, s.KeyAccessServerRegistry, platformEndpoint)
137137
if err != nil {
138138
return fmt.Errorf("failed to get allowlist from registry: %w", err)
139139
}
140140
bulkReq.kasAllowlist = allowlist
141141
bulkReq.NanoTDFDecryptOptions = append(bulkReq.NanoTDFDecryptOptions, withNanoKasAllowlist(bulkReq.kasAllowlist))
142142
bulkReq.TDF3DecryptOptions = append(bulkReq.TDF3DecryptOptions, withKasAllowlist(bulkReq.kasAllowlist))
143143
} else {
144-
slog.Error("no KAS allowlist provided and no KeyAccessServerRegistry available")
144+
s.Logger().Error("no KAS allowlist provided and no KeyAccessServerRegistry available")
145145
return errors.New("no KAS allowlist provided and no KeyAccessServerRegistry available")
146146
}
147147
}
@@ -172,7 +172,7 @@ func (s SDK) BulkDecrypt(ctx context.Context, opts ...BulkDecryptOption) error {
172172
var err error
173173
for kasurl, rewrapRequests := range kasRewrapRequests {
174174
if bulkReq.ignoreAllowList {
175-
slog.Warn("kasAllowlist is ignored, kas url is allowed", slog.String("kas_url", kasurl))
175+
s.Logger().Warn("kasAllowlist is ignored, kas url is allowed", slog.String("kas_url", kasurl))
176176
} else if !bulkReq.kasAllowlist.IsAllowed(kasurl) {
177177
// if kas url is not allowed, the result for each kao in each rewrap request is set to error
178178
for _, req := range rewrapRequests {

sdk/fuzz_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"crypto/rand"
77
"encoding/base64"
88
"io"
9+
"log/slog"
910
"net/http"
1011
"testing"
1112

@@ -26,6 +27,7 @@ func writeBytes(writerFunc func(io.Writer) error) []byte {
2627
func newSDK() *SDK {
2728
key, _ := ocrypto.NewRSAKeyPair(tdf3KeySize)
2829
cfg := &config{
30+
logger: slog.Default(),
2931
kasSessionKey: &key,
3032
}
3133
sdk := &SDK{

sdk/granter.go

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,8 @@ func (a AttributeValueFQN) Name() string {
200200

201201
// Structure capable of generating a split plan from a given set of data tags.
202202
type granter struct {
203+
logger *slog.Logger
204+
203205
// The data attributes (tags) that this granter is responsible for.
204206
tags []AttributeValueFQN
205207

@@ -235,7 +237,7 @@ func (r *granter) addGrant(fqn AttributeValueFQN, kas string, attr *policy.Attri
235237
func (r *granter) addMappedKey(fqn AttributeValueFQN, sk *policy.SimpleKasKey) error {
236238
key := sk.GetPublicKey()
237239
if key == nil || key.GetKid() == "" || key.GetPem() == "" {
238-
slog.Debug("invalid cached key in policy service",
240+
r.logger.Debug("invalid cached key in policy service",
239241
slog.String("kas", sk.GetKasUri()),
240242
slog.Any("value", fqn),
241243
)
@@ -252,15 +254,15 @@ func (r *granter) addMappedKey(fqn AttributeValueFQN, sk *policy.SimpleKasKey) e
252254

253255
rl, err := NewResourceLocator(sk.GetKasUri())
254256
if err != nil {
255-
slog.Debug("invalid KAS URL in policy service",
257+
r.logger.Debug("invalid KAS URL in policy service",
256258
slog.String("kas", sk.GetKasUri()),
257259
slog.Any("value", fqn),
258260
slog.Any("error", err),
259261
)
260262
return fmt.Errorf("invalid KAS URL in policy service associated with [%s]: %w", fqn, err)
261263
}
262264
rl.identifier = key.GetKid()
263-
slog.Debug("added mapped key",
265+
r.logger.Debug("added mapped key",
264266
slog.Any("fqn", fqn),
265267
slog.String("kas", sk.GetKasUri()),
266268
slog.String("kid", key.GetKid()),
@@ -330,7 +332,7 @@ func (r *granter) addAllGrants(fqn AttributeValueFQN, ag grantableObject, attr *
330332
// Check for mapped keys
331333
for _, k := range ag.GetKasKeys() {
332334
if k == nil || k.GetKasUri() == "" {
333-
slog.Debug("invalid KAS key in policy service",
335+
r.logger.Debug("invalid KAS key in policy service",
334336
slog.Any("simple_kas_key", k),
335337
slog.Any("value", fqn),
336338
)
@@ -341,7 +343,7 @@ func (r *granter) addAllGrants(fqn AttributeValueFQN, ag grantableObject, attr *
341343
result = r.typ
342344
err := r.addMappedKey(fqn, k)
343345
if err != nil {
344-
slog.Debug("failed to add mapped key",
346+
r.logger.Debug("failed to add mapped key",
345347
slog.Any("fqn", fqn),
346348
slog.String("kas", kasURI),
347349
slog.Any("error", err),
@@ -367,7 +369,7 @@ func (r *granter) addAllGrants(fqn AttributeValueFQN, ag grantableObject, attr *
367369
for _, k := range g.GetKasKeys() {
368370
err := r.addMappedKey(fqn, k)
369371
if err != nil {
370-
slog.Warn("failed to add mapped key",
372+
r.logger.Warn("failed to add mapped key",
371373
slog.Any("fqn", fqn),
372374
slog.String("kas", kasURI),
373375
slog.Any("error", err),
@@ -378,15 +380,15 @@ func (r *granter) addAllGrants(fqn AttributeValueFQN, ag grantableObject, attr *
378380
}
379381
ks := g.GetPublicKey().GetCached().GetKeys()
380382
if len(ks) == 0 {
381-
slog.Debug("no cached key in policy service",
383+
r.logger.Debug("no cached key in policy service",
382384
slog.String("kas", kasURI),
383385
slog.Any("value", fqn),
384386
)
385387
continue
386388
}
387389
for _, k := range ks {
388390
if k.GetKid() == "" || k.GetPem() == "" {
389-
slog.Debug("invalid cached key in policy service",
391+
r.logger.Debug("invalid cached key in policy service",
390392
slog.String("kas", kasURI),
391393
slog.Any("value", fqn),
392394
slog.Any("key", k),
@@ -404,7 +406,7 @@ func (r *granter) addAllGrants(fqn AttributeValueFQN, ag grantableObject, attr *
404406
}
405407
err := r.addMappedKey(fqn, sk)
406408
if err != nil {
407-
slog.Warn("failed to add mapped key",
409+
r.logger.Warn("failed to add mapped key",
408410
slog.Any("fqn", fqn),
409411
slog.String("kas", kasURI),
410412
slog.Any("error", err),
@@ -426,7 +428,7 @@ func (r granter) byAttribute(fqn AttributeValueFQN) *keyAccessGrant {
426428
}
427429

428430
// Gets a list of directory of KAS grants for a list of attribute FQNs
429-
func newGranterFromService(ctx context.Context, keyCache *kasKeyCache, as sdkconnect.AttributesServiceClient, fqns ...AttributeValueFQN) (granter, error) {
431+
func newGranterFromService(ctx context.Context, logger *slog.Logger, keyCache *kasKeyCache, as sdkconnect.AttributesServiceClient, fqns ...AttributeValueFQN) (granter, error) {
430432
fqnsStr := make([]string, len(fqns))
431433
for i, v := range fqns {
432434
fqnsStr[i] = v.String()
@@ -443,6 +445,7 @@ func newGranterFromService(ctx context.Context, keyCache *kasKeyCache, as sdkcon
443445
}
444446

445447
grants := granter{
448+
logger: logger,
446449
tags: fqns,
447450
grantTable: make(map[string]*keyAccessGrant),
448451
keyCache: &rlKeyCache{c: make(map[ResourceLocator]*policy.SimpleKasKey)},
@@ -455,23 +458,23 @@ func newGranterFromService(ctx context.Context, keyCache *kasKeyCache, as sdkcon
455458
def := pair.GetAttribute()
456459

457460
if def != nil {
458-
storeKeysToCache(def.GetGrants(), def.GetKasKeys(), keyCache, grants.keyCache)
461+
storeKeysToCache(logger, def.GetGrants(), def.GetKasKeys(), keyCache, grants.keyCache)
459462
}
460463
v := pair.GetValue()
461464
gType := noKeysFound
462465
if v != nil {
463466
gType = grants.addAllGrants(fqn, v, def)
464-
storeKeysToCache(v.GetGrants(), v.GetKasKeys(), keyCache, grants.keyCache)
467+
storeKeysToCache(logger, v.GetGrants(), v.GetKasKeys(), keyCache, grants.keyCache)
465468
}
466469

467470
// If no more specific grant was found, then add the value grants
468471
if gType == noKeysFound && def != nil {
469472
gType = grants.addAllGrants(fqn, def, def)
470-
storeKeysToCache(def.GetGrants(), def.GetKasKeys(), keyCache, grants.keyCache)
473+
storeKeysToCache(logger, def.GetGrants(), def.GetKasKeys(), keyCache, grants.keyCache)
471474
}
472475
if gType == noKeysFound && def.GetNamespace() != nil {
473476
grants.addAllGrants(fqn, def.GetNamespace(), def)
474-
storeKeysToCache(def.GetNamespace().GetGrants(), def.GetNamespace().GetKasKeys(), keyCache, grants.keyCache)
477+
storeKeysToCache(logger, def.GetNamespace().GetGrants(), def.GetNamespace().GetKasKeys(), keyCache, grants.keyCache)
475478
}
476479
}
477480

@@ -515,11 +518,11 @@ func algProto2OcryptoKeyType(e policy.Algorithm) ocrypto.KeyType {
515518
}
516519
}
517520

518-
func storeKeysToCache(kases []*policy.KeyAccessServer, keys []*policy.SimpleKasKey, c *kasKeyCache, kc *rlKeyCache) {
521+
func storeKeysToCache(logger *slog.Logger, kases []*policy.KeyAccessServer, keys []*policy.SimpleKasKey, c *kasKeyCache, kc *rlKeyCache) {
519522
for _, kas := range kases {
520523
keys := kas.GetPublicKey().GetCached().GetKeys()
521524
if len(keys) == 0 {
522-
slog.Debug("no cached key in policy service", slog.String("kas", kas.GetUri()))
525+
logger.Debug("no cached key in policy service", slog.String("kas", kas.GetUri()))
523526
continue
524527
}
525528
for _, ki := range keys {
@@ -535,7 +538,7 @@ func storeKeysToCache(kases []*policy.KeyAccessServer, keys []*policy.SimpleKasK
535538
if kc != nil && ki.GetKid() != "" && ki.GetPem() != "" {
536539
rl, err := NewResourceLocator(kas.GetUri())
537540
if err != nil {
538-
slog.Debug("failed to create ResourceLocator",
541+
logger.Debug("failed to create ResourceLocator",
539542
slog.String("kas", kas.GetUri()),
540543
slog.Any("error", err),
541544
)
@@ -570,7 +573,7 @@ func storeKeysToCache(kases []*policy.KeyAccessServer, keys []*policy.SimpleKasK
570573
if kc != nil && key.GetPublicKey().GetKid() != "" && key.GetPublicKey().GetPem() != "" {
571574
rl, err := NewResourceLocator(key.GetKasUri())
572575
if err != nil {
573-
slog.Debug("failed to create ResourceLocator",
576+
logger.Debug("failed to create ResourceLocator",
574577
slog.String("kas", key.GetKasUri()),
575578
slog.Any("error", err),
576579
)
@@ -585,8 +588,9 @@ func storeKeysToCache(kases []*policy.KeyAccessServer, keys []*policy.SimpleKasK
585588
// Given a policy (list of data attributes or tags),
586589
// get a set of grants from attribute values to KASes.
587590
// Unlike `newGranterFromService`, this works offline.
588-
func newGranterFromAttributes(keyCache *kasKeyCache, attrs ...*policy.Value) (granter, error) {
591+
func newGranterFromAttributes(logger *slog.Logger, keyCache *kasKeyCache, attrs ...*policy.Value) (granter, error) {
589592
grants := granter{
593+
logger: logger,
590594
grantTable: make(map[string]*keyAccessGrant),
591595
mapTable: make(map[string][]*ResourceLocator),
592596
tags: make([]AttributeValueFQN, len(attrs)),
@@ -608,16 +612,16 @@ func newGranterFromAttributes(keyCache *kasKeyCache, attrs ...*policy.Value) (gr
608612
}
609613

610614
if grants.addAllGrants(fqn, v, def) != noKeysFound {
611-
storeKeysToCache(v.GetGrants(), v.GetKasKeys(), keyCache, grants.keyCache)
615+
storeKeysToCache(logger, v.GetGrants(), v.GetKasKeys(), keyCache, grants.keyCache)
612616
continue
613617
}
614618
// If no more specific grant was found, then add the attr grants
615619
if grants.addAllGrants(fqn, def, def) != noKeysFound {
616-
storeKeysToCache(def.GetGrants(), def.GetKasKeys(), keyCache, grants.keyCache)
620+
storeKeysToCache(logger, def.GetGrants(), def.GetKasKeys(), keyCache, grants.keyCache)
617621
continue
618622
}
619623
grants.addAllGrants(fqn, namespace, def)
620-
storeKeysToCache(namespace.GetGrants(), namespace.GetKasKeys(), keyCache, grants.keyCache)
624+
storeKeysToCache(logger, namespace.GetGrants(), namespace.GetKasKeys(), keyCache, grants.keyCache)
621625
}
622626

623627
return grants, nil
@@ -846,7 +850,7 @@ func (r *granter) insertKeysForAttribute(e attributeBooleanExpression) (booleanK
846850
var err error
847851
rl, err = NewResourceLocator(kas)
848852
if err != nil {
849-
slog.Warn("invalid KAS URL in policy service",
853+
r.logger.Warn("invalid KAS URL in policy service",
850854
slog.String("kas", kas),
851855
slog.Any("value", term),
852856
slog.Any("error", err),
@@ -859,7 +863,7 @@ func (r *granter) insertKeysForAttribute(e attributeBooleanExpression) (booleanK
859863
}
860864
op := ruleToOperator(clause.def.GetRule())
861865
if op == unspecified {
862-
slog.Warn("unknown attribute rule type", slog.Any("rule", clause))
866+
r.logger.Warn("unknown attribute rule type", slog.Any("rule", clause))
863867
}
864868
kc := keyClause{
865869
operator: op,
@@ -888,7 +892,7 @@ func (r *granter) assignKeysTo(e attributeBooleanExpression) (booleanKeyExpressi
888892
}
889893
op := ruleToOperator(clause.def.GetRule())
890894
if op == unspecified {
891-
slog.Warn("unknown attribute rule type", slog.Any("rule", clause))
895+
r.logger.Warn("unknown attribute rule type", slog.Any("rule", clause))
892896
}
893897
kc := keyClause{
894898
operator: op,

sdk/granter_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package sdk
33
import (
44
"context"
55
"errors"
6+
"log/slog"
67
"maps"
78
"regexp"
89
"slices"
@@ -568,7 +569,7 @@ func TestConfigurationServicePutGet(t *testing.T) {
568569
} {
569570
t.Run(tc.n, func(t *testing.T) {
570571
v := valuesToPolicy(tc.policy...)
571-
grants, err := newGranterFromAttributes(newKasKeyCache(), v...)
572+
grants, err := newGranterFromAttributes(slog.Default(), newKasKeyCache(), v...)
572573
require.NoError(t, err)
573574
assert.Len(t, grants.grantTable, tc.size)
574575
assert.Subset(t, policyToStringKeys(tc.policy), slices.Collect(maps.Keys(grants.grantTable)))
@@ -726,7 +727,7 @@ func TestReasonerConstructAttributeBoolean(t *testing.T) {
726727
},
727728
} {
728729
t.Run(tc.n, func(t *testing.T) {
729-
reasoner, err := newGranterFromAttributes(newKasKeyCache(), valuesToPolicy(tc.policy...)...)
730+
reasoner, err := newGranterFromAttributes(slog.Default(), newKasKeyCache(), valuesToPolicy(tc.policy...)...)
730731
require.NoError(t, err)
731732

732733
reasoner.keyInfoFetcher = &fakeKeyInfoFetcher{}
@@ -874,7 +875,7 @@ func TestReasonerSpecificity(t *testing.T) {
874875
},
875876
} {
876877
t.Run(tc.n, func(t *testing.T) {
877-
reasoner, err := newGranterFromService(t.Context(), newKasKeyCache(), &mockAttributesClient{}, tc.policy...)
878+
reasoner, err := newGranterFromService(t.Context(), slog.Default(), newKasKeyCache(), &mockAttributesClient{}, tc.policy...)
878879
require.NoError(t, err)
879880
i := 0
880881
plan, err := reasoner.plan(tc.defaults, func() string {
@@ -1025,7 +1026,7 @@ func TestReasonerSpecificityWithNamespaces(t *testing.T) {
10251026
},
10261027
} {
10271028
t.Run((tc.n + "\n" + tc.desc), func(t *testing.T) {
1028-
reasoner, err := newGranterFromService(t.Context(), newKasKeyCache(), &mockAttributesClient{}, tc.policy...)
1029+
reasoner, err := newGranterFromService(t.Context(), slog.Default(), newKasKeyCache(), &mockAttributesClient{}, tc.policy...)
10291030
require.NoError(t, err)
10301031
i := 0
10311032
plan, err := reasoner.plan(tc.defaults, func() string {

sdk/idp_access_token_source.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package sdk
33
import (
44
"context"
55
"fmt"
6-
"log/slog"
76
"net/http"
87
"net/url"
98
"sync"
@@ -60,7 +59,10 @@ type IDPAccessTokenSource struct {
6059
}
6160

6261
func NewIDPAccessTokenSource(
63-
credentials oauth.ClientCredentials, idpTokenEndpoint string, scopes []string, key *ocrypto.RsaKeyPair,
62+
credentials oauth.ClientCredentials,
63+
idpTokenEndpoint string,
64+
scopes []string,
65+
key *ocrypto.RsaKeyPair,
6466
) (*IDPAccessTokenSource, error) {
6567
endpoint, err := url.Parse(idpTokenEndpoint)
6668
if err != nil {
@@ -73,6 +75,7 @@ func NewIDPAccessTokenSource(
7375
}
7476

7577
tokenSource := IDPAccessTokenSource{
78+
// logger: log,
7679
credentials: credentials,
7780
idpTokenEndpoint: *endpoint,
7881
token: nil,
@@ -87,12 +90,11 @@ func NewIDPAccessTokenSource(
8790
}
8891

8992
// AccessToken use a pointer receiver so that the token state is shared
90-
func (t *IDPAccessTokenSource) AccessToken(ctx context.Context, client *http.Client) (auth.AccessToken, error) {
93+
func (t *IDPAccessTokenSource) AccessToken(_ context.Context, client *http.Client) (auth.AccessToken, error) {
9194
t.tokenMutex.Lock()
9295
defer t.tokenMutex.Unlock()
9396

9497
if t.token == nil || t.token.Expired() {
95-
slog.DebugContext(ctx, "getting new access token")
9698
tok, err := oauth.GetAccessToken(client, t.idpTokenEndpoint.String(), t.scopes, t.credentials, t.dpopKey)
9799
if err != nil {
98100
return "", fmt.Errorf("error getting access token: %w", err)

sdk/idp_cert_exchange.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package sdk
22

33
import (
44
"context"
5+
"log/slog"
56
"net/http"
67
"sync"
78

@@ -13,6 +14,7 @@ import (
1314

1415
type CertExchangeTokenSource struct {
1516
auth.AccessTokenSource
17+
logger *slog.Logger
1618
IdpEndpoint string
1719
credentials oauth.ClientCredentials
1820
tokenMutex *sync.Mutex
@@ -21,13 +23,14 @@ type CertExchangeTokenSource struct {
2123
key jwk.Key
2224
}
2325

24-
func NewCertExchangeTokenSource(info oauth.CertExchangeInfo, credentials oauth.ClientCredentials, idpTokenEndpoint string, dpop *ocrypto.RsaKeyPair) (auth.AccessTokenSource, error) {
26+
func NewCertExchangeTokenSource(logger *slog.Logger, info oauth.CertExchangeInfo, credentials oauth.ClientCredentials, idpTokenEndpoint string, dpop *ocrypto.RsaKeyPair) (auth.AccessTokenSource, error) {
2527
_, dpopKey, _, err := getNewDPoPKey(dpop)
2628
if err != nil {
2729
return nil, err
2830
}
2931

3032
exchangeSource := CertExchangeTokenSource{
33+
logger: logger,
3134
info: info,
3235
IdpEndpoint: idpTokenEndpoint,
3336
credentials: credentials,

0 commit comments

Comments
 (0)