Skip to content

Commit

Permalink
creds share: do not persist several fields not used for token acquire…
Browse files Browse the repository at this point in the history
…/refresh (Azure#217)
  • Loading branch information
yugangw-msft committed May 6, 2016
1 parent f4017f5 commit d2aab1b
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 2 deletions.
29 changes: 28 additions & 1 deletion src/azure/cli/_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from codecs import open as codecs_open
import json
import os.path
import errno
from msrest.authentication import BasicTokenAuthentication
import adal
from azure.mgmt.resource.subscriptions import (SubscriptionClient,
Expand Down Expand Up @@ -31,10 +32,16 @@
_SERVICE_PRINCIPAL_ID = 'servicePrincipalId'
_SERVICE_PRINCIPAL_TENANT = 'servicePrincipalTenant'
_TOKEN_ENTRY_USER_ID = 'userId'
#This could mean real access token, or client secret of a service principal
#This could mean either real access token, or client secret of a service principal
#This naming is no good, but can't change because xplat-cli does so.
_ACCESS_TOKEN = 'accessToken'

TOKEN_FIELDS_EXCLUDED_FROM_PERSISTENCE = ['familyName',
'givenName',
'isUserIdDisplayable',
'tenantId']


_AUTH_CTX_FACTORY = lambda authority, cache: adal.AuthenticationContext(authority, cache=cache)

def _read_file_content(file_path):
Expand All @@ -44,6 +51,13 @@ def _read_file_content(file_path):
file_text = file_to_read.read()
return file_text

def _delete_file(file_path):
try:
os.remove(file_path)
except OSError as e:
if e.errno != errno.ENOENT:
raise

class Profile(object):
def __init__(self, storage=None, auth_ctx_factory=None):
self._storage = storage or ACCOUNT
Expand Down Expand Up @@ -165,6 +179,9 @@ def logout(self, user_or_sp):

self._creds_cache.remove_cached_creds(user_or_sp)

def logout_all(self):
self._cache_subscriptions_to_local_storage({})
self._creds_cache.remove_all_cached_creds()

def load_cached_subscriptions(self):
return self._storage.get(_SUBSCRIPTIONS) or []
Expand Down Expand Up @@ -283,6 +300,12 @@ def persist_cached_creds(self):
with codecs_open(self._token_file, 'w', encoding='ascii') as cred_file:
items = self.adal_token_cache.read_items()
all_creds = [entry for _, entry in items]

#trim away useless fields (needed for cred sharing with xplat)
for i in all_creds:
for key in TOKEN_FIELDS_EXCLUDED_FROM_PERSISTENCE:
i.pop(key, None)

all_creds.extend(self._service_principal_creds)
cred_file.write(json.dumps(all_creds))
self.adal_token_cache.has_state_changed = False
Expand Down Expand Up @@ -372,3 +395,7 @@ def remove_cached_creds(self, user_or_sp):

if state_changed:
self.persist_cached_creds()

def remove_all_cached_creds(self):
#we can clear file contents, but deleting it is simpler
_delete_file(self._token_file)
23 changes: 23 additions & 0 deletions src/azure/cli/tests/test_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,29 @@ def test_logout(self, mock_persist_creds, mock_read_cred_file):
self.assertEqual(mock_read_cred_file.call_count, 1)
self.assertEqual(mock_persist_creds.call_count, 1)

@mock.patch('azure.cli._profile._delete_file', autospec=True)
def test_logout_all(self, mock_delete_cred_file):
#setup
storage_mock = {'subscriptions': None}
profile = Profile(storage_mock)
consolidated = Profile._normalize_properties(self.user1,
[self.subscription1],
False,
ENV_DEFAULT)
consolidated2 = Profile._normalize_properties(self.user2,
[self.subscription2],
False,
ENV_DEFAULT)
profile._set_subscriptions(consolidated + consolidated2)

self.assertEqual(2, len(storage_mock['subscriptions']))
#action
profile.logout_all()

#verify
self.assertEqual(0, len(storage_mock['subscriptions']))
self.assertEqual(mock_delete_cred_file.call_count, 1)

@mock.patch('adal.AuthenticationContext', autospec=True)
def test_find_subscriptions_thru_username_password(self, mock_auth_context):
mock_auth_context.acquire_token_with_username_password.return_value = self.token_entry1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from azure.cli._profile import Profile
from azure.cli._profile import Profile
from azure.cli.commands import CommandTable
from azure.cli._locale import L
from .command_tables import COMMAND_TABLES
Expand Down Expand Up @@ -32,3 +32,11 @@ def set_active_subscription(args):

profile = Profile()
profile.set_active_subscription(subscription_name_or_id)

@command_table.command('account clear')
@command_table.description(L('Clear all stored subscriptions. '
'To clear individual, use "logout".'))
def clear(_):
profile = Profile()
profile.logout_all()

0 comments on commit d2aab1b

Please sign in to comment.