From 380449fbcfdf1d4ae536855ed17247a31be741fe Mon Sep 17 00:00:00 2001 From: Joel Hendrix Date: Fri, 11 Dec 2020 08:03:43 -0800 Subject: [PATCH] Add redirectURI param back to auth code flow (#13942) --- sdk/azidentity/aad_identity_client.go | 11 +++++------ .../authorization_code_credential.go | 8 +++++--- .../authorization_code_credential_test.go | 18 +++++++++++------- 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/sdk/azidentity/aad_identity_client.go b/sdk/azidentity/aad_identity_client.go index ed0835de0f51..df4b3ba4d144 100644 --- a/sdk/azidentity/aad_identity_client.go +++ b/sdk/azidentity/aad_identity_client.go @@ -390,7 +390,7 @@ func (c *aadIdentityClient) authenticateInteractiveBrowser(ctx context.Context, if err != nil { return nil, err } - return c.authenticateAuthCode(ctx, tenantID, clientID, cfg.authCode, clientSecret, scopes) + return c.authenticateAuthCode(ctx, tenantID, clientID, cfg.authCode, clientSecret, cfg.redirectURI, scopes) } // authenticateAuthCode requests an Access Token with the authorization code and returns the token or an error in case of authentication failure. @@ -401,8 +401,8 @@ func (c *aadIdentityClient) authenticateInteractiveBrowser(ctx context.Context, // clientSecret: Gets the client secret that was generated for the App Registration used to authenticate the client. // redirectURI: The redirect URI that was used to request the authorization code. Must be the same URI that is configured for the App Registration. // scopes: The scopes required for the token -func (c *aadIdentityClient) authenticateAuthCode(ctx context.Context, tenantID string, clientID string, authCode string, clientSecret string, scopes []string) (*azcore.AccessToken, error) { - req, err := c.createAuthorizationCodeAuthRequest(ctx, tenantID, clientID, authCode, clientSecret, scopes) +func (c *aadIdentityClient) authenticateAuthCode(ctx context.Context, tenantID string, clientID string, authCode string, clientSecret string, redirectURI string, scopes []string) (*azcore.AccessToken, error) { + req, err := c.createAuthorizationCodeAuthRequest(ctx, tenantID, clientID, authCode, clientSecret, redirectURI, scopes) if err != nil { return nil, err } @@ -420,15 +420,14 @@ func (c *aadIdentityClient) authenticateAuthCode(ctx context.Context, tenantID s } // createAuthorizationCodeAuthRequest creates a request for an Access Token for authorization_code grant types. -func (c *aadIdentityClient) createAuthorizationCodeAuthRequest(ctx context.Context, tenantID string, clientID string, authCode string, clientSecret string, scopes []string) (*azcore.Request, error) { +func (c *aadIdentityClient) createAuthorizationCodeAuthRequest(ctx context.Context, tenantID string, clientID string, authCode string, clientSecret string, redirectURI string, scopes []string) (*azcore.Request, error) { data := url.Values{} data.Set(qpGrantType, "authorization_code") data.Set(qpClientID, clientID) if clientSecret != "" { data.Set(qpClientSecret, clientSecret) // only for web apps } - // similar to MSAL, we use a hard-coded redirect URI here as we never actually redirect to it - data.Set(qpRedirectURI, "https://login.microsoftonline.com/common/oauth2/nativeclient") + data.Set(qpRedirectURI, redirectURI) data.Set(qpScope, strings.Join(scopes, " ")) data.Set(qpCode, authCode) dataEncoded := data.Encode() diff --git a/sdk/azidentity/authorization_code_credential.go b/sdk/azidentity/authorization_code_credential.go index c96d3d00fdf5..4f6c5036a468 100644 --- a/sdk/azidentity/authorization_code_credential.go +++ b/sdk/azidentity/authorization_code_credential.go @@ -43,14 +43,16 @@ type AuthorizationCodeCredential struct { clientID string // Gets the client (application) ID of the service principal authCode string // The authorization code received from the authorization code flow. The authorization code must not have been used to obtain another token. clientSecret string // Gets the client secret that was generated for the App Registration used to authenticate the client. + redirectURI string // The redirect URI that was used to request the authorization code. Must be the same URI that is configured for the App Registration. } // NewAuthorizationCodeCredential constructs a new AuthorizationCodeCredential with the details needed to authenticate against Azure Active Directory with an authorization code. // tenantID: The Azure Active Directory tenant (directory) ID of the service principal. // clientID: The client (application) ID of the service principal. // authCode: The authorization code received from the authorization code flow. The authorization code must not have been used to obtain another token. +// redirectURL: The redirect URL that was used to request the authorization code. Must be the same URL that is configured for the App Registration. // options: Manage the configuration of the requests sent to Azure Active Directory, they can also include a client secret for web app authentication. -func NewAuthorizationCodeCredential(tenantID string, clientID string, authCode string, options *AuthorizationCodeCredentialOptions) (*AuthorizationCodeCredential, error) { +func NewAuthorizationCodeCredential(tenantID string, clientID string, authCode string, redirectURL string, options *AuthorizationCodeCredentialOptions) (*AuthorizationCodeCredential, error) { if !validTenantID(tenantID) { return nil, &CredentialUnavailableError{credentialType: "Authorization Code Credential", message: tenantIDValidationErr} } @@ -66,7 +68,7 @@ func NewAuthorizationCodeCredential(tenantID string, clientID string, authCode s if err != nil { return nil, err } - return &AuthorizationCodeCredential{tenantID: tenantID, clientID: clientID, authCode: authCode, clientSecret: options.ClientSecret, client: c}, nil + return &AuthorizationCodeCredential{tenantID: tenantID, clientID: clientID, authCode: authCode, clientSecret: options.ClientSecret, redirectURI: redirectURL, client: c}, nil } // GetToken obtains a token from Azure Active Directory, using the specified authorization code to authenticate. @@ -74,7 +76,7 @@ func NewAuthorizationCodeCredential(tenantID string, clientID string, authCode s // opts: TokenRequestOptions contains the list of scopes for which the token will have access. // Returns an AccessToken which can be used to authenticate service client calls. func (c *AuthorizationCodeCredential) GetToken(ctx context.Context, opts azcore.TokenRequestOptions) (*azcore.AccessToken, error) { - tk, err := c.client.authenticateAuthCode(ctx, c.tenantID, c.clientID, c.authCode, c.clientSecret, opts.Scopes) + tk, err := c.client.authenticateAuthCode(ctx, c.tenantID, c.clientID, c.authCode, c.clientSecret, c.redirectURI, opts.Scopes) if err != nil { addGetTokenFailureLogs("Authorization Code Credential", err, true) return nil, err diff --git a/sdk/azidentity/authorization_code_credential_test.go b/sdk/azidentity/authorization_code_credential_test.go index bc92af429d80..7ad95e15b647 100644 --- a/sdk/azidentity/authorization_code_credential_test.go +++ b/sdk/azidentity/authorization_code_credential_test.go @@ -16,11 +16,12 @@ import ( ) const ( - testAuthCode = "12345" + testAuthCode = "12345" + testRedirectURI = "http://localhost" ) func TestAuthorizationCodeCredential_InvalidTenantID(t *testing.T) { - cred, err := NewAuthorizationCodeCredential(badTenantID, clientID, testAuthCode, nil) + cred, err := NewAuthorizationCodeCredential(badTenantID, clientID, testAuthCode, testRedirectURI, nil) if err == nil { t.Fatal("Expected an error but received none") } @@ -34,11 +35,11 @@ func TestAuthorizationCodeCredential_InvalidTenantID(t *testing.T) { } func TestAuthorizationCodeCredential_CreateAuthRequestSuccess(t *testing.T) { - cred, err := NewAuthorizationCodeCredential(tenantID, clientID, testAuthCode, nil) + cred, err := NewAuthorizationCodeCredential(tenantID, clientID, testAuthCode, testRedirectURI, nil) if err != nil { t.Fatalf("Unable to create credential. Received: %v", err) } - req, err := cred.client.createAuthorizationCodeAuthRequest(context.Background(), cred.tenantID, cred.clientID, cred.authCode, cred.clientSecret, []string{scope}) + req, err := cred.client.createAuthorizationCodeAuthRequest(context.Background(), cred.tenantID, cred.clientID, cred.authCode, cred.clientSecret, cred.redirectURI, []string{scope}) if err != nil { t.Fatalf("Unexpectedly received an error: %v", err) } @@ -63,6 +64,9 @@ func TestAuthorizationCodeCredential_CreateAuthRequestSuccess(t *testing.T) { if reqQueryParams[qpCode][0] != testAuthCode { t.Fatal("Unexpected authorization code") } + if reqQueryParams[qpRedirectURI][0] != testRedirectURI { + t.Fatal("Unexpected redirectURI") + } if req.Request.URL.Host != defaultTestAuthorityHost { t.Fatal("Unexpected default authority host") } @@ -79,7 +83,7 @@ func TestAuthorizationCodeCredential_GetTokenSuccess(t *testing.T) { options.ClientSecret = secret options.AuthorityHost = srv.URL() options.HTTPClient = srv - cred, err := NewAuthorizationCodeCredential(tenantID, clientID, testAuthCode, &options) + cred, err := NewAuthorizationCodeCredential(tenantID, clientID, testAuthCode, testRedirectURI, &options) if err != nil { t.Fatalf("Unable to create credential. Received: %v", err) } @@ -97,7 +101,7 @@ func TestAuthorizationCodeCredential_GetTokenInvalidCredentials(t *testing.T) { options.ClientSecret = secret options.AuthorityHost = srv.URL() options.HTTPClient = srv - cred, err := NewAuthorizationCodeCredential(tenantID, clientID, testAuthCode, &options) + cred, err := NewAuthorizationCodeCredential(tenantID, clientID, testAuthCode, testRedirectURI, &options) if err != nil { t.Fatalf("Unable to create credential. Received: %v", err) } @@ -146,7 +150,7 @@ func TestAuthorizationCodeCredential_GetTokenUnexpectedJSON(t *testing.T) { options.ClientSecret = secret options.AuthorityHost = srv.URL() options.HTTPClient = srv - cred, err := NewAuthorizationCodeCredential(tenantID, clientID, testAuthCode, &options) + cred, err := NewAuthorizationCodeCredential(tenantID, clientID, testAuthCode, testRedirectURI, &options) if err != nil { t.Fatalf("Failed to create the credential") }