From 2ba693bdb7692d7c332a8c6826a5c20c38aedd93 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Tue, 26 Sep 2023 16:49:45 +0100 Subject: [PATCH 01/13] auth: ability to manually expire a cached token Whilst the new ExpireToken() method only has any meaning or effect when a token is being cached, in practise this all the existing Authorizers wrap themselves with a CachedAuthorizer. For consistency this is added to the Authorizer interface to make downstream implementation simpler and this should not cause any incompatibility or errors where one of the provided constructors is used. --- sdk/auth/azure_cli_authorizer.go | 5 +++++ sdk/auth/cached_authorizer.go | 7 +++++++ sdk/auth/client_credentials.go | 5 +++++ sdk/auth/client_secret_authorizer.go | 5 +++++ sdk/auth/github_oidc_authorizer.go | 5 +++++ sdk/auth/interface.go | 2 +- sdk/auth/managed_identity_authorizer.go | 5 +++++ sdk/auth/shared_key_authorizer.go | 5 ++++- sdk/auth/token.go | 12 ++++++++++-- 9 files changed, 47 insertions(+), 4 deletions(-) diff --git a/sdk/auth/azure_cli_authorizer.go b/sdk/auth/azure_cli_authorizer.go index 21de3aaa457..ce15e47a890 100644 --- a/sdk/auth/azure_cli_authorizer.go +++ b/sdk/auth/azure_cli_authorizer.go @@ -143,6 +143,11 @@ func (a *AzureCliAuthorizer) AuxiliaryTokens(_ context.Context, _ *http.Request) return tokens, nil } +// ExpireToken has no effect with uncached Authorizers +func (a *AzureCliAuthorizer) ExpireToken() error { + return nil +} + const ( AzureCliMinimumVersion = "2.0.81" AzureCliMsalVersion = "2.30.0" diff --git a/sdk/auth/cached_authorizer.go b/sdk/auth/cached_authorizer.go index 6043dda77ce..e4613b47d08 100644 --- a/sdk/auth/cached_authorizer.go +++ b/sdk/auth/cached_authorizer.go @@ -8,6 +8,7 @@ import ( "fmt" "net/http" "sync" + "time" "golang.org/x/oauth2" ) @@ -67,6 +68,12 @@ func (c *CachedAuthorizer) AuxiliaryTokens(ctx context.Context, req *http.Reques return c.auxTokens, nil } +// ExpireToken expires the currently cached token, forcing a new token to be acquired when Token() is next called +func (c *CachedAuthorizer) ExpireToken() error { + c.token.Expiry = time.Now() + return nil +} + // NewCachedAuthorizer returns an Authorizer that caches an access token for the duration of its validity. // If the cached token expires, a new one is acquired and cached. func NewCachedAuthorizer(src Authorizer) (Authorizer, error) { diff --git a/sdk/auth/client_credentials.go b/sdk/auth/client_credentials.go index 571a12ccd8f..b42459f8275 100644 --- a/sdk/auth/client_credentials.go +++ b/sdk/auth/client_credentials.go @@ -314,6 +314,11 @@ func (a *ClientAssertionAuthorizer) AuxiliaryTokens(ctx context.Context, _ *http return tokens, nil } +// ExpireToken has no effect with uncached Authorizers +func (a *ClientAssertionAuthorizer) ExpireToken() error { + return nil +} + func clientCredentialsToken(ctx context.Context, endpoint string, params *url.Values) (*oauth2.Token, error) { req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewBuffer([]byte(params.Encode()))) if err != nil { diff --git a/sdk/auth/client_secret_authorizer.go b/sdk/auth/client_secret_authorizer.go index dcbd3ffe85a..28dc8f51ef8 100644 --- a/sdk/auth/client_secret_authorizer.go +++ b/sdk/auth/client_secret_authorizer.go @@ -131,3 +131,8 @@ func (a *ClientSecretAuthorizer) AuxiliaryTokens(ctx context.Context, _ *http.Re return tokens, nil } + +// ExpireToken has no effect with uncached Authorizers +func (a *ClientSecretAuthorizer) ExpireToken() error { + return nil +} diff --git a/sdk/auth/github_oidc_authorizer.go b/sdk/auth/github_oidc_authorizer.go index a369ebf9828..70f43de2782 100644 --- a/sdk/auth/github_oidc_authorizer.go +++ b/sdk/auth/github_oidc_authorizer.go @@ -190,6 +190,11 @@ type gitHubOIDCConfig struct { Audience string } +// ExpireToken has no effect with uncached Authorizers +func (a *GitHubOIDCAuthorizer) ExpireToken() error { + return nil +} + func (c *gitHubOIDCConfig) TokenSource(ctx context.Context) (Authorizer, error) { return NewCachedAuthorizer(&GitHubOIDCAuthorizer{ conf: c, diff --git a/sdk/auth/interface.go b/sdk/auth/interface.go index 7620f5d76f9..47e2ab6fac7 100644 --- a/sdk/auth/interface.go +++ b/sdk/auth/interface.go @@ -13,8 +13,8 @@ import ( // Authorizer is anything that can return an access token for authorizing API connections type Authorizer interface { Token(ctx context.Context, request *http.Request) (*oauth2.Token, error) - AuxiliaryTokens(ctx context.Context, request *http.Request) ([]*oauth2.Token, error) + ExpireToken() error } // HTTPClient is an HTTP client used for sending authentication requests and obtaining tokens diff --git a/sdk/auth/managed_identity_authorizer.go b/sdk/auth/managed_identity_authorizer.go index 718e6693d98..c52a1f94ecc 100644 --- a/sdk/auth/managed_identity_authorizer.go +++ b/sdk/auth/managed_identity_authorizer.go @@ -118,6 +118,11 @@ func (a *ManagedIdentityAuthorizer) AuxiliaryTokens(_ context.Context, _ *http.R return []*oauth2.Token{}, nil } +// ExpireToken has no effect with uncached Authorizers +func (a *ManagedIdentityAuthorizer) ExpireToken() error { + return nil +} + // managedIdentityConfig configures an ManagedIdentityAuthorizer. type managedIdentityConfig struct { // ClientID is optionally used to determine which application to assume when a resource has multiple managed identities diff --git a/sdk/auth/shared_key_authorizer.go b/sdk/auth/shared_key_authorizer.go index e817bb438ba..4abc343da0b 100644 --- a/sdk/auth/shared_key_authorizer.go +++ b/sdk/auth/shared_key_authorizer.go @@ -64,7 +64,10 @@ func (s *SharedKeyAuthorizer) AuxiliaryTokens(_ context.Context, _ *http.Request return []*oauth2.Token{}, nil } -// --- +// ExpireToken has no effect with shared keys +func (c *SharedKeyAuthorizer) ExpireToken() error { + return fmt.Errorf("SharedKeyAuthorizer tokens cannot expire") +} const ( storageEmulatorAccountName string = "devstoreaccount1" diff --git a/sdk/auth/token.go b/sdk/auth/token.go index 864b2d2fea3..dab35b5a83c 100644 --- a/sdk/auth/token.go +++ b/sdk/auth/token.go @@ -12,6 +12,10 @@ import ( const tokenExpiryDelta = 20 * time.Minute +func ForceExpiry(token *oauth2.Token) error { + return nil +} + // tokenExpiresSoon returns true if the token expires within 10 minutes, or if more than 50% of its validity period has elapsed (if this can be determined), whichever is later func tokenDueForRenewal(token *oauth2.Token) bool { if token == nil { @@ -26,7 +30,11 @@ func tokenDueForRenewal(token *oauth2.Token) bool { expiry := token.Expiry.Round(0) delta := tokenExpiryDelta now := time.Now() - expiresWithinTenMinutes := expiry.Add(-delta).Before(now) + + // Always return early if the token validity doesn't extend past the expiry delta + if expiry.Add(-delta).Before(now) { + return true + } // Try to parse the token claims to retrieve the issuedAt time if claims, err := claims.ParseClaims(token); err == nil { @@ -43,5 +51,5 @@ func tokenDueForRenewal(token *oauth2.Token) bool { } } - return expiresWithinTenMinutes + return false } From 96793a5475e7d2e55a831d87386f9be427e2e274 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Wed, 27 Sep 2023 01:00:54 +0100 Subject: [PATCH 02/13] auth: extend manual token expiration to include auxiliary tokens --- sdk/auth/azure_cli_authorizer.go | 2 +- sdk/auth/cached_authorizer.go | 7 +++++-- sdk/auth/client_credentials.go | 2 +- sdk/auth/client_secret_authorizer.go | 2 +- sdk/auth/github_oidc_authorizer.go | 2 +- sdk/auth/interface.go | 2 +- sdk/auth/managed_identity_authorizer.go | 2 +- sdk/auth/shared_key_authorizer.go | 2 +- 8 files changed, 12 insertions(+), 9 deletions(-) diff --git a/sdk/auth/azure_cli_authorizer.go b/sdk/auth/azure_cli_authorizer.go index ce15e47a890..48bd6729016 100644 --- a/sdk/auth/azure_cli_authorizer.go +++ b/sdk/auth/azure_cli_authorizer.go @@ -144,7 +144,7 @@ func (a *AzureCliAuthorizer) AuxiliaryTokens(_ context.Context, _ *http.Request) } // ExpireToken has no effect with uncached Authorizers -func (a *AzureCliAuthorizer) ExpireToken() error { +func (a *AzureCliAuthorizer) ExpireTokens() error { return nil } diff --git a/sdk/auth/cached_authorizer.go b/sdk/auth/cached_authorizer.go index e4613b47d08..0ae1d924107 100644 --- a/sdk/auth/cached_authorizer.go +++ b/sdk/auth/cached_authorizer.go @@ -68,9 +68,12 @@ func (c *CachedAuthorizer) AuxiliaryTokens(ctx context.Context, req *http.Reques return c.auxTokens, nil } -// ExpireToken expires the currently cached token, forcing a new token to be acquired when Token() is next called -func (c *CachedAuthorizer) ExpireToken() error { +// ExpireTokens expires the currently cached token and auxTokens, forcing new tokens to be acquired when Token() or AuxiliaryTokens() are next called +func (c *CachedAuthorizer) ExpireTokens() error { c.token.Expiry = time.Now() + for i := range c.auxTokens { + c.auxTokens[i].Expiry = time.Now() + } return nil } diff --git a/sdk/auth/client_credentials.go b/sdk/auth/client_credentials.go index b42459f8275..84116240a88 100644 --- a/sdk/auth/client_credentials.go +++ b/sdk/auth/client_credentials.go @@ -315,7 +315,7 @@ func (a *ClientAssertionAuthorizer) AuxiliaryTokens(ctx context.Context, _ *http } // ExpireToken has no effect with uncached Authorizers -func (a *ClientAssertionAuthorizer) ExpireToken() error { +func (a *ClientAssertionAuthorizer) ExpireTokens() error { return nil } diff --git a/sdk/auth/client_secret_authorizer.go b/sdk/auth/client_secret_authorizer.go index 28dc8f51ef8..91fb5d23568 100644 --- a/sdk/auth/client_secret_authorizer.go +++ b/sdk/auth/client_secret_authorizer.go @@ -133,6 +133,6 @@ func (a *ClientSecretAuthorizer) AuxiliaryTokens(ctx context.Context, _ *http.Re } // ExpireToken has no effect with uncached Authorizers -func (a *ClientSecretAuthorizer) ExpireToken() error { +func (a *ClientSecretAuthorizer) ExpireTokens() error { return nil } diff --git a/sdk/auth/github_oidc_authorizer.go b/sdk/auth/github_oidc_authorizer.go index 70f43de2782..4a6212d5212 100644 --- a/sdk/auth/github_oidc_authorizer.go +++ b/sdk/auth/github_oidc_authorizer.go @@ -191,7 +191,7 @@ type gitHubOIDCConfig struct { } // ExpireToken has no effect with uncached Authorizers -func (a *GitHubOIDCAuthorizer) ExpireToken() error { +func (a *GitHubOIDCAuthorizer) ExpireTokens() error { return nil } diff --git a/sdk/auth/interface.go b/sdk/auth/interface.go index 47e2ab6fac7..fe5256e5e9b 100644 --- a/sdk/auth/interface.go +++ b/sdk/auth/interface.go @@ -14,7 +14,7 @@ import ( type Authorizer interface { Token(ctx context.Context, request *http.Request) (*oauth2.Token, error) AuxiliaryTokens(ctx context.Context, request *http.Request) ([]*oauth2.Token, error) - ExpireToken() error + ExpireTokens() error } // HTTPClient is an HTTP client used for sending authentication requests and obtaining tokens diff --git a/sdk/auth/managed_identity_authorizer.go b/sdk/auth/managed_identity_authorizer.go index c52a1f94ecc..3a16e553f8c 100644 --- a/sdk/auth/managed_identity_authorizer.go +++ b/sdk/auth/managed_identity_authorizer.go @@ -119,7 +119,7 @@ func (a *ManagedIdentityAuthorizer) AuxiliaryTokens(_ context.Context, _ *http.R } // ExpireToken has no effect with uncached Authorizers -func (a *ManagedIdentityAuthorizer) ExpireToken() error { +func (a *ManagedIdentityAuthorizer) ExpireTokens() error { return nil } diff --git a/sdk/auth/shared_key_authorizer.go b/sdk/auth/shared_key_authorizer.go index 4abc343da0b..4e41c87cedc 100644 --- a/sdk/auth/shared_key_authorizer.go +++ b/sdk/auth/shared_key_authorizer.go @@ -65,7 +65,7 @@ func (s *SharedKeyAuthorizer) AuxiliaryTokens(_ context.Context, _ *http.Request } // ExpireToken has no effect with shared keys -func (c *SharedKeyAuthorizer) ExpireToken() error { +func (c *SharedKeyAuthorizer) ExpireTokens() error { return fmt.Errorf("SharedKeyAuthorizer tokens cannot expire") } From ca32de0022223c79625f8fcaa91116ad7fba3778 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Wed, 27 Sep 2023 01:01:40 +0100 Subject: [PATCH 03/13] auth: support the `exp` claim --- sdk/claims/claims.go | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/claims/claims.go b/sdk/claims/claims.go index 97683b0c65f..15444cdf36d 100644 --- a/sdk/claims/claims.go +++ b/sdk/claims/claims.go @@ -15,6 +15,7 @@ import ( // Claims is used to unmarshall the claims from a JWT issued by the Microsoft Identity Platform. type Claims struct { Audience string `json:"aud"` + Expires int64 `json:"exp"` IssuedAt int64 `json:"iat"` Issuer string `json:"iss"` IdentityProvider string `json:"idp"` From 5165d82f9246ef3cb19df82a4270ac646b769c16 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Wed, 27 Sep 2023 01:02:39 +0100 Subject: [PATCH 04/13] auth: add a `SetAuthHeaders()` function for decorating the Request with both standard and auxiliary access tokens --- sdk/auth/token.go | 37 +++++++++++++++ sdk/auth/token_test.go | 105 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+) create mode 100644 sdk/auth/token_test.go diff --git a/sdk/auth/token.go b/sdk/auth/token.go index dab35b5a83c..837dd27ee9d 100644 --- a/sdk/auth/token.go +++ b/sdk/auth/token.go @@ -4,12 +4,49 @@ package auth import ( + "context" + "fmt" "golang.org/x/oauth2" + "net/http" + "strings" "time" "github.com/hashicorp/go-azure-sdk/sdk/claims" ) +func SetAuthHeaders(ctx context.Context, req *http.Request, authorizer Authorizer) error { + if req == nil { + return fmt.Errorf("request was nil") + } + if authorizer == nil { + return fmt.Errorf("authorizer was nil") + } + + token, err := authorizer.Token(ctx, req) + if err != nil { + return err + } + + if req.Header == nil { + req.Header = make(http.Header) + } + + req.Header.Set("Authorization", fmt.Sprintf("%s %s", token.Type(), token.AccessToken)) + + auxTokens, err := authorizer.AuxiliaryTokens(ctx, req) + if err != nil { + return err + } + + auxTokenValues := make([]string, 0) + for _, auxToken := range auxTokens { + auxTokenValues = append(auxTokenValues, fmt.Sprintf("%s %s", auxToken.Type(), auxToken.AccessToken)) + } + req.Header.Set("X-Ms-Authorization-Auxiliary", strings.Join(auxTokenValues, ", ")) + + return nil +} + const tokenExpiryDelta = 20 * time.Minute func ForceExpiry(token *oauth2.Token) error { diff --git a/sdk/auth/token_test.go b/sdk/auth/token_test.go new file mode 100644 index 00000000000..e5327d4e86c --- /dev/null +++ b/sdk/auth/token_test.go @@ -0,0 +1,105 @@ +package auth_test + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "fmt" + "net/http" + "regexp" + "strings" + "testing" + "time" + + "github.com/golang-jwt/jwt/v4" + "github.com/hashicorp/go-azure-sdk/sdk/auth" + "golang.org/x/oauth2" +) + +var ( + testTokenIssued = time.Now() + testTokenExpiry = time.Now().Add(3599 * time.Second) +) + +var _ auth.Authorizer = &testAuthorizer{} + +type testAuthorizer struct{} + +func (*testAuthorizer) Token(_ context.Context, _ *http.Request) (*oauth2.Token, error) { + return &oauth2.Token{ + AccessToken: testAccessToken(), + TokenType: "Bearer", + Expiry: testTokenExpiry, + }, nil +} + +func (*testAuthorizer) AuxiliaryTokens(_ context.Context, _ *http.Request) ([]*oauth2.Token, error) { + return []*oauth2.Token{ + { + AccessToken: testAccessToken(), + TokenType: "Bearer", + Expiry: testTokenExpiry, + }, + { + AccessToken: testAccessToken(), + TokenType: "Bearer", + Expiry: testTokenExpiry, + }, + { + AccessToken: testAccessToken(), + TokenType: "Bearer", + Expiry: testTokenExpiry, + }, + }, nil +} + +func (*testAuthorizer) ExpireTokens() error { + return nil +} + +func testAccessToken() string { + token := jwt.NewWithClaims(jwt.SigningMethodES256, jwt.MapClaims{ + "appid": "10000000-2000-3000-4000-500000000000", + "aud": "https://not.azure.net", + "exp": testTokenExpiry.Unix(), + "iat": testTokenIssued.Unix(), + "iss": "Not Azure", + "nbf": testTokenIssued.Unix(), + "oid": "aaaaaa48-bb50-cc40-dd17-beeeeeeeeeef", + "sub": "12345678-1234-5678-90ab-1234567890ab", + "tid": "99999999-8888-7777-6666-555555543210", + }) + + key, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + ret, err := token.SignedString(key) + if err != nil { + panic(fmt.Sprintf("failed to sign test token: %+v", err)) + } + + return ret +} + +func TestSetAuthHeaders(t *testing.T) { + req := &http.Request{} + authorizer := &testAuthorizer{} + + err := auth.SetAuthHeaders(context.Background(), req, authorizer) + if err != nil { + t.Fatalf("received error: %+v", err) + } + + expected := regexp.MustCompile("^Bearer [a-zA-Z0-9-]+[.][a-zA-Z0-9-]+[.][a-zA-Z0-9-]+") + if val := req.Header.Get("Authorization"); !expected.MatchString(val) { + t.Fatalf("Authorization header mismatch, received: %q", val) + } + auxVals := strings.Split(req.Header.Get("X-Ms-Authorization-Auxiliary"), ",") + if len(auxVals) != 3 { + t.Fatalf("X-Ms-Authorization-Auxiliary mismatch, should contain 3 tokens, received: %q", strings.Join(auxVals, ",")) + } + for i, auxVal := range auxVals { + if !expected.MatchString(strings.TrimSpace(auxVal)) { + t.Fatalf("Authorization header mismatch for token %d, received: %q", i, auxVal) + } + } +} From 107241873de45c6fe57d9c0a2e9011eb1a8c8d91 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Wed, 27 Sep 2023 01:03:30 +0100 Subject: [PATCH 05/13] auth: proper unit testing for the CachedAuthorizer --- sdk/auth/cached_authorizer_test.go | 160 +++++++++++++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 sdk/auth/cached_authorizer_test.go diff --git a/sdk/auth/cached_authorizer_test.go b/sdk/auth/cached_authorizer_test.go new file mode 100644 index 00000000000..28942a57e73 --- /dev/null +++ b/sdk/auth/cached_authorizer_test.go @@ -0,0 +1,160 @@ +package auth_test + +import ( + "context" + "net/http" + "regexp" + "testing" + "time" + + "github.com/hashicorp/go-azure-sdk/sdk/auth" + "github.com/hashicorp/go-azure-sdk/sdk/claims" +) + +func TestCachedAuthorizer(t *testing.T) { + tokenPattern := regexp.MustCompile("^[a-zA-Z0-9-]+[.][a-zA-Z0-9-]+[.][a-zA-Z0-9-]+") + req := &http.Request{} + + authorizer, err := auth.NewCachedAuthorizer(&testAuthorizer{}) + if err != nil { + t.Fatalf("received error for NewCachedAuthorizer(): %+v", err) + } + + // Retrieve the first access tokens + token, err := authorizer.Token(context.Background(), req) + if err != nil { + t.Fatalf("received error for CachedAuthorizer.Token(): %+v", err) + } + if !tokenPattern.MatchString(token.AccessToken) { + t.Fatalf("unexpected access token received: %q", token.AccessToken) + } + auxTokens, err := authorizer.AuxiliaryTokens(context.Background(), req) + if err != nil { + t.Fatalf("received error for CachedAuthorizer.AuxiliaryTokens(): %+v", err) + } + for i, auxToken := range auxTokens { + if !tokenPattern.MatchString(auxToken.AccessToken) { + t.Fatalf("unexpected auxiliary access token received at %d: %q", i, token.AccessToken) + } + } + + // Parse the claims and compare the IssuedAt and Expiry times + tokenClaims, err := claims.ParseClaims(token) + if err != nil { + t.Fatalf("received error for claims.ParseClaims(): %+v", err) + } + if tokenClaims.IssuedAt != testTokenIssued.Unix() { + t.Fatalf("unexpected `iat` claim for access token, expected: %d, received: %d", testTokenIssued.Unix(), tokenClaims.IssuedAt) + } + if tokenClaims.Expires != testTokenExpiry.Unix() { + t.Fatalf("unexpected `exp` claim for access token, expected: %d, received: %d", testTokenExpiry.Unix(), tokenClaims.Expires) + } + for i, auxToken := range auxTokens { + auxTokenClaims, err := claims.ParseClaims(auxToken) + if err != nil { + t.Fatalf("received error for claims.ParseClaims(): %+v", err) + } + if auxTokenClaims.IssuedAt != testTokenIssued.Unix() { + t.Fatalf("unexpected `iat` claim for auxiliary access token at %d, expected: %d, received: %d", i, testTokenIssued.Unix(), auxTokenClaims.IssuedAt) + } + if auxTokenClaims.Expires != testTokenExpiry.Unix() { + t.Fatalf("unexpected `exp` claim for auxiliary access token at %d, expected: %d, received: %d", i, testTokenExpiry.Unix(), auxTokenClaims.Expires) + } + } + + // Wait for 5 seconds and advance the issued/expiry times for the testAuthorizer + time.Sleep(5 * time.Second) + earlierTestTokenIssued := testTokenIssued + earlierTestTokenExpiry := testTokenExpiry + testTokenIssued = time.Now() + testTokenExpiry = time.Now().Add(3599 * time.Second) + + // Retrieve a second token, this should be retrieved from the cache + token, err = authorizer.Token(context.Background(), req) + if err != nil { + t.Fatalf("received error for CachedAuthorizer.Token(): %+v", err) + } + if !tokenPattern.MatchString(token.AccessToken) { + t.Fatalf("unexpected access token received: %q", token.AccessToken) + } + auxTokens, err = authorizer.AuxiliaryTokens(context.Background(), req) + if err != nil { + t.Fatalf("received error for CachedAuthorizer.AuxiliaryTokens(): %+v", err) + } + for i, auxToken := range auxTokens { + if !tokenPattern.MatchString(auxToken.AccessToken) { + t.Fatalf("unexpected auxiliary access token received at %d: %q", i, token.AccessToken) + } + } + + // Parse the claims for the second token, ensure the IssuedAt and Expiry times _have not_ changed + tokenClaims, err = claims.ParseClaims(token) + if err != nil { + t.Fatalf("received error for claims.ParseClaims(): %+v", err) + } + if tokenClaims.IssuedAt != earlierTestTokenIssued.Unix() { + t.Fatalf("unexpected `iat` claim for access token, expected: %d, received: %d", earlierTestTokenIssued.Unix(), tokenClaims.IssuedAt) + } + if tokenClaims.Expires != earlierTestTokenExpiry.Unix() { + t.Fatalf("unexpected `exp` claim for access token, expected: %d, received: %d", earlierTestTokenExpiry.Unix(), tokenClaims.Expires) + } + for i, auxToken := range auxTokens { + auxTokenClaims, err := claims.ParseClaims(auxToken) + if err != nil { + t.Fatalf("received error for claims.ParseClaims(): %+v", err) + } + if auxTokenClaims.IssuedAt != earlierTestTokenIssued.Unix() { + t.Fatalf("unexpected `iat` claim for auxiliary access token at %d, expected: %d, received: %d", i, earlierTestTokenIssued.Unix(), auxTokenClaims.IssuedAt) + } + if auxTokenClaims.Expires != earlierTestTokenExpiry.Unix() { + t.Fatalf("unexpected `exp` claim for auxiliary access token at %d, expected: %d, received: %d", i, earlierTestTokenExpiry.Unix(), auxTokenClaims.Expires) + } + } + + // Invalidate the access tokens + if err = authorizer.ExpireTokens(); err != nil { + t.Fatalf("received error for CachedAuthorizer.ExpireTokens(): %+v", err) + } + + // Retrieve a third token, which should be re-acquired from the testAuthorizer + token, err = authorizer.Token(context.Background(), req) + if err != nil { + t.Fatalf("received error for CachedAuthorizer.Token(): %+v", err) + } + if !tokenPattern.MatchString(token.AccessToken) { + t.Fatalf("unexpected access token received: %q", token.AccessToken) + } + auxTokens, err = authorizer.AuxiliaryTokens(context.Background(), req) + if err != nil { + t.Fatalf("received error for CachedAuthorizer.AuxiliaryTokens(): %+v", err) + } + for i, auxToken := range auxTokens { + if !tokenPattern.MatchString(auxToken.AccessToken) { + t.Fatalf("unexpected auxiliary access token received at %d: %q", i, token.AccessToken) + } + } + + // Parse the claims for the third token, ensure the IssuedAt and Expiry times _have_ changed + tokenClaims, err = claims.ParseClaims(token) + if err != nil { + t.Fatalf("received error for claims.ParseClaims(): %+v", err) + } + if tokenClaims.IssuedAt != testTokenIssued.Unix() { + t.Fatalf("unexpected `iat` claim for access token, expected: %d, received: %d", testTokenIssued.Unix(), tokenClaims.IssuedAt) + } + if tokenClaims.Expires != testTokenExpiry.Unix() { + t.Fatalf("unexpected `exp` claim for access token, expected: %d, received: %d", testTokenExpiry.Unix(), tokenClaims.Expires) + } + for i, auxToken := range auxTokens { + auxTokenClaims, err := claims.ParseClaims(auxToken) + if err != nil { + t.Fatalf("received error for claims.ParseClaims(): %+v", err) + } + if auxTokenClaims.IssuedAt != testTokenIssued.Unix() { + t.Fatalf("unexpected `iat` claim for auxiliary access token at %d, expected: %d, received: %d", i, testTokenIssued.Unix(), auxTokenClaims.IssuedAt) + } + if auxTokenClaims.Expires != testTokenExpiry.Unix() { + t.Fatalf("unexpected `exp` claim for auxiliary access token at %d, expected: %d, received: %d", i, testTokenExpiry.Unix(), auxTokenClaims.Expires) + } + } +} From 60af3f3e689c10917bccc4460267e8d7743a9879 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Wed, 27 Sep 2023 01:03:49 +0100 Subject: [PATCH 06/13] auth: fix a bug with auxiliary token caching --- sdk/auth/cached_authorizer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/auth/cached_authorizer.go b/sdk/auth/cached_authorizer.go index 0ae1d924107..bc6ff15dbf7 100644 --- a/sdk/auth/cached_authorizer.go +++ b/sdk/auth/cached_authorizer.go @@ -55,7 +55,7 @@ func (c *CachedAuthorizer) AuxiliaryTokens(ctx context.Context, req *http.Reques } c.mutex.RUnlock() - if !dueForRenewal { + if dueForRenewal { c.mutex.Lock() defer c.mutex.Unlock() var err error From 82cc4489f84b8d67eb78b076b770d1c034a2dfd3 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Wed, 27 Sep 2023 01:10:26 +0100 Subject: [PATCH 07/13] auth: documentation and cleanup --- sdk/auth/token.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/sdk/auth/token.go b/sdk/auth/token.go index 837dd27ee9d..853af8939a6 100644 --- a/sdk/auth/token.go +++ b/sdk/auth/token.go @@ -6,14 +6,16 @@ package auth import ( "context" "fmt" - "golang.org/x/oauth2" "net/http" "strings" "time" "github.com/hashicorp/go-azure-sdk/sdk/claims" + "golang.org/x/oauth2" ) +// SetAuthHeaders decorates a *http.Request with necessary authorization headers for Azure APIs. For more information about the vendor-specific +// `x-ms-authorization-auxiliary` header, see https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/authenticate-multi-tenant func SetAuthHeaders(ctx context.Context, req *http.Request, authorizer Authorizer) error { if req == nil { return fmt.Errorf("request was nil") @@ -49,10 +51,6 @@ func SetAuthHeaders(ctx context.Context, req *http.Request, authorizer Authorize const tokenExpiryDelta = 20 * time.Minute -func ForceExpiry(token *oauth2.Token) error { - return nil -} - // tokenExpiresSoon returns true if the token expires within 10 minutes, or if more than 50% of its validity period has elapsed (if this can be determined), whichever is later func tokenDueForRenewal(token *oauth2.Token) bool { if token == nil { From 4e0f2710d0e5b5292126bd62f9f0c37b2e90b74e Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Wed, 27 Sep 2023 13:03:58 +0100 Subject: [PATCH 08/13] auth: fix up test token pattern --- sdk/auth/cached_authorizer_test.go | 2 +- sdk/auth/token.go | 2 +- sdk/auth/token_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/auth/cached_authorizer_test.go b/sdk/auth/cached_authorizer_test.go index 28942a57e73..b5662e898d9 100644 --- a/sdk/auth/cached_authorizer_test.go +++ b/sdk/auth/cached_authorizer_test.go @@ -12,7 +12,7 @@ import ( ) func TestCachedAuthorizer(t *testing.T) { - tokenPattern := regexp.MustCompile("^[a-zA-Z0-9-]+[.][a-zA-Z0-9-]+[.][a-zA-Z0-9-]+") + tokenPattern := regexp.MustCompile("^[a-zA-Z0-9_-]+[.][a-zA-Z0-9_-]+[.][a-zA-Z0-9_-]+") req := &http.Request{} authorizer, err := auth.NewCachedAuthorizer(&testAuthorizer{}) diff --git a/sdk/auth/token.go b/sdk/auth/token.go index 853af8939a6..ea06e560c97 100644 --- a/sdk/auth/token.go +++ b/sdk/auth/token.go @@ -51,7 +51,7 @@ func SetAuthHeaders(ctx context.Context, req *http.Request, authorizer Authorize const tokenExpiryDelta = 20 * time.Minute -// tokenExpiresSoon returns true if the token expires within 10 minutes, or if more than 50% of its validity period has elapsed (if this can be determined), whichever is later +// tokenDueForRenewal returns true if the token expires within 10 minutes, or if more than 50% of its validity period has elapsed (if this can be determined), whichever is later func tokenDueForRenewal(token *oauth2.Token) bool { if token == nil { return true diff --git a/sdk/auth/token_test.go b/sdk/auth/token_test.go index e5327d4e86c..7eaa15365d0 100644 --- a/sdk/auth/token_test.go +++ b/sdk/auth/token_test.go @@ -89,7 +89,7 @@ func TestSetAuthHeaders(t *testing.T) { t.Fatalf("received error: %+v", err) } - expected := regexp.MustCompile("^Bearer [a-zA-Z0-9-]+[.][a-zA-Z0-9-]+[.][a-zA-Z0-9-]+") + expected := regexp.MustCompile("^Bearer [a-zA-Z0-9_-]+[.][a-zA-Z0-9_-]+[.][a-zA-Z0-9_-]+") if val := req.Header.Get("Authorization"); !expected.MatchString(val) { t.Fatalf("Authorization header mismatch, received: %q", val) } From 15a78fe615fc30fd9848b1b6ce67454ecf2f10ac Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Wed, 27 Sep 2023 17:22:20 +0100 Subject: [PATCH 09/13] client: use auth package helper to set authorization headers --- sdk/client/client.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/sdk/client/client.go b/sdk/client/client.go index 05682f8bcaa..f38b6e497e9 100644 --- a/sdk/client/client.go +++ b/sdk/client/client.go @@ -327,14 +327,11 @@ func (c *Client) Execute(ctx context.Context, req *Request) (*Response, error) { return nil, fmt.Errorf("req.Request was nil") } - // at this point we're ready to send the HTTP Request, as such let's get the Authorization token - // and add that to the request + // Set Authorization and X-Ms-Authorization-Auxiliary headers if c.Authorizer != nil { - token, err := c.Authorizer.Token(ctx, req.Request) - if err != nil { + if err := auth.SetAuthHeaders(ctx, req.Request, c.Authorizer); err != nil { return nil, err } - token.SetAuthHeader(req.Request) } var err error From e2ff919cff2f6240cf0ed909cdeb41e7f8611b12 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Thu, 28 Sep 2023 22:30:51 +0100 Subject: [PATCH 10/13] auth: fix bug where empty slice of auxTokens is skipped --- sdk/auth/cached_authorizer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/auth/cached_authorizer.go b/sdk/auth/cached_authorizer.go index bc6ff15dbf7..2bc2e210c01 100644 --- a/sdk/auth/cached_authorizer.go +++ b/sdk/auth/cached_authorizer.go @@ -55,7 +55,7 @@ func (c *CachedAuthorizer) AuxiliaryTokens(ctx context.Context, req *http.Reques } c.mutex.RUnlock() - if dueForRenewal { + if dueForRenewal || len(c.auxTokens) == 0 { c.mutex.Lock() defer c.mutex.Unlock() var err error From 3dd1893d81ff8392fe7477f6c9a5d4fc330607ca Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Tue, 24 Oct 2023 01:43:29 +0100 Subject: [PATCH 11/13] Implement optional custom authorization function for API clients * Add a custom authorization function for ResourceManagerClient, which sets the `Authorization` and `x-ms-authorization-auxiliary` headers. * When custom authorization function is not defined, use the auth.SetAuthHeader() function to decorate requests with a standard `Authorization` header having a bearer token. --- sdk/auth/azure_cli_authorizer.go | 5 -- sdk/auth/cached_authorizer.go | 12 ++- sdk/auth/cached_authorizer_test.go | 45 +++++----- sdk/auth/client_credentials.go | 5 -- sdk/auth/client_secret_authorizer.go | 5 -- sdk/auth/github_oidc_authorizer.go | 5 -- sdk/auth/interface.go | 13 ++- sdk/auth/managed_identity_authorizer.go | 5 -- sdk/auth/shared_key_authorizer.go | 5 -- sdk/auth/token.go | 18 +--- sdk/auth/token_test.go | 87 +------------------ sdk/client/client.go | 20 +++-- sdk/client/dataplane/storage/client.go | 1 + sdk/client/msgraph/client.go | 1 + sdk/client/resourcemanager/authorization.go | 43 +++++++++ .../resourcemanager/authorization_test.go | 36 ++++++++ sdk/client/resourcemanager/client.go | 4 + sdk/internal/test/auth.go | 78 +++++++++++++++++ 18 files changed, 228 insertions(+), 160 deletions(-) create mode 100644 sdk/client/resourcemanager/authorization.go create mode 100644 sdk/client/resourcemanager/authorization_test.go create mode 100644 sdk/internal/test/auth.go diff --git a/sdk/auth/azure_cli_authorizer.go b/sdk/auth/azure_cli_authorizer.go index 48bd6729016..21de3aaa457 100644 --- a/sdk/auth/azure_cli_authorizer.go +++ b/sdk/auth/azure_cli_authorizer.go @@ -143,11 +143,6 @@ func (a *AzureCliAuthorizer) AuxiliaryTokens(_ context.Context, _ *http.Request) return tokens, nil } -// ExpireToken has no effect with uncached Authorizers -func (a *AzureCliAuthorizer) ExpireTokens() error { - return nil -} - const ( AzureCliMinimumVersion = "2.0.81" AzureCliMsalVersion = "2.30.0" diff --git a/sdk/auth/cached_authorizer.go b/sdk/auth/cached_authorizer.go index 2bc2e210c01..6e89884857d 100644 --- a/sdk/auth/cached_authorizer.go +++ b/sdk/auth/cached_authorizer.go @@ -13,7 +13,7 @@ import ( "golang.org/x/oauth2" ) -var _ Authorizer = &CachedAuthorizer{} +var _ CachingAuthorizer = &CachedAuthorizer{} // CachedAuthorizer caches a token until it expires, then acquires a new token from Source type CachedAuthorizer struct { @@ -68,8 +68,12 @@ func (c *CachedAuthorizer) AuxiliaryTokens(ctx context.Context, req *http.Reques return c.auxTokens, nil } -// ExpireTokens expires the currently cached token and auxTokens, forcing new tokens to be acquired when Token() or AuxiliaryTokens() are next called -func (c *CachedAuthorizer) ExpireTokens() error { +// InvalidateCachedTokens expires the currently cached token and auxTokens, forcing new +// tokens to be acquired when Token() or AuxiliaryTokens() are next called +func (c *CachedAuthorizer) InvalidateCachedTokens() error { + if c.token == nil { + return fmt.Errorf("internal-error: c.token was nil") + } c.token.Expiry = time.Now() for i := range c.auxTokens { c.auxTokens[i].Expiry = time.Now() @@ -79,7 +83,7 @@ func (c *CachedAuthorizer) ExpireTokens() error { // NewCachedAuthorizer returns an Authorizer that caches an access token for the duration of its validity. // If the cached token expires, a new one is acquired and cached. -func NewCachedAuthorizer(src Authorizer) (Authorizer, error) { +func NewCachedAuthorizer(src Authorizer) (CachingAuthorizer, error) { if _, ok := src.(*SharedKeyAuthorizer); ok { return nil, fmt.Errorf("internal-error: SharedKeyAuthorizer cannot be cached") } diff --git a/sdk/auth/cached_authorizer_test.go b/sdk/auth/cached_authorizer_test.go index b5662e898d9..1ae2f50f789 100644 --- a/sdk/auth/cached_authorizer_test.go +++ b/sdk/auth/cached_authorizer_test.go @@ -9,13 +9,14 @@ import ( "github.com/hashicorp/go-azure-sdk/sdk/auth" "github.com/hashicorp/go-azure-sdk/sdk/claims" + "github.com/hashicorp/go-azure-sdk/sdk/internal/test" ) func TestCachedAuthorizer(t *testing.T) { tokenPattern := regexp.MustCompile("^[a-zA-Z0-9_-]+[.][a-zA-Z0-9_-]+[.][a-zA-Z0-9_-]+") req := &http.Request{} - authorizer, err := auth.NewCachedAuthorizer(&testAuthorizer{}) + authorizer, err := auth.NewCachedAuthorizer(&test.TestAuthorizer{}) if err != nil { t.Fatalf("received error for NewCachedAuthorizer(): %+v", err) } @@ -43,31 +44,31 @@ func TestCachedAuthorizer(t *testing.T) { if err != nil { t.Fatalf("received error for claims.ParseClaims(): %+v", err) } - if tokenClaims.IssuedAt != testTokenIssued.Unix() { - t.Fatalf("unexpected `iat` claim for access token, expected: %d, received: %d", testTokenIssued.Unix(), tokenClaims.IssuedAt) + if tokenClaims.IssuedAt != test.TestTokenIssued.Unix() { + t.Fatalf("unexpected `iat` claim for access token, expected: %d, received: %d", test.TestTokenIssued.Unix(), tokenClaims.IssuedAt) } - if tokenClaims.Expires != testTokenExpiry.Unix() { - t.Fatalf("unexpected `exp` claim for access token, expected: %d, received: %d", testTokenExpiry.Unix(), tokenClaims.Expires) + if tokenClaims.Expires != test.TestTokenExpiry.Unix() { + t.Fatalf("unexpected `exp` claim for access token, expected: %d, received: %d", test.TestTokenExpiry.Unix(), tokenClaims.Expires) } for i, auxToken := range auxTokens { auxTokenClaims, err := claims.ParseClaims(auxToken) if err != nil { t.Fatalf("received error for claims.ParseClaims(): %+v", err) } - if auxTokenClaims.IssuedAt != testTokenIssued.Unix() { - t.Fatalf("unexpected `iat` claim for auxiliary access token at %d, expected: %d, received: %d", i, testTokenIssued.Unix(), auxTokenClaims.IssuedAt) + if auxTokenClaims.IssuedAt != test.TestTokenIssued.Unix() { + t.Fatalf("unexpected `iat` claim for auxiliary access token at %d, expected: %d, received: %d", i, test.TestTokenIssued.Unix(), auxTokenClaims.IssuedAt) } - if auxTokenClaims.Expires != testTokenExpiry.Unix() { - t.Fatalf("unexpected `exp` claim for auxiliary access token at %d, expected: %d, received: %d", i, testTokenExpiry.Unix(), auxTokenClaims.Expires) + if auxTokenClaims.Expires != test.TestTokenExpiry.Unix() { + t.Fatalf("unexpected `exp` claim for auxiliary access token at %d, expected: %d, received: %d", i, test.TestTokenExpiry.Unix(), auxTokenClaims.Expires) } } // Wait for 5 seconds and advance the issued/expiry times for the testAuthorizer time.Sleep(5 * time.Second) - earlierTestTokenIssued := testTokenIssued - earlierTestTokenExpiry := testTokenExpiry - testTokenIssued = time.Now() - testTokenExpiry = time.Now().Add(3599 * time.Second) + earlierTestTokenIssued := test.TestTokenIssued + earlierTestTokenExpiry := test.TestTokenExpiry + test.TestTokenIssued = time.Now() + test.TestTokenExpiry = time.Now().Add(3599 * time.Second) // Retrieve a second token, this should be retrieved from the cache token, err = authorizer.Token(context.Background(), req) @@ -112,7 +113,7 @@ func TestCachedAuthorizer(t *testing.T) { } // Invalidate the access tokens - if err = authorizer.ExpireTokens(); err != nil { + if err = authorizer.InvalidateCachedTokens(); err != nil { t.Fatalf("received error for CachedAuthorizer.ExpireTokens(): %+v", err) } @@ -139,22 +140,22 @@ func TestCachedAuthorizer(t *testing.T) { if err != nil { t.Fatalf("received error for claims.ParseClaims(): %+v", err) } - if tokenClaims.IssuedAt != testTokenIssued.Unix() { - t.Fatalf("unexpected `iat` claim for access token, expected: %d, received: %d", testTokenIssued.Unix(), tokenClaims.IssuedAt) + if tokenClaims.IssuedAt != test.TestTokenIssued.Unix() { + t.Fatalf("unexpected `iat` claim for access token, expected: %d, received: %d", test.TestTokenIssued.Unix(), tokenClaims.IssuedAt) } - if tokenClaims.Expires != testTokenExpiry.Unix() { - t.Fatalf("unexpected `exp` claim for access token, expected: %d, received: %d", testTokenExpiry.Unix(), tokenClaims.Expires) + if tokenClaims.Expires != test.TestTokenExpiry.Unix() { + t.Fatalf("unexpected `exp` claim for access token, expected: %d, received: %d", test.TestTokenExpiry.Unix(), tokenClaims.Expires) } for i, auxToken := range auxTokens { auxTokenClaims, err := claims.ParseClaims(auxToken) if err != nil { t.Fatalf("received error for claims.ParseClaims(): %+v", err) } - if auxTokenClaims.IssuedAt != testTokenIssued.Unix() { - t.Fatalf("unexpected `iat` claim for auxiliary access token at %d, expected: %d, received: %d", i, testTokenIssued.Unix(), auxTokenClaims.IssuedAt) + if auxTokenClaims.IssuedAt != test.TestTokenIssued.Unix() { + t.Fatalf("unexpected `iat` claim for auxiliary access token at %d, expected: %d, received: %d", i, test.TestTokenIssued.Unix(), auxTokenClaims.IssuedAt) } - if auxTokenClaims.Expires != testTokenExpiry.Unix() { - t.Fatalf("unexpected `exp` claim for auxiliary access token at %d, expected: %d, received: %d", i, testTokenExpiry.Unix(), auxTokenClaims.Expires) + if auxTokenClaims.Expires != test.TestTokenExpiry.Unix() { + t.Fatalf("unexpected `exp` claim for auxiliary access token at %d, expected: %d, received: %d", i, test.TestTokenExpiry.Unix(), auxTokenClaims.Expires) } } } diff --git a/sdk/auth/client_credentials.go b/sdk/auth/client_credentials.go index 84116240a88..571a12ccd8f 100644 --- a/sdk/auth/client_credentials.go +++ b/sdk/auth/client_credentials.go @@ -314,11 +314,6 @@ func (a *ClientAssertionAuthorizer) AuxiliaryTokens(ctx context.Context, _ *http return tokens, nil } -// ExpireToken has no effect with uncached Authorizers -func (a *ClientAssertionAuthorizer) ExpireTokens() error { - return nil -} - func clientCredentialsToken(ctx context.Context, endpoint string, params *url.Values) (*oauth2.Token, error) { req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewBuffer([]byte(params.Encode()))) if err != nil { diff --git a/sdk/auth/client_secret_authorizer.go b/sdk/auth/client_secret_authorizer.go index 91fb5d23568..dcbd3ffe85a 100644 --- a/sdk/auth/client_secret_authorizer.go +++ b/sdk/auth/client_secret_authorizer.go @@ -131,8 +131,3 @@ func (a *ClientSecretAuthorizer) AuxiliaryTokens(ctx context.Context, _ *http.Re return tokens, nil } - -// ExpireToken has no effect with uncached Authorizers -func (a *ClientSecretAuthorizer) ExpireTokens() error { - return nil -} diff --git a/sdk/auth/github_oidc_authorizer.go b/sdk/auth/github_oidc_authorizer.go index 4a6212d5212..a369ebf9828 100644 --- a/sdk/auth/github_oidc_authorizer.go +++ b/sdk/auth/github_oidc_authorizer.go @@ -190,11 +190,6 @@ type gitHubOIDCConfig struct { Audience string } -// ExpireToken has no effect with uncached Authorizers -func (a *GitHubOIDCAuthorizer) ExpireTokens() error { - return nil -} - func (c *gitHubOIDCConfig) TokenSource(ctx context.Context) (Authorizer, error) { return NewCachedAuthorizer(&GitHubOIDCAuthorizer{ conf: c, diff --git a/sdk/auth/interface.go b/sdk/auth/interface.go index fe5256e5e9b..4736418a1d4 100644 --- a/sdk/auth/interface.go +++ b/sdk/auth/interface.go @@ -12,9 +12,20 @@ import ( // Authorizer is anything that can return an access token for authorizing API connections type Authorizer interface { + // Token obtains a new access token for the configured tenant Token(ctx context.Context, request *http.Request) (*oauth2.Token, error) + + // AuxiliaryTokens obtains new access tokens for the configured auxiliary tenants AuxiliaryTokens(ctx context.Context, request *http.Request) ([]*oauth2.Token, error) - ExpireTokens() error +} + +// CachingAuthorizer implements Authorizer whilst caching access tokens and offering a way to intentionally invalidate them +type CachingAuthorizer interface { + Authorizer + + // InvalidateCachedTokens invalidates any cached access tokens, so that new tokens are automatically + // retrieved from the authorization service on the next call to Token or AuxiliaryTokens. + InvalidateCachedTokens() error } // HTTPClient is an HTTP client used for sending authentication requests and obtaining tokens diff --git a/sdk/auth/managed_identity_authorizer.go b/sdk/auth/managed_identity_authorizer.go index 3a16e553f8c..718e6693d98 100644 --- a/sdk/auth/managed_identity_authorizer.go +++ b/sdk/auth/managed_identity_authorizer.go @@ -118,11 +118,6 @@ func (a *ManagedIdentityAuthorizer) AuxiliaryTokens(_ context.Context, _ *http.R return []*oauth2.Token{}, nil } -// ExpireToken has no effect with uncached Authorizers -func (a *ManagedIdentityAuthorizer) ExpireTokens() error { - return nil -} - // managedIdentityConfig configures an ManagedIdentityAuthorizer. type managedIdentityConfig struct { // ClientID is optionally used to determine which application to assume when a resource has multiple managed identities diff --git a/sdk/auth/shared_key_authorizer.go b/sdk/auth/shared_key_authorizer.go index 4e41c87cedc..3a9ff1ea7ba 100644 --- a/sdk/auth/shared_key_authorizer.go +++ b/sdk/auth/shared_key_authorizer.go @@ -64,11 +64,6 @@ func (s *SharedKeyAuthorizer) AuxiliaryTokens(_ context.Context, _ *http.Request return []*oauth2.Token{}, nil } -// ExpireToken has no effect with shared keys -func (c *SharedKeyAuthorizer) ExpireTokens() error { - return fmt.Errorf("SharedKeyAuthorizer tokens cannot expire") -} - const ( storageEmulatorAccountName string = "devstoreaccount1" diff --git a/sdk/auth/token.go b/sdk/auth/token.go index ea06e560c97..ddd31d17c99 100644 --- a/sdk/auth/token.go +++ b/sdk/auth/token.go @@ -7,16 +7,15 @@ import ( "context" "fmt" "net/http" - "strings" "time" "github.com/hashicorp/go-azure-sdk/sdk/claims" "golang.org/x/oauth2" ) -// SetAuthHeaders decorates a *http.Request with necessary authorization headers for Azure APIs. For more information about the vendor-specific -// `x-ms-authorization-auxiliary` header, see https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/authenticate-multi-tenant -func SetAuthHeaders(ctx context.Context, req *http.Request, authorizer Authorizer) error { +// SetAuthHeader decorates a *http.Request with the Authorization header using a bearer token obtained from the Token +// method of the supplied Authorizer. +func SetAuthHeader(ctx context.Context, req *http.Request, authorizer Authorizer) error { if req == nil { return fmt.Errorf("request was nil") } @@ -35,17 +34,6 @@ func SetAuthHeaders(ctx context.Context, req *http.Request, authorizer Authorize req.Header.Set("Authorization", fmt.Sprintf("%s %s", token.Type(), token.AccessToken)) - auxTokens, err := authorizer.AuxiliaryTokens(ctx, req) - if err != nil { - return err - } - - auxTokenValues := make([]string, 0) - for _, auxToken := range auxTokens { - auxTokenValues = append(auxTokenValues, fmt.Sprintf("%s %s", auxToken.Type(), auxToken.AccessToken)) - } - req.Header.Set("X-Ms-Authorization-Auxiliary", strings.Join(auxTokenValues, ", ")) - return nil } diff --git a/sdk/auth/token_test.go b/sdk/auth/token_test.go index 7eaa15365d0..f820c2797d8 100644 --- a/sdk/auth/token_test.go +++ b/sdk/auth/token_test.go @@ -2,89 +2,19 @@ package auth_test import ( "context" - "crypto/ecdsa" - "crypto/elliptic" - "crypto/rand" - "fmt" "net/http" "regexp" - "strings" "testing" - "time" - "github.com/golang-jwt/jwt/v4" "github.com/hashicorp/go-azure-sdk/sdk/auth" - "golang.org/x/oauth2" + "github.com/hashicorp/go-azure-sdk/sdk/internal/test" ) -var ( - testTokenIssued = time.Now() - testTokenExpiry = time.Now().Add(3599 * time.Second) -) - -var _ auth.Authorizer = &testAuthorizer{} - -type testAuthorizer struct{} - -func (*testAuthorizer) Token(_ context.Context, _ *http.Request) (*oauth2.Token, error) { - return &oauth2.Token{ - AccessToken: testAccessToken(), - TokenType: "Bearer", - Expiry: testTokenExpiry, - }, nil -} - -func (*testAuthorizer) AuxiliaryTokens(_ context.Context, _ *http.Request) ([]*oauth2.Token, error) { - return []*oauth2.Token{ - { - AccessToken: testAccessToken(), - TokenType: "Bearer", - Expiry: testTokenExpiry, - }, - { - AccessToken: testAccessToken(), - TokenType: "Bearer", - Expiry: testTokenExpiry, - }, - { - AccessToken: testAccessToken(), - TokenType: "Bearer", - Expiry: testTokenExpiry, - }, - }, nil -} - -func (*testAuthorizer) ExpireTokens() error { - return nil -} - -func testAccessToken() string { - token := jwt.NewWithClaims(jwt.SigningMethodES256, jwt.MapClaims{ - "appid": "10000000-2000-3000-4000-500000000000", - "aud": "https://not.azure.net", - "exp": testTokenExpiry.Unix(), - "iat": testTokenIssued.Unix(), - "iss": "Not Azure", - "nbf": testTokenIssued.Unix(), - "oid": "aaaaaa48-bb50-cc40-dd17-beeeeeeeeeef", - "sub": "12345678-1234-5678-90ab-1234567890ab", - "tid": "99999999-8888-7777-6666-555555543210", - }) - - key, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - ret, err := token.SignedString(key) - if err != nil { - panic(fmt.Sprintf("failed to sign test token: %+v", err)) - } - - return ret -} - -func TestSetAuthHeaders(t *testing.T) { +func TestSetAuthHeader(t *testing.T) { req := &http.Request{} - authorizer := &testAuthorizer{} + authorizer := &test.TestAuthorizer{} - err := auth.SetAuthHeaders(context.Background(), req, authorizer) + err := auth.SetAuthHeader(context.Background(), req, authorizer) if err != nil { t.Fatalf("received error: %+v", err) } @@ -93,13 +23,4 @@ func TestSetAuthHeaders(t *testing.T) { if val := req.Header.Get("Authorization"); !expected.MatchString(val) { t.Fatalf("Authorization header mismatch, received: %q", val) } - auxVals := strings.Split(req.Header.Get("X-Ms-Authorization-Auxiliary"), ",") - if len(auxVals) != 3 { - t.Fatalf("X-Ms-Authorization-Auxiliary mismatch, should contain 3 tokens, received: %q", strings.Join(auxVals, ",")) - } - for i, auxVal := range auxVals { - if !expected.MatchString(strings.TrimSpace(auxVal)) { - t.Fatalf("Authorization header mismatch for token %d, received: %q", i, auxVal) - } - } } diff --git a/sdk/client/client.go b/sdk/client/client.go index d08581c0569..5dc6879f87c 100644 --- a/sdk/client/client.go +++ b/sdk/client/client.go @@ -68,7 +68,8 @@ func RequestRetryAll(retryFuncs ...RequestRetryFunc) func(resp *http.Response, o } } -// RetryableErrorHandler simply returns the resp and err, this is needed to makes the retryablehttp client's Do() return early with the response body not drained. +// RetryableErrorHandler simply returns the resp and err, this is needed to make the Do() method +// of retryablehttp client return early with the response body not drained. func RetryableErrorHandler(resp *http.Response, err error, _ int) (*http.Response, error) { return resp, err } @@ -260,6 +261,11 @@ type Client struct { // Authorizer is anything that can provide an access token with which to authorize requests. Authorizer auth.Authorizer + // AuthorizeRequest is an optional function to decorate a Request for authorization prior to being sent. + // When nil, a standard Authorization header will be added using a bearer token as returned by the Token method + // of the configured Authorizer. Define this function in order to customize the request authorization. + AuthorizeRequest func(context.Context, *http.Request, auth.Authorizer) error + // DisableRetries prevents the client from reattempting failed requests (which it does to work around eventual consistency issues). // This does not impact handling of retries related to rate limiting, which are always performed. DisableRetries bool @@ -327,10 +333,14 @@ func (c *Client) Execute(ctx context.Context, req *Request) (*Response, error) { return nil, fmt.Errorf("req.Request was nil") } - // Set Authorization and X-Ms-Authorization-Auxiliary headers - if c.Authorizer != nil { - if err := auth.SetAuthHeaders(ctx, req.Request, c.Authorizer); err != nil { - return nil, err + // Authorize the request + if c.AuthorizeRequest != nil { + if err := c.AuthorizeRequest(ctx, req.Request, c.Authorizer); err != nil { + return nil, fmt.Errorf("authorizing request: %+v", err) + } + } else if c.Authorizer != nil { + if err := auth.SetAuthHeader(ctx, req.Request, c.Authorizer); err != nil { + return nil, fmt.Errorf("authorizing request: %+v", err) } } diff --git a/sdk/client/dataplane/storage/client.go b/sdk/client/dataplane/storage/client.go index 40804d92526..47f74a98165 100644 --- a/sdk/client/dataplane/storage/client.go +++ b/sdk/client/dataplane/storage/client.go @@ -30,6 +30,7 @@ func NewBaseClient(baseUri string, componentName, apiVersion string) (*BaseClien } func (c *BaseClient) NewRequest(ctx context.Context, input client.RequestOptions) (*client.Request, error) { + // TODO move these validations to base client method if _, ok := ctx.Deadline(); !ok { return nil, fmt.Errorf("the context used must have a deadline attached for polling purposes, but got no deadline") } diff --git a/sdk/client/msgraph/client.go b/sdk/client/msgraph/client.go index 32ffa14d51a..3cf67d08f25 100644 --- a/sdk/client/msgraph/client.go +++ b/sdk/client/msgraph/client.go @@ -50,6 +50,7 @@ func NewMsGraphClient(api environments.Api, serviceName string, apiVersion ApiVe } func (c *Client) NewRequest(ctx context.Context, input client.RequestOptions) (*client.Request, error) { + // TODO move these validations to base client method if _, ok := ctx.Deadline(); !ok { return nil, fmt.Errorf("the context used must have a deadline attached for polling purposes, but got no deadline") } diff --git a/sdk/client/resourcemanager/authorization.go b/sdk/client/resourcemanager/authorization.go new file mode 100644 index 00000000000..7f1273e98b0 --- /dev/null +++ b/sdk/client/resourcemanager/authorization.go @@ -0,0 +1,43 @@ +package resourcemanager + +import ( + "context" + "fmt" + "net/http" + "strings" + + "github.com/hashicorp/go-azure-sdk/sdk/auth" +) + +func AuthorizeResourceManagerRequest(ctx context.Context, req *http.Request, authorizer auth.Authorizer) error { + if req == nil { + return fmt.Errorf("request was nil") + } + if authorizer == nil { + return fmt.Errorf("authorizer was nil") + } + + token, err := authorizer.Token(ctx, req) + if err != nil { + return err + } + + if req.Header == nil { + req.Header = make(http.Header) + } + + req.Header.Set("Authorization", fmt.Sprintf("%s %s", token.Type(), token.AccessToken)) + + auxTokens, err := authorizer.AuxiliaryTokens(ctx, req) + if err != nil { + return err + } + + auxTokenValues := make([]string, 0) + for _, auxToken := range auxTokens { + auxTokenValues = append(auxTokenValues, fmt.Sprintf("%s %s", auxToken.Type(), auxToken.AccessToken)) + } + req.Header.Set("X-Ms-Authorization-Auxiliary", strings.Join(auxTokenValues, ", ")) + + return nil +} diff --git a/sdk/client/resourcemanager/authorization_test.go b/sdk/client/resourcemanager/authorization_test.go new file mode 100644 index 00000000000..bb550042f01 --- /dev/null +++ b/sdk/client/resourcemanager/authorization_test.go @@ -0,0 +1,36 @@ +package resourcemanager_test + +import ( + "context" + "net/http" + "regexp" + "strings" + "testing" + + "github.com/hashicorp/go-azure-sdk/sdk/client/resourcemanager" + "github.com/hashicorp/go-azure-sdk/sdk/internal/test" +) + +func TestAuthorizeResourceManagerRequest(t *testing.T) { + req := &http.Request{} + authorizer := &test.TestAuthorizer{} + + err := resourcemanager.AuthorizeResourceManagerRequest(context.Background(), req, authorizer) + if err != nil { + t.Fatalf("received error: %+v", err) + } + + expected := regexp.MustCompile("^Bearer [a-zA-Z0-9_-]+[.][a-zA-Z0-9_-]+[.][a-zA-Z0-9_-]+") + if val := req.Header.Get("Authorization"); !expected.MatchString(val) { + t.Fatalf("Authorization header mismatch, received: %q", val) + } + auxVals := strings.Split(req.Header.Get("X-Ms-Authorization-Auxiliary"), ",") + if len(auxVals) != 3 { + t.Fatalf("X-Ms-Authorization-Auxiliary mismatch, should contain 3 tokens, received: %q", strings.Join(auxVals, ",")) + } + for i, auxVal := range auxVals { + if !expected.MatchString(strings.TrimSpace(auxVal)) { + t.Fatalf("Authorization header mismatch for token %d, received: %q", i, auxVal) + } + } +} diff --git a/sdk/client/resourcemanager/client.go b/sdk/client/resourcemanager/client.go index d712ca12c83..104b4c002f2 100644 --- a/sdk/client/resourcemanager/client.go +++ b/sdk/client/resourcemanager/client.go @@ -28,7 +28,10 @@ func NewResourceManagerClient(api environments.Api, serviceName, apiVersion stri if !ok { return nil, fmt.Errorf("no `endpoint` was returned for this environment") } + baseClient := client.NewClient(*endpoint, serviceName, apiVersion) + baseClient.AuthorizeRequest = AuthorizeResourceManagerRequest + return &Client{ Client: baseClient, apiVersion: apiVersion, @@ -36,6 +39,7 @@ func NewResourceManagerClient(api environments.Api, serviceName, apiVersion stri } func (c *Client) NewRequest(ctx context.Context, input client.RequestOptions) (*client.Request, error) { + // TODO move these validations to base client method if _, ok := ctx.Deadline(); !ok { return nil, fmt.Errorf("the context used must have a deadline attached for polling purposes, but got no deadline") } diff --git a/sdk/internal/test/auth.go b/sdk/internal/test/auth.go new file mode 100644 index 00000000000..40218771e57 --- /dev/null +++ b/sdk/internal/test/auth.go @@ -0,0 +1,78 @@ +package test + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "fmt" + "net/http" + "time" + + "github.com/golang-jwt/jwt/v4" + "github.com/hashicorp/go-azure-sdk/sdk/auth" + "golang.org/x/oauth2" +) + +var ( + TestTokenIssued = time.Now() + TestTokenExpiry = time.Now().Add(3599 * time.Second) +) + +var _ auth.Authorizer = &TestAuthorizer{} + +type TestAuthorizer struct{} + +func (*TestAuthorizer) Token(_ context.Context, _ *http.Request) (*oauth2.Token, error) { + return &oauth2.Token{ + AccessToken: testAccessToken(), + TokenType: "Bearer", + Expiry: TestTokenExpiry, + }, nil +} + +func (*TestAuthorizer) AuxiliaryTokens(_ context.Context, _ *http.Request) ([]*oauth2.Token, error) { + return []*oauth2.Token{ + { + AccessToken: testAccessToken(), + TokenType: "Bearer", + Expiry: TestTokenExpiry, + }, + { + AccessToken: testAccessToken(), + TokenType: "Bearer", + Expiry: TestTokenExpiry, + }, + { + AccessToken: testAccessToken(), + TokenType: "Bearer", + Expiry: TestTokenExpiry, + }, + }, nil +} + +func (*TestAuthorizer) ExpireTokens() error { + return nil +} + +func testAccessToken() string { + token := jwt.NewWithClaims(jwt.SigningMethodES256, jwt.MapClaims{ + "appid": "10000000-2000-3000-4000-500000000000", + "aud": "https://not.azure.net", + "exp": TestTokenExpiry.Unix(), + "iat": TestTokenIssued.Unix(), + "iss": "Not Azure", + "nbf": TestTokenIssued.Unix(), + "oid": "aaaaaa48-bb50-cc40-dd17-beeeeeeeeeef", + "sub": "12345678-1234-5678-90ab-1234567890ab", + "tid": "99999999-8888-7777-6666-555555543210", + }) + + key, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + ret, err := token.SignedString(key) + if err != nil { + panic(fmt.Sprintf("failed to sign test token: %+v", err)) + } + + return ret +} From 55249e02dc3fd6b8db19659f53c968873f41418e Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Mon, 27 Nov 2023 18:55:33 +0000 Subject: [PATCH 12/13] auth: ignore missing token when invalidating cache --- sdk/auth/cached_authorizer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/auth/cached_authorizer.go b/sdk/auth/cached_authorizer.go index 6e89884857d..c01c57a56aa 100644 --- a/sdk/auth/cached_authorizer.go +++ b/sdk/auth/cached_authorizer.go @@ -72,7 +72,7 @@ func (c *CachedAuthorizer) AuxiliaryTokens(ctx context.Context, req *http.Reques // tokens to be acquired when Token() or AuxiliaryTokens() are next called func (c *CachedAuthorizer) InvalidateCachedTokens() error { if c.token == nil { - return fmt.Errorf("internal-error: c.token was nil") + return nil } c.token.Expiry = time.Now() for i := range c.auxTokens { From 6445851d2d7c61a951e89866ccb2376699c70b49 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Mon, 27 Nov 2023 18:56:30 +0000 Subject: [PATCH 13/13] resourcemanager: don't set x-ms-authorization-auxiliary header when no aux tokens are present --- sdk/client/resourcemanager/authorization.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sdk/client/resourcemanager/authorization.go b/sdk/client/resourcemanager/authorization.go index 7f1273e98b0..7a6bdc04cce 100644 --- a/sdk/client/resourcemanager/authorization.go +++ b/sdk/client/resourcemanager/authorization.go @@ -37,7 +37,9 @@ func AuthorizeResourceManagerRequest(ctx context.Context, req *http.Request, aut for _, auxToken := range auxTokens { auxTokenValues = append(auxTokenValues, fmt.Sprintf("%s %s", auxToken.Type(), auxToken.AccessToken)) } - req.Header.Set("X-Ms-Authorization-Auxiliary", strings.Join(auxTokenValues, ", ")) + if len(auxTokenValues) > 0 { + req.Header.Set("X-Ms-Authorization-Auxiliary", strings.Join(auxTokenValues, ", ")) + } return nil }