Skip to content

Commit

Permalink
Default to constructor scopes
Browse files Browse the repository at this point in the history
  • Loading branch information
mccoyp committed May 2, 2022
1 parent 7b12675 commit c409f0b
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ class BearerTokenChallengePolicy(BearerTokenCredentialPolicy):
:param str scopes: Lets you specify the type of access needed.
:keyword bool discover_tenant: Determines if tenant discovery should be enabled. Defaults to True.
:keyword bool discover_scopes: Determines if scopes from authentication challenges should be provided to token
requests, instead of the scopes given to the policy's constructor. Defaults to True.
requests, instead of the scopes given to the policy's constructor, if any are present. Defaults to True.
:raises: :class:`~azure.core.exceptions.ServiceRequestError`
"""

Expand Down Expand Up @@ -257,7 +257,10 @@ def on_challenge(self, request: "PipelineRequest", response: "PipelineResponse")
try:
challenge = _HttpChallenge(response.http_response.headers.get("WWW-Authenticate"))
# azure-identity credentials require an AADv2 scope but the challenge may specify an AADv1 resource
# if no scopes are included in the challenge, challenge.scope and challenge.resource will both be ''
scope = challenge.scope or challenge.resource + "/.default" if self._discover_scopes else self._scopes
if scope == "/.default":
scope = self._scopes
except ValueError:
return False

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ class AsyncBearerTokenChallengePolicy(AsyncBearerTokenCredentialPolicy):
:param str scopes: Lets you specify the type of access needed.
:keyword bool discover_tenant: Determines if tenant discovery should be enabled. Defaults to True.
:keyword bool discover_scopes: Determines if scopes from authentication challenges should be provided to token
requests, instead of the scopes given to the policy's constructor. Defaults to True.
requests, instead of the scopes given to the policy's constructor, if any are present. Defaults to True.
:raises: :class:`~azure.core.exceptions.ServiceRequestError`
"""

