Skip to content

Commit cfa4c81

Browse files
hiddecohiddeco
committed
Tweak token credential composition once again
- `authorityHost` and `clientCertificateSendChain` can now be set where applicable. - AZ CLI fields have been removed. - Fallback to `ChainedTokenCredential` with `EnvironmentCredential` and `ManagedIdentityCredential` with defaults if no Secret is given. Signed-off-by: Hidde Beydals <[email protected]>
1 parent f5d7a37 commit cfa4c81

File tree

2 files changed

+123
-95
lines changed

2 files changed

+123
-95
lines changed

‎pkg/azure/blob.go

Lines changed: 111 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -44,18 +44,14 @@ var (
4444
)
4545

4646
const (
47-
resourceIDField = "resourceId"
48-
clientIDField = "clientId"
49-
tenantIDField = "tenantId"
50-
clientSecretField = "clientSecret"
51-
clientCertificateField = "clientCertificate"
52-
clientCertificatePasswordField = "clientCertificatePassword"
53-
accountKeyField = "accountKey"
54-
55-
// Ref: https://docs.microsoft.com/en-us/azure/aks/kubernetes-service-principal?tabs=azure-cli#manually-create-a-service-principal
56-
tenantField = "tenant"
57-
appIDField = "appId"
58-
passwordField = "password"
47+
clientIDField = "clientId"
48+
tenantIDField = "tenantId"
49+
clientSecretField = "clientSecret"
50+
clientCertificateField = "clientCertificate"
51+
clientCertificatePasswordField = "clientCertificatePassword"
52+
clientCertificateSendChainField = "clientCertificateSendChain"
53+
authorityHostField = "authorityHost"
54+
accountKeyField = "accountKey"
5955
)
6056

