Skip to content

{Role} az ad sp create-for-rbac: Stop setting identifierUris on app #18312

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
jiasli merged 3 commits into from
Jun 7, 2021
Merged

{Role} az ad sp create-for-rbac: Stop setting identifierUris on app #18312

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
68e28e3
identifierUris
jiasli May 31, 2021
c0a02c4
hybrid_2018_03_01
jiasli Jun 2, 2021
b0629ac
Merge branch 'dev' into identifier-uri
jiasli Jun 7, 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
Prev Previous commit
Next Next commit
hybrid_2018_03_01
  • Loading branch information
@jiasli
jiasli committed Jun 2, 2021
commit c0a02c4e778ae1ce44b4333386a6bb6c46fc3bf2
457 changes: 311 additions & 146 deletions ...le/tests/hybrid_2018_03_01/recordings/test_create_for_rbac_with_cert_with_assignment.yaml
View file
Open in desktop

Large diffs are not rendered by default.

1,025 changes: 637 additions & 388 deletions ...s/role/tests/hybrid_2018_03_01/recordings/test_create_for_rbac_with_existing_kv_cert.yaml
View file
Open in desktop

Large diffs are not rendered by default.

712 changes: 395 additions & 317 deletions ...odules/role/tests/hybrid_2018_03_01/recordings/test_create_for_rbac_with_new_kv_cert.yaml
View file
Open in desktop

Large diffs are not rendered by default.

204 changes: 132 additions & 72 deletions ...le/tests/hybrid_2018_03_01/recordings/test_create_for_rbac_with_secret_no_assignment.yaml
View file
Open in desktop

Large diffs are not rendered by default.

1,295 changes: 936 additions & 359 deletions .../tests/hybrid_2018_03_01/recordings/test_create_for_rbac_with_secret_with_assignment.yaml
View file
Open in desktop

Large diffs are not rendered by default.

69 changes: 40 additions & 29 deletions src/azure-cli/azure/cli/command_modules/role/tests/hybrid_2018_03_01/test_role.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,13 @@ class RbacSPSecretScenarioTest(RoleScenarioTest):
@ResourceGroupPreparer(name_prefix='cli_create_rbac_sp_minimal')
def test_create_for_rbac_with_secret_no_assignment(self, resource_group):

self.kwargs['sp'] = 'http://{}'.format(resource_group)
self.kwargs['display_name'] = resource_group
try:
self.cmd('ad sp create-for-rbac -n {display_name} --skip-assignment', checks=self.check('name', '{sp}'))
result = self.cmd('ad sp create-for-rbac -n {display_name} --skip-assignment',
checks=self.check('displayName', '{display_name}')).get_output_in_json()
self.kwargs['app_id'] = result['appId']
finally:
self.cmd('ad app delete --id {sp}')
self.cmd('ad app delete --id {app_id}')

@AllowLargeResponse()
@ResourceGroupPreparer(name_prefix='cli_create_rbac_sp_with_password')
Expand All @@ -44,24 +45,24 @@ def test_create_for_rbac_with_secret_with_assignment(self, resource_group):
self.kwargs.update({
'sub': subscription_id,
'scope': '/subscriptions/{}'.format(subscription_id),
'sp': 'http://{}'.format(resource_group),
'display_name': resource_group
})

