Skip to content

Conversation

@polatengin
Copy link
Contributor

Fixes: #711

With this pull request we address the lack of credential reuse in the GetTokenForScopes method.

The authentication methods in auth.go created a new credential object on every call, bypassing the Azure SDK's built-in token caching and renewal mechanisms. This change implements a credential caching system that creates credentials once and reuses them across subsequent calls.

The GetTokenForScopes method and its associated authentication methods (AuthenticateClientSecret, AuthenticateUsingCli, AuthenticateClientCertificate, etc.) were creating new Azure SDK credential objects on every invocation.

This approach had these issues:

  • Unnecessary overhead from repeatedly initializing credential objects
  • The Azure SDK's internal token caching was not being utilized
  • The Azure SDK's automatic token renewal was not being leveraged
  • Each call incurred the cost of credential construction even when the underlying token was still valid

In this pull request we implement an interface-based credential holder pattern with map-based lookup.

  • A new credentialType enum representing the eight different credential types supported by the provider
  • A new credentialHolder struct that wraps an azcore.TokenCredential with thread-safe lazy initialization using sync.Once
  • A credentials map on the Auth struct that stores cached credential holders
  • A new getOrCreateCredential helper method that handles the lazy initialization and caching logic
  • Updated authentication methods that use the new helper instead of creating credentials directly

The getOrCreateCredential method uses a double-checked locking pattern with sync.RWMutex for the map access and sync.Once for the credential initialization. This ensures thread safety while minimizing lock contention after the initial credential creation.

We also introduced a set of tests to improve code coverage;

go test -coverprofile=coverage.out -run "TestUnit" ./internal/api/... && go tool cover -func=coverage.out | grep -E "(auth\.go|total)"

Before:

github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:30:         Error                            0.0%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:56:         NewAuthBase                      100.0%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:62:         AuthenticateClientCertificate    0.0%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:90:         AuthenticateUsingCli             0.0%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:107:        AuthenticateUsingAzureDeveloperCli0.0%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:124:        AuthenticateClientSecret         0.0%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:146:        NewOidcCredential                28.6%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:180:        GetToken                         0.0%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:184:        AuthenticateOIDC                 38.5%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:218:        AuthenticateUserManagedIdentity  71.4%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:237:        AuthenticateSystemManagedIdentity71.4%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:255:        AuthenticateAzDOWorkloadIdentityFederation                        46.7%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:294:        createTokenRequestOptions        100.0%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:313:        getAssertion                     0.0%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:376:        GetTokenForScopes                57.1%
total:                                                                                  (statements)                     23.2%

After:

github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:31:         Error                                     100.0%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:79:         NewAuthBase                               100.0%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:86:         getOrCreateCredential                     100.0%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:115:        AuthenticateClientCertificate             63.6%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:146:        AuthenticateUsingCli                      87.5%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:165:        AuthenticateUsingAzureDeveloperCli        87.5%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:184:        AuthenticateClientSecret                  87.5%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:209:        NewOidcCredential                         92.9%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:243:        GetToken                                  0.0%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:247:        AuthenticateOIDC                          91.7%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:279:        AuthenticateUserManagedIdentity           100.0%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:300:        AuthenticateSystemManagedIdentity         100.0%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:320:        AuthenticateAzDOWorkloadIdentityFederation93.8%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:361:        createTokenRequestOptions                 100.0%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:380:        getAssertion                              94.1%
github.com/microsoft/terraform-provider-power-platform/internal/api/auth.go:443:        GetTokenForScopes                         100.0%
total:                                                                                  (statements)                              54.8%

@polatengin polatengin self-assigned this Dec 17, 2025
@polatengin polatengin requested a review from a team as a code owner December 17, 2025 18:58
Copilot AI review requested due to automatic review settings December 17, 2025 18:58
@polatengin polatengin added go Pull requests that update Go code security labels Dec 17, 2025
@polatengin polatengin linked an issue Dec 17, 2025 that may be closed by this pull request
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request implements a credential caching and retrieval mechanism to address the lack of credential reuse in the authentication layer. The changes ensure that Azure SDK credential objects are created once and reused across subsequent calls, enabling proper utilization of the SDK's built-in token caching and automatic renewal capabilities.

Key changes:

  • Introduces a credential holder pattern with thread-safe lazy initialization using sync.Once and sync.RWMutex
  • Refactors all eight authentication methods to use the new getOrCreateCredential helper
  • Adds comprehensive unit tests with concurrency validation, improving code coverage from 23.2% to 54.8%

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
internal/api/auth.go Adds credential caching infrastructure (credentialType enum, credentialHolder struct, credentials map with RWMutex) and refactors all authentication methods to use the new getOrCreateCredential helper for credential reuse
internal/api/auth_test.go Implements comprehensive unit tests covering credential caching behavior, thread safety with concurrent access, error handling, and authentication method functionality using mock credentials

Comment on lines +102 to +112
holder.once.Do(func() {
tflog.Debug(ctx, fmt.Sprintf("Initializing credential for type: %s", credType))
holder.credential, holder.err = factory()
if holder.err != nil {
tflog.Error(ctx, fmt.Sprintf("Failed to create credential for type %s: %s", credType, holder.err.Error()))
} else {
tflog.Debug(ctx, fmt.Sprintf("Successfully created credential for type: %s", credType))
}
})

return holder.credential, holder.err
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The credential caching mechanism caches errors permanently using sync.Once. This means if credential creation fails due to a transient error (e.g., temporary network issue, rate limiting), all subsequent attempts will return the same cached error without retrying credential creation. Consider whether this behavior is acceptable for your use case, or if you need a mechanism to retry credential creation after transient failures. For long-running provider instances, this could cause persistent authentication failures even after the underlying issue is resolved.

Suggested change
holder.once.Do(func() {
tflog.Debug(ctx, fmt.Sprintf("Initializing credential for type: %s", credType))
holder.credential, holder.err = factory()
if holder.err != nil {
tflog.Error(ctx, fmt.Sprintf("Failed to create credential for type %s: %s", credType, holder.err.Error()))
} else {
tflog.Debug(ctx, fmt.Sprintf("Successfully created credential for type: %s", credType))
}
})
return holder.credential, holder.err
tflog.Debug(ctx, fmt.Sprintf("Initializing credential for type: %s", credType))
credential, err := factory()
if err != nil {
holder.err = err
tflog.Error(ctx, fmt.Sprintf("Failed to create credential for type %s: %s", credType, err.Error()))
return nil, err
}
holder.credential = credential
holder.err = nil
tflog.Debug(ctx, fmt.Sprintf("Successfully created credential for type: %s", credType))
return holder.credential, nil

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go Pull requests that update Go code security skip-changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lack of Credential Reuse in GetTokenForScopes

2 participants