6157
// BlobClient is a minimal Azure Blob client for fetching objects.
@@ -83,39 +79,48 @@ type BlobClient struct {
8379
// - azblob.SharedKeyCredential when an `accountKey` field is found.
8480
// The account name is extracted from the endpoint specified on the Bucket
8581
// object.
82+
// - azidentity.ChainedTokenCredential with azidentity.EnvironmentCredential
83+
// and azidentity.ManagedIdentityCredential with defaults if no Secret is
84+
// given.
8685
//
87-
// If no credentials are found, a simple client without credentials is
88-
// returned.
86+
// If no credentials are found, and the azidentity.ChainedTokenCredential can
87+
// not be established. A simple client without credentials is returned.
8988
func NewClient(obj *sourcev1.Bucket, secret *corev1.Secret) (c *BlobClient, err error) {
9089
c = &BlobClient{}
9190

92-
// Without a Secret, we can return a simple client.
93-
if secret == nil || len(secret.Data) == 0 {
94-
c.ServiceClient, err = azblob.NewServiceClientWithNoCredential(obj.Spec.Endpoint, nil)
95-
return
96-
}
97-
98-
// Attempt AAD Token Credential options first.
9991
var token azcore.TokenCredential
100-
if token, err = tokenCredentialFromSecret(secret); err != nil {
101-
return
102-
}
103-
if token != nil {
104-
c.ServiceClient, err = azblob.NewServiceClient(obj.Spec.Endpoint, token, nil)
105-
return
106-
}
10792

108-
// Fallback to Shared Key Credential.
109-
cred, err := sharedCredentialFromSecret(obj.Spec.Endpoint, secret)
110-
if err != nil {
111-
return
93+
if secret != nil && len(secret.Data) > 0 {
94+
// Attempt AAD Token Credential options first.
95+
if token, err = tokenCredentialFromSecret(secret); err != nil {
96+
err = fmt.Errorf("failed to create token credential from '%s' Secret: %w", secret.Name, err)
97+
return
98+
}
99+
if token != nil {
100+
c.ServiceClient, err = azblob.NewServiceClient(obj.Spec.Endpoint, token, nil)
101+
return
102+
}
103+
104+
// Fallback to Shared Key Credential.
105+
var cred *azblob.SharedKeyCredential
106+
if cred, err = sharedCredentialFromSecret(obj.Spec.Endpoint, secret); err != nil {
107+
return
108+
}
109+
if cred != nil {
110+
c.ServiceClient, err = azblob.NewServiceClientWithSharedKey(obj.Spec.Endpoint, cred, &azblob.ClientOptions{})
111+
return
112+
}
112113
}
113-
if cred != nil {
114-
c.ServiceClient, err = azblob.NewServiceClientWithSharedKey(obj.Spec.Endpoint, cred, &azblob.ClientOptions{})
115-
return
114+
115+
// Compose token chain based on environment.
116+
// This functions as a replacement for azidentity.NewDefaultAzureCredential
117+
// to not shell out.
118+
if token, err = chainCredentialWithSecret(secret); err != nil {
119+
err = fmt.Errorf("failed to create environment credential chain: %w", err)
120+
return nil, err
116121
}
117122

118-
// Secret does not contain a valid set of credentials, fallback to simple client.
123+
// Fallback to simple client.
119124
c.ServiceClient, err = azblob.NewServiceClientWithNoCredential(obj.Spec.Endpoint, nil)
120125
return
121126
}
@@ -138,26 +143,19 @@ func ValidateSecret(secret *corev1.Secret) error {
138143
}
139144
}
140145
}
141-
if _, hasTenant := secret.Data[tenantField]; hasTenant {
142-
if _, hasAppID := secret.Data[appIDField]; hasAppID {
143-
if _, hasPassword := secret.Data[passwordField]; hasPassword {
144-
valid = true
145-
}
146-
}
147-
}
148-
if _, hasResourceID := secret.Data[resourceIDField]; hasResourceID {
149-
valid = true
150-
}
151146
if _, hasClientID := secret.Data[clientIDField]; hasClientID {
152147
valid = true
153148
}
154149
if _, hasAccountKey := secret.Data[accountKeyField]; hasAccountKey {
155150
valid = true
156151
}
152+
if _, hasAuthorityHost := secret.Data[authorityHostField]; hasAuthorityHost {
153+
valid = true
154+
}
157155

158156
if !valid {
159-
return fmt.Errorf("invalid '%s' secret data: requires a '%s', '%s', or '%s' field, a combination of '%s', '%s' and '%s', or '%s', '%s' and '%s'",
160-
secret.Name, resourceIDField, clientIDField, accountKeyField, tenantIDField, clientIDField, clientSecretField, tenantIDField, clientIDField, clientCertificateField)
157+
return fmt.Errorf("invalid '%s' secret data: requires a '%s' or '%s' field, a combination of '%s', '%s' and '%s', or '%s', '%s' and '%s'",
158+
secret.Name, clientIDField, accountKeyField, tenantIDField, clientIDField, clientSecretField, tenantIDField, clientIDField, clientCertificateField)
161159
}
162160
return nil
163161
}
@@ -285,40 +283,61 @@ func (c *BlobClient) ObjectIsNotFound(err error) bool {
285283
return false
286284
}
287285

