Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add redirectURI param back to auth code flow #13942

Merged
merged 1 commit into from
Dec 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions sdk/azidentity/aad_identity_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}
Expand All @@ -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()
Expand Down
8 changes: 5 additions & 3 deletions sdk/azidentity/authorization_code_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
}
Expand All @@ -66,15 +68,15 @@ 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.
// ctx: Context used to control the request lifetime.
// 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
Expand Down
18 changes: 11 additions & 7 deletions sdk/azidentity/authorization_code_credential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand All @@ -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)
}
Expand All @@ -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")
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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")
}
Expand Down