Skip to content

[Profile] Fix "missing secret or certificate in order to authenticate through a service principal" #16219

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 2 commits into from
Dec 24, 2020
Merged

[Profile] Fix "missing secret or certificate in order to authenticate through a service principal" #16219

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
fbab65a
Fix "missing secret or certificate"
jiasli Dec 10, 2020
a39d140
tests
jiasli Dec 10, 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
11 changes: 6 additions & 5 deletions src/azure-cli-core/azure/cli/core/_profile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ def get_refresh_token(self, resource=None,
username_or_sp_id, account[_TENANT_ID], resource)
return None, token_entry.get(_REFRESH_TOKEN), token_entry[_ACCESS_TOKEN], str(account[_TENANT_ID])

sp_secret = self._creds_cache.retrieve_secret_of_service_principal(username_or_sp_id)
sp_secret = self._creds_cache.retrieve_cred_for_service_principal(username_or_sp_id)
return username_or_sp_id, sp_secret, None, str(account[_TENANT_ID])

def get_raw_token(self, resource=None, subscription=None, tenant=None):
Expand Down Expand Up @@ -706,7 +706,7 @@ def refresh_accounts(self, subscription_finder=None):
subscriptions = []
try:
if is_service_principal:
sp_auth = ServicePrincipalAuth(self._creds_cache.retrieve_secret_of_service_principal(user_name))
sp_auth = ServicePrincipalAuth(self._creds_cache.retrieve_cred_for_service_principal(user_name))
subscriptions = subscription_finder.find_from_service_principal_id(user_name, sp_auth, tenant,
self._ad_resource_uri)
else:
Expand Down Expand Up @@ -752,7 +752,7 @@ def get_sp_auth_info(self, subscription_id=None, name=None, password=None, cert_
user_type = account[_USER_ENTITY].get(_USER_TYPE)
if user_type == _SERVICE_PRINCIPAL:
result['clientId'] = account[_USER_ENTITY][_USER_NAME]
sp_auth = ServicePrincipalAuth(self._creds_cache.retrieve_secret_of_service_principal(
sp_auth = ServicePrincipalAuth(self._creds_cache.retrieve_cred_for_service_principal(
account[_USER_ENTITY][_USER_NAME]))
secret = getattr(sp_auth, 'secret', None)
if secret:
Expand Down Expand Up @@ -1109,13 +1109,14 @@ def retrieve_token_for_service_principal(self, sp_id, resource, tenant, use_cert
token_entry = sp_auth.acquire_token(context, resource, sp_id)
return (token_entry[_TOKEN_ENTRY_TOKEN_TYPE], token_entry[_ACCESS_TOKEN], token_entry)

def retrieve_secret_of_service_principal(self, sp_id):
def retrieve_cred_for_service_principal(self, sp_id):
Copy link
Member Author

@jiasli jiasli Dec 10, 2020

Choose a reason for hiding this comment

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

Replace of with for to be in sync with retrieve_token_for_service_principal.

"""Returns the secret or certificate of the specified service principal."""
self.load_adal_token_cache()
matched = [x for x in self._service_principal_creds if sp_id == x[_SERVICE_PRINCIPAL_ID]]
if not matched:
raise CLIError("No matched service principal found")
cred = matched[0]
return cred.get(_ACCESS_TOKEN, None)
return cred.get(_ACCESS_TOKEN) or cred.get(_SERVICE_PRINCIPAL_CERT_FILE)

@property
def adal_token_cache(self):
Expand Down
26 changes: 17 additions & 9 deletions src/azure-cli-core/azure/cli/core/tests/test_profile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1525,7 +1525,7 @@ def test_refresh_accounts_one_user_account_one_sp_account(self, mock_auth_contex
mock_arm_client.tenants.list.return_value = [TenantStub(self.tenant_id)]
mock_arm_client.subscriptions.list.side_effect = deepcopy([[self.subscription1], [self.subscription2, sp_subscription1]])
finder = SubscriptionFinder(cli, lambda _, _1, _2: mock_auth_context, None, lambda _: mock_arm_client)
profile._creds_cache.retrieve_secret_of_service_principal = lambda _: 'verySecret'
profile._creds_cache.retrieve_cred_for_service_principal = lambda _: 'verySecret'
profile._creds_cache.flush_to_disk = lambda _: ''
# action
profile.refresh_accounts(finder)
Expand Down Expand Up @@ -1594,21 +1594,29 @@ def test_credscache_load_tokens_and_sp_creds_with_cert(self, mock_read_file):
self.assertEqual(creds_cache._service_principal_creds, [test_sp])

@mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True)
def test_credscache_retrieve_sp_secret_with_cert(self, mock_read_file):
def test_credscache_retrieve_sp_cred(self, mock_read_file):
cli = DummyCli()
test_sp = {
"servicePrincipalId": "myapp",
"servicePrincipalTenant": "mytenant",
"certificateFile": 'junkcert.pem'
}
mock_read_file.return_value = [test_sp]
test_cache = [
{
"servicePrincipalId": "myapp",
"servicePrincipalTenant": "mytenant",
"accessToken": "Secret"
},
{
"servicePrincipalId": "myapp2",
"servicePrincipalTenant": "mytenant",
"certificateFile": 'junkcert.pem'
}
]
mock_read_file.return_value = test_cache

# action
creds_cache = CredsCache(cli, async_persist=False)
creds_cache.load_adal_token_cache()

# assert
self.assertEqual(creds_cache.retrieve_secret_of_service_principal(test_sp['servicePrincipalId']), None)
self.assertEqual(creds_cache.retrieve_cred_for_service_principal('myapp'), 'Secret')
self.assertEqual(creds_cache.retrieve_cred_for_service_principal('myapp2'), 'junkcert.pem')

@mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True)
@mock.patch('os.fdopen', autospec=True)
Expand Down
26 changes: 17 additions & 9 deletions src/azure-cli-core/azure/cli/core/tests/test_profile_v2016_06_01.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1379,7 +1379,7 @@ def test_refresh_accounts_one_user_account_one_sp_account(self, mock_auth_contex
mock_arm_client.tenants.list.return_value = [TenantStub(self.tenant_id)]
mock_arm_client.subscriptions.list.side_effect = deepcopy([[self.subscription1], [self.subscription2, sp_subscription1]])
finder = SubscriptionFinder(cli, lambda _, _1, _2: mock_auth_context, None, lambda _: mock_arm_client)
profile._creds_cache.retrieve_secret_of_service_principal = lambda _: 'verySecret'
profile._creds_cache.retrieve_cred_for_service_principal = lambda _: 'verySecret'
profile._creds_cache.flush_to_disk = lambda _: ''
# action
profile.refresh_accounts(finder)
Expand Down Expand Up @@ -1448,21 +1448,29 @@ def test_credscache_load_tokens_and_sp_creds_with_cert(self, mock_read_file):
self.assertEqual(creds_cache._service_principal_creds, [test_sp])

@mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True)
def test_credscache_retrieve_sp_secret_with_cert(self, mock_read_file):
def test_credscache_retrieve_sp_cred(self, mock_read_file):
cli = DummyCli()
test_sp = {
"servicePrincipalId": "myapp",
"servicePrincipalTenant": "mytenant",
"certificateFile": 'junkcert.pem'
}
mock_read_file.return_value = [test_sp]
test_cache = [
{
"servicePrincipalId": "myapp",
"servicePrincipalTenant": "mytenant",
"accessToken": "Secret"
},
{
"servicePrincipalId": "myapp2",
"servicePrincipalTenant": "mytenant",
"certificateFile": 'junkcert.pem'
}
]
mock_read_file.return_value = test_cache

# action
creds_cache = CredsCache(cli, async_persist=False)
creds_cache.load_adal_token_cache()

# assert
self.assertEqual(creds_cache.retrieve_secret_of_service_principal(test_sp['servicePrincipalId']), None)
self.assertEqual(creds_cache.retrieve_cred_for_service_principal('myapp'), 'Secret')
self.assertEqual(creds_cache.retrieve_cred_for_service_principal('myapp2'), 'junkcert.pem')

@mock.patch('azure.cli.core._profile._load_tokens_from_file', autospec=True)
@mock.patch('os.fdopen', autospec=True)
Expand Down