Skip to content

[KeyVault] Fix #11871: AKV10032: Invalid issuer error for operations in nondefault tenant/subscription #18162

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
evelyn-ys merged 6 commits into from
May 26, 2021
Merged

[KeyVault] Fix #11871: AKV10032: Invalid issuer error for operations in nondefault tenant/subscription #18162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
be9e57e
keyvault data plane cross tenant
evelyn-ys May 20, 2021
686d740
all data plane
evelyn-ys May 21, 2021
558b9e7
cli_ctx for client factory
evelyn-ys May 24, 2021
6fc0781
other modules
evelyn-ys May 25, 2021
4d554c4
remove unused import
evelyn-ys May 25, 2021
f862d04
omit extra `None`
evelyn-ys May 25, 2021
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 src/azure-cli/azure/cli/command_modules/appconfig/_kv_helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ def __read_kv_from_config_store(azconfig_client,

if cli_ctx:
from azure.cli.command_modules.keyvault._client_factory import keyvault_data_plane_factory
keyvault_client = keyvault_data_plane_factory(cli_ctx, None)
keyvault_client = keyvault_data_plane_factory(cli_ctx)
else:
keyvault_client = None

Expand Down
8 changes: 5 additions & 3 deletions src/azure-cli/azure/cli/command_modules/keyvault/_client_factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def _keyvault_mgmt_client_factory(cli_ctx, _):
return _keyvault_mgmt_client_factory


def keyvault_data_plane_factory(cli_ctx, _):
def keyvault_data_plane_factory(cli_ctx, *_):
Copy link
Member

@qwordy qwordy May 26, 2021

Choose a reason for hiding this comment

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

If *_ is ignored, why it is here?

Copy link
Member Author

@evelyn-ys evelyn-ys May 26, 2021

Choose a reason for hiding this comment

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

When creating from keyvault's command handler, a command_args will be passed in so we can't delete *_ directly. See L100

azure-cli/src/azure-cli/azure/cli/command_modules/keyvault/_command_type.py

Lines 86 to 138 in c3b56d8

def keyvault_command_handler(command_args):
from azure.cli.core.commands.client_factory import resolve_client_arg_name
from azure.cli.core.profiles import ResourceType
from azure.cli.core.util import get_arg_list, poller_classes
from msrest.paging import Paged
op = get_op_handler()
op_args = get_arg_list(op)
command_type = merged_kwargs.get('command_type', None)
client_factory = command_type.settings.get('client_factory', None) if command_type \
else merged_kwargs.get('client_factory', None)
client_arg_name = resolve_client_arg_name(operations_tmpl.format(method_name), kwargs)
if client_arg_name in op_args:
client = client_factory(self.command_loader.cli_ctx, command_args)
command_args[client_arg_name] = client
if 'cmd' not in op_args:
command_args.pop('cmd')
try:
if command_type.settings.get('resource_type') == ResourceType.DATA_KEYVAULT_ADMINISTRATION_BACKUP:
abandoned_args = ['identifier', 'vault_base_url']
for arg in abandoned_args:
if arg in command_args:
command_args.pop(arg)
result = op(**command_args)
# apply results transform if specified
transform_result = merged_kwargs.get('transform', None)
if transform_result:
return _encode_hex(transform_result(result, **{**command_args, 'kv_transform': True}))
# otherwise handle based on return type of results
if isinstance(result, poller_classes()):
return _encode_hex(
LongRunningOperation(self.command_loader.cli_ctx, 'Starting {}'.format(name))(result))
if isinstance(result, Paged):
try:
return _encode_hex(list(result))
except TypeError:
# TODO: Workaround for an issue in either KeyVault server-side or msrest
# See https://github.com/Azure/autorest/issues/1309
return []
return _encode_hex(result)
except Exception as ex: # pylint: disable=broad-except
if name == 'show':
# show_exception_handler needs to be called before the keyvault_exception_handler
from azure.cli.core.commands.arm import show_exception_handler
try:
show_exception_handler(ex)
except Exception: # pylint: disable=broad-except
pass
return keyvault_exception_handler(self.command_loader, ex)

from azure.keyvault import KeyVaultAuthentication, KeyVaultClient
from azure.cli.core.util import should_disable_connection_verify

Expand All @@ -134,7 +134,8 @@ def keyvault_data_plane_factory(cli_ctx, _):
def get_token(server, resource, scope): # pylint: disable=unused-argument
import adal
try:
return Profile(cli_ctx=cli_ctx).get_raw_token(resource)[0]
return Profile(cli_ctx=cli_ctx).get_raw_token(resource=resource,
subscription=cli_ctx.data.get('subscription_id'))[0]
Copy link
Member

@qwordy qwordy May 26, 2021

Choose a reason for hiding this comment

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

[0] is magic code.

Copy link
Member Author

@evelyn-ys evelyn-ys May 26, 2021

Choose a reason for hiding this comment

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

I just keep it as it is

except adal.AdalError as err:
# pylint: disable=no-member
if (hasattr(err, 'error_response') and
Expand Down Expand Up @@ -170,7 +171,8 @@ def keyvault_private_data_plane_factory_v7_2_preview(cli_ctx, _):
def get_token(server, resource, scope): # pylint: disable=unused-argument
import adal
try:
return Profile(cli_ctx=cli_ctx).get_raw_token(resource)[0]
return Profile(cli_ctx=cli_ctx).get_raw_token(resource=resource,
subscription=cli_ctx.data.get('subscription_id'))[0]
Copy link
Member

@qwordy qwordy May 26, 2021

Choose a reason for hiding this comment

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

cli_ctx.data.get('subscription_id'))[0] appears multiple times. Can we extract it to a method? String literal 'subscription_id' is error prone. [0] is magic number.

except adal.AdalError as err:
# pylint: disable=no-member
if (hasattr(err, 'error_response') and
Expand Down
2 changes: 1 addition & 1 deletion src/azure-cli/azure/cli/command_modules/keyvault/_command_type.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def keyvault_command_handler(command_args):

client_arg_name = resolve_client_arg_name(operations_tmpl.format(method_name), kwargs)
if client_arg_name in op_args:
client = client_factory(self.command_loader.cli_ctx, command_args)
client = client_factory(command_args['cmd'].cli_ctx, command_args)
command_args[client_arg_name] = client
if 'cmd' not in op_args:
command_args.pop('cmd')
Expand Down
12 changes: 3 additions & 9 deletions src/azure-cli/azure/cli/command_modules/role/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from knack.log import get_logger
from knack.util import CLIError, todict

from azure.cli.core.profiles import ResourceType, get_api_version
from azure.cli.core.profiles import ResourceType
from azure.graphrbac.models import GraphErrorException

from azure.cli.core.util import get_file_json, shell_safe_json_parse, is_guid
Expand Down Expand Up @@ -1538,14 +1538,8 @@ def _get_signed_in_user_object_id(graph_client):


def _get_keyvault_client(cli_ctx):
from azure.cli.core._profile import Profile
from azure.keyvault import KeyVaultAuthentication, KeyVaultClient
version = str(get_api_version(cli_ctx, ResourceType.DATA_KEYVAULT))

def _get_token(server, resource, scope): # pylint: disable=unused-argument
return Profile(cli_ctx=cli_ctx).get_raw_token(resource)[0]

return KeyVaultClient(KeyVaultAuthentication(_get_token), api_version=version)
from azure.cli.command_modules.keyvault._client_factory import keyvault_data_plane_factory
return keyvault_data_plane_factory(cli_ctx)


def _create_self_signed_cert(start_date, end_date): # pylint: disable=too-many-locals
Expand Down
13 changes: 3 additions & 10 deletions src/azure-cli/azure/cli/command_modules/servicefabric/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@
from azure.cli.core.util import CLIError, get_file_json, b64_to_hex, sdk_no_wait
from azure.cli.core.commands import LongRunningOperation
from azure.graphrbac import GraphRbacManagementClient
from azure.cli.core.profiles import ResourceType, get_sdk, get_api_version
from azure.keyvault import KeyVaultAuthentication, KeyVaultClient
from azure.cli.core.profiles import ResourceType, get_sdk
from azure.cli.command_modules.servicefabric._arm_deployment_utils import validate_and_deploy_arm_template
from azure.cli.command_modules.servicefabric._sf_utils import _get_resource_group_by_name, _create_resource_group_name

Expand Down Expand Up @@ -1598,14 +1597,8 @@ def _create_self_signed_key_vault_certificate(cli_ctx, vault_base_url, certifica


def _get_keyVault_not_arm_client(cli_ctx):
from azure.cli.core._profile import Profile
version = str(get_api_version(cli_ctx, ResourceType.DATA_KEYVAULT))

def get_token(server, resource, scope): # pylint: disable=unused-argument
return Profile(cli_ctx=cli_ctx).get_raw_token(resource)[0]

client = KeyVaultClient(KeyVaultAuthentication(get_token), api_version=version)
return client
from azure.cli.command_modules.keyvault._client_factory import keyvault_data_plane_factory
return keyvault_data_plane_factory(cli_ctx)


def _create_keyvault(cmd,
Expand Down
11 changes: 2 additions & 9 deletions src/azure-cli/azure/cli/command_modules/vm/_vm_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,8 @@ def check_existence(cli_ctx, value, resource_group, provider_namespace, resource


def create_keyvault_data_plane_client(cli_ctx):
from azure.cli.core._profile import Profile
from azure.cli.core.profiles import get_api_version, ResourceType
version = str(get_api_version(cli_ctx, ResourceType.DATA_KEYVAULT))

def get_token(server, resource, scope): # pylint: disable=unused-argument
return Profile(cli_ctx=cli_ctx).get_raw_token(resource)[0]

from azure.keyvault import KeyVaultAuthentication, KeyVaultClient
return KeyVaultClient(KeyVaultAuthentication(get_token), api_version=version)
from azure.cli.command_modules.keyvault._client_factory import keyvault_data_plane_factory
return keyvault_data_plane_factory(cli_ctx)


def get_key_vault_base_url(cli_ctx, vault_name):
Expand Down