try:
with mock.patch('azure.cli.command_modules.role.custom._gen_guid', side_effect=self.create_guid):
self.cmd('ad sp create-for-rbac -n {display_name} --scopes {scope} {scope}/resourceGroups/{rg}',
checks=self.check('name', '{sp}'))
self.cmd('role assignment list --assignee {sp} --scope {scope}',
result = self.cmd('ad sp create-for-rbac -n {display_name} --scopes {scope} {scope}/resourceGroups/{rg}',
checks=self.check('displayName', '{display_name}')).get_output_in_json()
self.kwargs['app_id'] = result['appId']
self.cmd('role assignment list --assignee {app_id} --scope {scope}',
checks=self.check("length([])", 1))
self.cmd('role assignment list --assignee {sp} -g {rg}',
self.cmd('role assignment list --assignee {app_id} -g {rg}',
checks=self.check("length([])", 1))
self.cmd('role assignment delete --assignee {sp} -g {rg}',
self.cmd('role assignment delete --assignee {app_id} -g {rg}',
checks=self.is_empty())
self.cmd('role assignment delete --assignee {sp}',
self.cmd('role assignment delete --assignee {app_id}',
checks=self.is_empty())
finally:
self.cmd('ad app delete --id {sp}')
self.cmd('ad app delete --id {app_id}')


class RbacSPCertScenarioTest(RoleScenarioTest):
Expand All @@ -72,25 +73,25 @@ def test_create_for_rbac_with_cert_with_assignment(self, resource_group):
self.kwargs.update({
'sub': subscription_id,
'scope': '/subscriptions/{}'.format(subscription_id),
'sp': 'http://' + resource_group,
'display_name': resource_group
})

try:
with mock.patch('azure.cli.command_modules.role.custom._gen_guid', side_effect=self.create_guid):
result = self.cmd('ad sp create-for-rbac -n {display_name} --scopes {scope} {scope}/resourceGroups/{rg} --create-cert',
checks=self.check('name', '{sp}')).get_output_in_json()
checks=self.check('displayName', '{display_name}')).get_output_in_json()
self.kwargs['app_id'] = result['appId']

self.assertTrue(result['fileWithCertAndPrivateKey'].endswith('.pem'))
os.remove(result['fileWithCertAndPrivateKey'])
result = self.cmd('ad sp credential reset -n {sp} --create-cert',
checks=self.check('name', '{sp}')).get_output_in_json()
result = self.cmd('ad sp credential reset -n {app_id} --create-cert',
checks=self.check('name', '{app_id}')).get_output_in_json()
self.assertTrue(result['fileWithCertAndPrivateKey'].endswith('.pem'))
os.remove(result['fileWithCertAndPrivateKey'])
finally:
self.cmd('ad app delete --id {sp}',
self.cmd('ad app delete --id {app_id}',
checks=self.is_empty())


class RbacSPKeyVaultScenarioTest2(ScenarioTest):
@ResourceGroupPreparer(name_prefix='cli_test_sp_with_kv_new_cert')
@KeyVaultPreparer(name_prefix='test-rbac-new-kv')
Expand All @@ -99,7 +100,6 @@ def test_create_for_rbac_with_new_kv_cert(self, resource_group, key_vault):
subscription_id = self.get_subscription_id()

self.kwargs.update({
'sp': 'http://{}'.format(resource_group),
'display_name': resource_group,
'sub': subscription_id,
'scope': '/subscriptions/{}'.format(subscription_id),
Expand All @@ -112,18 +112,20 @@ def test_create_for_rbac_with_new_kv_cert(self, resource_group, key_vault):
try:
with mock.patch('azure.cli.command_modules.role.custom._gen_guid', side_effect=self.create_guid):
try:
self.cmd('ad sp create-for-rbac --scopes {scope}/resourceGroups/{rg} --create-cert --keyvault {kv} --cert {cert} -n {display_name}')
result = self.cmd('ad sp create-for-rbac --scopes {scope}/resourceGroups/{rg} --create-cert '
'--keyvault {kv} --cert {cert} -n {display_name}').get_output_in_json()
self.kwargs['app_id'] = result['appId']
except KeyVaultErrorException:
if not self.is_live and not self.in_recording:
pass # temporary workaround for keyvault challenge handling was ignored under playback
else:
raise
cer1 = self.cmd('keyvault certificate show --vault-name {kv} -n {cert}').get_output_in_json()['cer']
self.cmd('ad sp credential reset -n {sp} --create-cert --keyvault {kv} --cert {cert}')
self.cmd('ad sp credential reset -n {app_id} --create-cert --keyvault {kv} --cert {cert}')
cer2 = self.cmd('keyvault certificate show --vault-name {kv} -n {cert}').get_output_in_json()['cer']
self.assertTrue(cer1 != cer2)
finally:
self.cmd('ad app delete --id {sp}')
self.cmd('ad app delete --id {app_id}')


class RbacSPKeyVaultScenarioTest(ScenarioTest):
Expand All @@ -135,8 +137,6 @@ def test_create_for_rbac_with_existing_kv_cert(self, resource_group, key_vault):
subscription_id = self.get_subscription_id()

self.kwargs.update({
'sp': 'http://{}'.format(resource_group),
'sp2': 'http://{}2'.format(resource_group),
'display_name': resource_group,
'display_name2': resource_group + '2',
'sub': subscription_id,
Expand All @@ -151,19 +151,30 @@ def test_create_for_rbac_with_existing_kv_cert(self, resource_group, key_vault):
self.kwargs['policy'] = self.cmd('keyvault certificate get-default-policy').get_output_in_json()
self.cmd('keyvault certificate create --vault-name {kv} -n {cert} -p "{policy}" --validity 24')
with mock.patch('azure.cli.command_modules.role.custom._gen_guid', side_effect=self.create_guid):
self.cmd('ad sp create-for-rbac -n {display_name} --keyvault {kv} --cert {cert} --scopes {scope}/resourceGroups/{rg}')
self.cmd('ad sp credential reset -n {sp} --keyvault {kv} --cert {cert}')
result = self.cmd('ad sp create-for-rbac -n {display_name} --keyvault {kv} '
'--cert {cert} --scopes {scope}/resourceGroups/{rg}').get_output_in_json()
self.kwargs['app_id'] = result['appId']
self.cmd('ad sp credential reset -n {app_id} --keyvault {kv} --cert {cert}')
finally:
self.cmd('ad app delete --id {sp}')
try:
self.cmd('ad app delete --id {app_id}')
except:
# Mute the exception, otherwise the exception thrown in the `try` clause will be hidden
pass

# test with cert that has too short a validity
try:
self.cmd('keyvault certificate create --vault-name {kv} -n {cert} -p "{policy}" --validity 6')
with mock.patch('azure.cli.command_modules.role.custom._gen_guid', side_effect=self.create_guid):
self.cmd('ad sp create-for-rbac --scopes {scope}/resourceGroups/{rg} --keyvault {kv} --cert {cert} -n {display_name2}')
self.cmd('ad sp credential reset -n {sp2} --keyvault {kv} --cert {cert}')
result = self.cmd('ad sp create-for-rbac --scopes {scope}/resourceGroups/{rg} --keyvault {kv} '
'--cert {cert} -n {display_name2}').get_output_in_json()
self.kwargs['app_id2'] = result['appId']
self.cmd('ad sp credential reset -n {app_id2} --keyvault {kv} --cert {cert}')
finally:
self.cmd('ad app delete --id {sp2}')
try:
self.cmd('ad app delete --id {app_id2}')
except:
pass


class RoleCreateScenarioTest(RoleScenarioTest):
Expand Down
Loading