Skip to content

Commit d7ce679

Browse files
hiddecohiddeco
committed
Reorder Azure token credential composition
- `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 <hello@hidde.co>
1 parent c5c9160 commit d7ce679

File tree

2 files changed

+125
-92
lines changed

2 files changed

+125
-92
lines changed

‎pkg/azure/blob.go‎

Lines changed: 113 additions & 58 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,53 @@ 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
92+
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+
}
106113
}
107114

108-
// Fallback to Shared Key Credential.
109-
cred, err := sharedCredentialFromSecret(obj.Spec.Endpoint, secret)
115+
// Compose token chain based on environment.
116+
// This functions as a replacement for azidentity.NewDefaultAzureCredential
117+
// to not shell out.
118+
token, err = chainCredentialWithSecret(secret)
110119
if err != nil {
111-
return
120+
err = fmt.Errorf("failed to create environment credential chain: %w", err)
121+
return nil, err
112122
}
113-
if cred != nil {
114-
c.ServiceClient, err = azblob.NewServiceClientWithSharedKey(obj.Spec.Endpoint, cred, &azblob.ClientOptions{})
123+
if token != nil {
124+
c.ServiceClient, err = azblob.NewServiceClient(obj.Spec.Endpoint, token, nil)
115125
return
116126
}
117127

118-
// Secret does not contain a valid set of credentials, fallback to simple client.
128+
// Fallback to simple client.
119129
c.ServiceClient, err = azblob.NewServiceClientWithNoCredential(obj.Spec.Endpoint, nil)
120130
return
121131
}
@@ -138,26 +148,19 @@ func ValidateSecret(secret *corev1.Secret) error {
138148
}
139149
}
140150
}
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-
}
151151
if _, hasClientID := secret.Data[clientIDField]; hasClientID {
152152
valid = true
153153
}
154154
if _, hasAccountKey := secret.Data[accountKeyField]; hasAccountKey {
155155
valid = true
156156
}
157+
if _, hasAuthorityHost := secret.Data[authorityHostField]; hasAuthorityHost {
158+
valid = true
159+
}
157160

158161
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)
162+
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'",
163+
secret.Name, clientIDField, accountKeyField, tenantIDField, clientIDField, clientSecretField, tenantIDField, clientIDField, clientCertificateField)
161164
}
162165
return nil
163166
}
@@ -285,40 +288,61 @@ func (c *BlobClient) ObjectIsNotFound(err error) bool {
285288
return false
286289
}
287290

291+
// tokenCredentialsFromSecret attempts to create an azcore.TokenCredential
292+
// based on the data fields of the given Secret. It returns, in order:
293+
// - azidentity.ClientSecretCredential when `tenantId`, `clientId` and
294+
// `clientSecret` fields are found.
295+
// - azidentity.ClientSecretCredential when `tenant`, `appId` and `password`
296+
// fields are found. To match with the JSON from:
297+
// https://docs.microsoft.com/en-us/azure/aks/kubernetes-service-principal?tabs=azure-cli#manually-create-a-service-principal
298+
// - azidentity.ClientCertificateCredential when `tenantId`,
299+
// `clientCertificate` (and optionally `clientCertificatePassword`) fields
300+
// are found.
301+
// - azidentity.ManagedIdentityCredential for a User ID, when a `clientId`
302+
// field but no `tenantId` is found.
303+
// - azidentity.ManagedIdentityCredential for a Resource ID, when a
304+
// `resourceId` field is found.
305+
// - Nil, if no valid set of credential fields was found.
288306
func tokenCredentialFromSecret(secret *corev1.Secret) (azcore.TokenCredential, error) {
307+
if secret == nil {
308+
return nil, nil
309+
}
310+
289311
clientID, hasClientID := secret.Data[clientIDField]
290312
if tenantID, hasTenantID := secret.Data[tenantIDField]; hasTenantID && hasClientID {
291313
if clientSecret, hasClientSecret := secret.Data[clientSecretField]; hasClientSecret && len(clientSecret) > 0 {
292-
return azidentity.NewClientSecretCredential(string(tenantID), string(clientID), string(clientSecret), nil)
314+
opts := &azidentity.ClientSecretCredentialOptions{}
315+
if authorityHost, hasAuthorityHost := secret.Data[authorityHostField]; hasAuthorityHost {
316+
opts.AuthorityHost = azidentity.AuthorityHost(authorityHost)
317+
}
318+
return azidentity.NewClientSecretCredential(string(tenantID), string(clientID), string(clientSecret), opts)
293319
}
294320
if clientCertificate, hasClientCertificate := secret.Data[clientCertificateField]; hasClientCertificate && len(clientCertificate) > 0 {
295321
certs, key, err := azidentity.ParseCertificates(clientCertificate, secret.Data[clientCertificatePasswordField])
296322
if err != nil {
297323
return nil, fmt.Errorf("failed to parse client certificates: %w", err)
298324
}
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)
325+
opts := &azidentity.ClientCertificateCredentialOptions{}
326+
if authorityHost, hasAuthorityHost := secret.Data[authorityHostField]; hasAuthorityHost {
327+
opts.AuthorityHost = azidentity.AuthorityHost(authorityHost)
328+
}
329+
if v, sendChain := secret.Data[clientCertificateSendChainField]; sendChain {
330+
opts.SendCertificateChain = string(v) == "1" || strings.ToLower(string(v)) == "true"
306331
}
332+
return azidentity.NewClientCertificateCredential(string(tenantID), string(clientID), certs, key, opts)
307333
}
308334
}
309335
if hasClientID {
310336
return azidentity.NewManagedIdentityCredential(&azidentity.ManagedIdentityCredentialOptions{
311337
ID: azidentity.ClientID(clientID),
312338
})
313339
}
314-
if resourceID, hasResourceID := secret.Data[resourceIDField]; hasResourceID {
315-
return azidentity.NewManagedIdentityCredential(&azidentity.ManagedIdentityCredentialOptions{
316-
ID: azidentity.ResourceID(resourceID),
317-
})
318-
}
319340
return nil, nil
320341
}
321342

343+
// sharedCredentialFromSecret attempts to create an azblob.SharedKeyCredential
344+
// based on the data fields of the given Secret. It returns nil if the Secret
345+
// does not contain a valid set of credentials.
322346
func sharedCredentialFromSecret(endpoint string, secret *corev1.Secret) (*azblob.SharedKeyCredential, error) {
323347
if accountKey, hasAccountKey := secret.Data[accountKeyField]; hasAccountKey {
324348
accountName, err := extractAccountNameFromEndpoint(endpoint)
@@ -330,6 +354,37 @@ func sharedCredentialFromSecret(endpoint string, secret *corev1.Secret) (*azblob
330354
return nil, nil
331355
}
332356

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