286+
// tokenCredentialsFromSecret attempts to create an azcore.TokenCredential
287+
// based on the data fields of the given Secret. It returns, in order:
288+
// - azidentity.ClientSecretCredential when `tenantId`, `clientId` and
289+
// `clientSecret` fields are found.
290+
// - azidentity.ClientSecretCredential when `tenant`, `appId` and `password`
291+
// fields are found. To match with the JSON from:
292+
// https://docs.microsoft.com/en-us/azure/aks/kubernetes-service-principal?tabs=azure-cli#manually-create-a-service-principal
293+
// - azidentity.ClientCertificateCredential when `tenantId`,
294+
// `clientCertificate` (and optionally `clientCertificatePassword`) fields
295+
// are found.
296+
// - azidentity.ManagedIdentityCredential for a User ID, when a `clientId`
297+
// field but no `tenantId` is found.
298+
// - azidentity.ManagedIdentityCredential for a Resource ID, when a
299+
// `resourceId` field is found.
300+
// - Nil, if no valid set of credential fields was found.
288301
func tokenCredentialFromSecret(secret *corev1.Secret) (azcore.TokenCredential, error) {
302+
if secret == nil {
303+
return nil, nil
304+
}
305+
289306
clientID, hasClientID := secret.Data[clientIDField]
290307
if tenantID, hasTenantID := secret.Data[tenantIDField]; hasTenantID && hasClientID {
291308
if clientSecret, hasClientSecret := secret.Data[clientSecretField]; hasClientSecret && len(clientSecret) > 0 {
292-
return azidentity.NewClientSecretCredential(string(tenantID), string(clientID), string(clientSecret), nil)
309+
opts := &azidentity.ClientSecretCredentialOptions{}
310+
if authorityHost, hasAuthorityHost := secret.Data[authorityHostField]; hasAuthorityHost {
311+
opts.AuthorityHost = azidentity.AuthorityHost(authorityHost)
312+
}
313+
return azidentity.NewClientSecretCredential(string(tenantID), string(clientID), string(clientSecret), opts)
293314
}
294315
if clientCertificate, hasClientCertificate := secret.Data[clientCertificateField]; hasClientCertificate && len(clientCertificate) > 0 {
295316
certs, key, err := azidentity.ParseCertificates(clientCertificate, secret.Data[clientCertificatePasswordField])
296317
if err != nil {
297318
return nil, fmt.Errorf("failed to parse client certificates: %w", err)
298319
}
299-
return azidentity.NewClientCertificateCredential(string(tenantID), string(clientID), certs, key, nil)
300-
}
301-
}
302-
if tenant, hasTenant := secret.Data[tenantField]; hasTenant {
303-
if appId, hasAppID := secret.Data[appIDField]; hasAppID {
304-
if password, hasPassword := secret.Data[passwordField]; hasPassword {
305-
return azidentity.NewClientSecretCredential(string(tenant), string(appId), string(password), nil)
320+
opts := &azidentity.ClientCertificateCredentialOptions{}
321+
if authorityHost, hasAuthorityHost := secret.Data[authorityHostField]; hasAuthorityHost {
322+
opts.AuthorityHost = azidentity.AuthorityHost(authorityHost)
306323
}
324+
if v, sendChain := secret.Data[clientCertificateSendChainField]; sendChain {
325+
opts.SendCertificateChain = string(v) == "1" || strings.ToLower(string(v)) == "true"
326+
}
327+
return azidentity.NewClientCertificateCredential(string(tenantID), string(clientID), certs, key, opts)
307328
}
308329
}
309330
if hasClientID {
310331
return azidentity.NewManagedIdentityCredential(&azidentity.ManagedIdentityCredentialOptions{
311332
ID: azidentity.ClientID(clientID),
312333
})
313334
}
314-
if resourceID, hasResourceID := secret.Data[resourceIDField]; hasResourceID {
315-
return azidentity.NewManagedIdentityCredential(&azidentity.ManagedIdentityCredentialOptions{
316-
ID: azidentity.ResourceID(resourceID),
317-
})
318-
}
319335
return nil, nil
320336
}
321337

338+
// sharedCredentialFromSecret attempts to create an azblob.SharedKeyCredential
339+
// based on the data fields of the given Secret. It returns nil if the Secret
340+
// does not contain a valid set of credentials.
322341
func sharedCredentialFromSecret(endpoint string, secret *corev1.Secret) (*azblob.SharedKeyCredential, error) {
323342
if accountKey, hasAccountKey := secret.Data[accountKeyField]; hasAccountKey {
324343
accountName, err := extractAccountNameFromEndpoint(endpoint)
@@ -330,6 +349,37 @@ func sharedCredentialFromSecret(endpoint string, secret *corev1.Secret) (*azblob
330349
return nil, nil
331350
}
332351

352+
// chainCredentialWithSecret tries to create a set of tokens, and returns an
353+
// azidentity.ChainedTokenCredential if at least one of the following tokens was
354+
// successfully created:
355+
// - azidentity.EnvironmentCredential
356+
// - azidentity.ManagedIdentityCredential
357+
// If a Secret with an `authorityHost` is provided, this is set on the
358+
// azidentity.EnvironmentCredentialOptions. It may return nil.
359+
func chainCredentialWithSecret(secret *corev1.Secret) (azcore.TokenCredential, error) {
360+
var creds []azcore.TokenCredential
361+
362+
credOpts := &azidentity.EnvironmentCredentialOptions{}
363+
if secret != nil {
364+
if authorityHost, hasAuthorityHost := secret.Data[authorityHostField]; hasAuthorityHost {
365+
credOpts.AuthorityHost = azidentity.AuthorityHost(authorityHost)
366+
}
367+
}
368+
369+
if token, _ := azidentity.NewEnvironmentCredential(credOpts); token != nil {
370+
creds = append(creds, token)
371+
}
372+
if token, _ := azidentity.NewManagedIdentityCredential(nil); token != nil {
373+
creds = append(creds, token)
374+
}
375+
376+
if len(creds) > 0 {
377+
return azidentity.NewChainedTokenCredential(creds, nil)
378+
}
379+
380+
return nil, nil
381+
}
382+
333383
// extractAccountNameFromEndpoint extracts the Azure account name from the
334384
// provided endpoint URL. It parses the endpoint as a URL, and returns the
335385
// first subdomain as the assumed account name.

‎pkg/azure/blob_test.go

Lines changed: 12 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,6 @@ func TestValidateSecret(t *testing.T) {
4040
secret *corev1.Secret
4141
wantErr bool
4242
}{
43-
{
44-
name: "valid SystemManagedIdentity Secret",
45-
secret: &corev1.Secret{
46-
Data: map[string][]byte{
47-
resourceIDField: []byte("/some/resource/id"),
48-
},
49-
},
50-
},
5143
{
5244
name: "valid UserManagedIdentity Secret",
5345
secret: &corev1.Secret{
@@ -77,20 +69,18 @@ func TestValidateSecret(t *testing.T) {
7769
},
7870
},
7971
{
80-
name: "valid ServicePrincipal Secret",
72+
name: "valid SharedKey Secret",
8173
secret: &corev1.Secret{
8274
Data: map[string][]byte{
83-
tenantField: []byte("some-tenant-id-"),
84-
appIDField: []byte("some-client-id-"),
85-
passwordField: []byte("some-client-secret-"),
75+
accountKeyField: []byte("some-account-key"),
8676
},
8777
},
8878
},
8979
{
90-
name: "valid SharedKey Secret",
80+
name: "valid AuthorityHost Secret",
9181
secret: &corev1.Secret{
9282
Data: map[string][]byte{
93-
accountKeyField: []byte("some-account-key"),
83+
authorityHostField: []byte("some.host.com"),
9484
},
9585
},
9686
},
@@ -200,15 +190,6 @@ func Test_tokenCredentialFromSecret(t *testing.T) {
200190
want azcore.TokenCredential
201191
wantErr bool
202192
}{
203-
{
204-
name: "with ResourceID field",
205-
secret: &corev1.Secret{
206-
Data: map[string][]byte{
207-
resourceIDField: []byte("resource-id"),
208-
},
209-
},
210-
want: &azidentity.ManagedIdentityCredential{},
211-
},
212193
{
213194
name: "with ClientID field",
214195
secret: &corev1.Secret{
@@ -240,17 +221,6 @@ func Test_tokenCredentialFromSecret(t *testing.T) {
240221
},
241222
want: &azidentity.ClientSecretCredential{},
242223
},
243-
{
244-
name: "with Tenant, AppID and Password fields",
245-
secret: &corev1.Secret{
246-
Data: map[string][]byte{
247-
appIDField: []byte("client-id"),
248-
tenantField: []byte("tenant-id"),
249-
passwordField: []byte("client-secret"),
250-
},
251-
},
252-
want: &azidentity.ClientSecretCredential{},
253-
},
254224
{
255225
name: "empty secret",
256226
secret: &corev1.Secret{},
@@ -322,6 +292,14 @@ func Test_sharedCredentialFromSecret(t *testing.T) {
322292
}
323293
}
324294

295+
func Test_chainCredentialWithSecret(t *testing.T) {
296+
g := NewWithT(t)
297+
298+
got, err := chainCredentialWithSecret(nil)
299+
g.Expect(err).ToNot(HaveOccurred())
300+
g.Expect(got).To(BeAssignableToTypeOf(&azidentity.ChainedTokenCredential{}))
301+
}
302+
325303
func Test_extractAccountNameFromEndpoint1(t *testing.T) {
326304
tests := []struct {
327305
name string

0 commit comments

Comments
 (0)