Skip to content

Update azure-identity history and version #9437

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.

Sign up for GitHub

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

Merged
chlowell merged 2 commits into from
Jan 13, 2020
Merged

Update azure-identity history and version #9437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8aa045d
azure-identity 1.2.0
chlowell Jan 13, 2020
4f33abf
auth code credential is an async context manager too
chlowell Jan 13, 2020
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
2 changes: 1 addition & 1 deletion sdk/identity/azure-identity/HISTORY.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Release History

## 1.1.1 (Unreleased)
## 1.2.0 (2020-01-14)

- All credential pipelines include `ProxyPolicy`
([#8945](https://github.com/Azure/azure-sdk-for-python/pull/8945))
Expand Down
2 changes: 1 addition & 1 deletion sdk/identity/azure-identity/azure/identity/_version.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.
# ------------------------------------
VERSION = "1.1.1"
VERSION = "1.2.0"
21 changes: 14 additions & 7 deletions sdk/identity/azure-identity/azure/identity/aio/_credentials/authorization_code.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from typing import TYPE_CHECKING

from azure.core.exceptions import ClientAuthenticationError
from .base import AsyncCredentialBase
from .._internal import AadClient

if TYPE_CHECKING:
Expand All @@ -14,7 +15,7 @@
from azure.core.credentials import AccessToken


class AuthorizationCodeCredential(object):
class AuthorizationCodeCredential(AsyncCredentialBase):
"""Authenticates by redeeming an authorization code previously obtained from Azure Active Directory.

See https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-auth-code-flow for more information
Expand All @@ -31,13 +32,19 @@ class AuthorizationCodeCredential(object):
:keyword str client_secret: One of the application's client secrets. Required only for web apps and web APIs.
"""

async def __aenter__(self):
if self._client:
await self._client.__aenter__()
return self

async def close(self):
"""Close the credential's transport session."""

if self._client:
await self._client.__aexit__()

def __init__(
self,
tenant_id: str,
client_id: str,
authorization_code: str,
redirect_uri: str,
**kwargs: "Any"
self, tenant_id: str, client_id: str, authorization_code: str, redirect_uri: str, **kwargs: "Any"
) -> None:
self._authorization_code = authorization_code # type: Optional[str]
Copy link
Member

@xiangyan99 xiangyan99 Jan 13, 2020

Choose a reason for hiding this comment

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

Why optional?

Copy link
Member Author

@chlowell chlowell Jan 13, 2020

Choose a reason for hiding this comment

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

Note the private field is optional but the parameter is not. The authorization code is a string, so the private field is initially a string. Authorization codes are single use. After successfully redeeming the code for a token, get_token sets the private field to None to indicate the credential no longer has an authorization code and can only return a cached token. So, Optional[str].

self._client_id = client_id
Expand Down
28 changes: 27 additions & 1 deletion sdk/identity/azure-identity/tests/test_auth_code_async.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import pytest

from helpers import build_aad_response, mock_response, Request
from helpers_async import async_validating_transport, wrap_in_future
from helpers_async import async_validating_transport, AsyncMockTransport, wrap_in_future


@pytest.mark.asyncio
Expand All @@ -32,6 +32,32 @@ async def send(*_, **__):
assert policy.on_request.called


@pytest.mark.asyncio
async def test_close():
transport = AsyncMockTransport()
credential = AuthorizationCodeCredential(
"tenant-id", "client-id", "auth-code", "http://localhost", transport=transport
)

await credential.close()

assert transport.__aexit__.call_count == 1


@pytest.mark.asyncio
async def test_context_manager():
transport = AsyncMockTransport()
credential = AuthorizationCodeCredential(
"tenant-id", "client-id", "auth-code", "http://localhost", transport=transport
)

async with credential:
assert transport.__aenter__.call_count == 1

assert transport.__aenter__.call_count == 1
assert transport.__aexit__.call_count == 1

Copy link
Member

@xiangyan99 xiangyan99 Jan 13, 2020

Choose a reason for hiding this comment

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

Can you add credential.close() after the context manager to make sure double close does not cause problems?

Copy link
Member Author

@chlowell chlowell Jan 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I could, but the credential just wraps the transport's methods, which these tests already verify. It seems like what you really want is to ensure double closing the transport is okay. I want that too, but I think it's a test for the real transport implementations.


@pytest.mark.asyncio
async def test_user_agent():
transport = async_validating_transport(
Expand Down