Expand Down Expand Up @@ -173,7 +173,10 @@ async def on_challenge(self, request: "PipelineRequest", response: "PipelineResp
try:
challenge = _HttpChallenge(response.http_response.headers.get("WWW-Authenticate"))
# azure-identity credentials require an AADv2 scope but the challenge may specify an AADv1 resource
# if no scopes are included in the challenge, challenge.scope and challenge.resource will both be ''
scope = challenge.scope or challenge.resource + "/.default" if self._discover_scopes else self._scopes
if scope == "/.default":
scope = self._scopes
except ValueError:
return False

Expand Down
68 changes: 64 additions & 4 deletions sdk/core/azure-core/tests/async_tests/test_authentication_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ async def fake_send(*args, **kwargs):


@pytest.mark.parametrize("http_request", HTTP_REQUESTS)
async def test_multitenant_policy_uses_scopes_and_tenant(http_request):
async def test_challenge_policy_uses_scopes_and_tenant(http_request):
"""The policy's token requests should pass the parsed scope and tenant ID from the challenge"""

async def test_with_challenge(challenge, expected_scope, expected_tenant):
Expand Down Expand Up @@ -298,7 +298,7 @@ async def get_token(*scopes, **kwargs):


@pytest.mark.parametrize("http_request", HTTP_REQUESTS)
async def test_multitenant_policy_disable_tenant_discovery(http_request):
async def test_challenge_policy_disable_tenant_discovery(http_request):
"""The policy's token requests should exclude the challenge's tenant if requested"""

async def test_with_challenge(challenge, challenge_scope):
Expand Down Expand Up @@ -358,7 +358,7 @@ async def get_token(*scopes, **kwargs):


@pytest.mark.parametrize("http_request", HTTP_REQUESTS)
async def test_multitenant_policy_disable_scopes_discovery(http_request):
async def test_challenge_policy_disable_scopes_discovery(http_request):
"""The policy's token requests should exclude the challenge's scope if requested"""

async def test_with_challenge(challenge, challenge_scope, challenge_tenant):
Expand Down Expand Up @@ -418,7 +418,7 @@ async def get_token(*scopes, **kwargs):


@pytest.mark.parametrize("http_request", HTTP_REQUESTS)
async def test_multitenant_policy_disable_any_discovery(http_request):
async def test_challenge_policy_disable_any_discovery(http_request):
"""The policy shouldn't respond to the challenge if it can't use the provided scope or tenant"""

async def test_with_challenge(challenge, challenge_scope, challenge_tenant):
Expand Down Expand Up @@ -469,6 +469,66 @@ async def get_token(*scopes, **kwargs):
await test_with_challenge(challenge_with_commas, scope, tenant)


@pytest.mark.parametrize("http_request", HTTP_REQUESTS)
async def test_challenge_policy_no_scope_in_challenge(http_request):
"""The policy's token requests should use constructor scopes if none are in the challenge"""

async def test_with_challenge(challenge, challenge_scope, challenge_tenant):
bad_token = "bad_token"

class Requests:
count = 0

class TokenRequests:
count = 0

async def send(request):
Requests.count += 1
if Requests.count == 1:
# first request triggers a 401 response w/ auth challenge
assert bad_token in request.headers["Authorization"]
return challenge
elif Requests.count == 2:
# second request should still be unauthorized because we didn't authenticate with the correct scope
assert bad_token in request.headers["Authorization"]
return challenge
raise ValueError("unexpected request")

async def get_token(*scopes, **kwargs):
assert len(scopes) == 1
TokenRequests.count += 1
if TokenRequests.count == 1:
# first request uses the scope given to the policy, and no tenant ID
assert scopes[0] != challenge_scope
assert kwargs.get("tenant_id") is None
return AccessToken(bad_token, 0)
elif TokenRequests.count == 2:
# second request should use the tenant specified in the auth challenge, and same scope as before
assert scopes[0] != challenge_scope
assert kwargs.get("tenant_id") == challenge_tenant
return AccessToken(bad_token, 0)
raise ValueError("unexpected token request")

credential = Mock(get_token=Mock(wraps=get_token))
policy = AsyncBearerTokenChallengePolicy(credential, "scope")
pipeline = AsyncPipeline(policies=[policy], transport=Mock(send=send))
await pipeline.run(http_request("GET", "https://localhost"))

tenant = "tenant-id"
endpoint = f"https://authority.net/{tenant}/oauth2/authorize"
resource = "https://challenge.resource"
scope = f"{resource}/.default"

# this challenge has no `resource` or `scope` field
challenge_with_commas = Mock(
status_code=401,
headers={"WWW-Authenticate": f'Bearer authorization="{endpoint}"'},
)
# the request should fail after the challenge because we don't use the correct scope
# after the second 4xx response, the policy should raise the authentication error
await test_with_challenge(challenge_with_commas, scope, tenant)


def get_completed_future(result=None):
fut = asyncio.Future()
fut.set_result(result)
Expand Down
68 changes: 64 additions & 4 deletions sdk/core/azure-core/tests/test_authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ def raise_the_second_time(*args, **kwargs):


@pytest.mark.parametrize("http_request", HTTP_REQUESTS)
def test_multitenant_policy_uses_scopes_and_tenant(http_request):
def test_challenge_policy_uses_scopes_and_tenant(http_request):
"""The policy's token requests should pass the parsed scope and tenant ID from the challenge, by default"""

def test_with_challenge(challenge, expected_scope, challenge_tenant):
Expand Down Expand Up @@ -337,7 +337,7 @@ def get_token(*scopes, **kwargs):


@pytest.mark.parametrize("http_request", HTTP_REQUESTS)
def test_multitenant_policy_disable_tenant_discovery(http_request):
def test_challenge_policy_disable_tenant_discovery(http_request):
"""The policy's token requests should exclude the challenge's tenant if requested"""

def test_with_challenge(challenge, expected_scope):
Expand Down Expand Up @@ -397,7 +397,7 @@ def get_token(*scopes, **kwargs):


@pytest.mark.parametrize("http_request", HTTP_REQUESTS)
def test_multitenant_policy_disable_scopes_discovery(http_request):
def test_challenge_policy_disable_scopes_discovery(http_request):
"""The policy's token requests should exclude the challenge's scope if requested"""

def test_with_challenge(challenge, expected_scope, challenge_tenant):
Expand Down Expand Up @@ -457,7 +457,7 @@ def get_token(*scopes, **kwargs):


@pytest.mark.parametrize("http_request", HTTP_REQUESTS)
def test_multitenant_policy_disable_any_discovery(http_request):
def test_challenge_policy_disable_any_discovery(http_request):
"""The policy shouldn't respond to the challenge if it can't use the provided scope or tenant"""

def test_with_challenge(challenge, expected_scope, challenge_tenant):
Expand Down Expand Up @@ -508,6 +508,66 @@ def get_token(*scopes, **kwargs):
test_with_challenge(challenge_with_commas, scope, tenant)


@pytest.mark.parametrize("http_request", HTTP_REQUESTS)
def test_challenge_policy_no_scope_in_challenge(http_request):
"""The policy's token requests should use constructor scopes if none are in the challenge"""

def test_with_challenge(challenge, expected_scope, challenge_tenant):
bad_token = "bad_token"

class Requests:
count = 0

class TokenRequests:
count = 0

def send(request):
Requests.count += 1
if Requests.count == 1:
# first request triggers a 401 response w/ auth challenge
assert bad_token in request.headers["Authorization"]
return challenge
elif Requests.count == 2:
# second request should still be unauthorized because we didn't authenticate with the correct scope
assert bad_token in request.headers["Authorization"]
return challenge
raise ValueError("unexpected request")

def get_token(*scopes, **kwargs):
assert len(scopes) == 1
TokenRequests.count += 1
if TokenRequests.count == 1:
# first request uses the scope given to the policy, and no tenant ID
assert scopes[0] != expected_scope
assert kwargs.get("tenant_id") is None
return AccessToken(bad_token, 0)
elif TokenRequests.count == 2:
# second request should use the tenant specified in the auth challenge, and same scope as before
assert scopes[0] != expected_scope
assert kwargs.get("tenant_id") == challenge_tenant
return AccessToken(bad_token, 0)
raise ValueError("unexpected token request")

credential = Mock(get_token=Mock(wraps=get_token))
policy = BearerTokenChallengePolicy(credential, "scope")
pipeline = Pipeline(policies=[policy], transport=Mock(send=send))
pipeline.run(http_request("GET", "https://localhost"))

tenant = "tenant-id"
endpoint = f"https://authority.net/{tenant}/oauth2/authorize"
resource = "https://challenge.resource"
scope = f"{resource}/.default"

# this challenge has no `resource` or `scope` field
challenge_with_commas = Mock(
status_code=401,
headers={"WWW-Authenticate": f'Bearer authorization="{endpoint}"'},
)
# the request should fail after the challenge because we don't use the correct scope
# after the second 4xx response, the policy should raise the authentication error
test_with_challenge(challenge_with_commas, scope, tenant)


@pytest.mark.skipif(azure.core.__version__ >= "2", reason="this test applies only to azure-core 1.x")
@pytest.mark.parametrize("http_request", HTTP_REQUESTS)
def test_key_vault_regression(http_request):
Expand Down

0 comments on commit c409f0b

Please sign in to comment.