Merge branch 'master' into 90-oauth-support

CHANGELOG.md

@@ -4,6 +4,8 @@ Change Log
 HEAD
 ----
 -   Added support for OAuth 2.0 authentication
+-   Fixed a bug where version 2.x could not open autodiscover cache files generated by
+    version 1.x packages.
 
 
 2.0.0

exchangelib/autodiscover.py

@@ -33,7 +33,7 @@
 from .credentials import BaseCredentials
 from .errors import AutoDiscoverFailed, AutoDiscoverRedirect, AutoDiscoverCircularRedirect, TransportError, \
     RedirectError, ErrorNonExistentMailbox, UnauthorizedError
-from .protocol import BaseProtocol, Protocol, RetryPolicy, FailFast
+from .protocol import BaseProtocol, Protocol, RetryPolicy
 from .transport import DEFAULT_ENCODING, DEFAULT_HEADERS
 from .util import create_element, get_xml_attr, add_xml_child, to_xml, is_xml, post_ratelimited, xml_to_str, \
     get_domain, CONNECTION_ERRORS, TLS_ERRORS
@@ -48,6 +48,9 @@
 
 
 def shelve_filename():
+    # Add the version of the cache format to the filename. If we change the format of the cached data, this version
+    # must be bumped. Otherwise, new versions of this package cannot open cache files generated by older versions.
+    version = 2
     # 'shelve' may pickle objects using different pickle protocol versions. Append the python major+minor version
     # numbers to the filename. Also append the username, to avoid permission errors.
     major, minor = sys.version_info[:2]
@@ -56,7 +59,9 @@ def shelve_filename():
     except KeyError:
         # getuser() fails on some systems. Provide a sane default. See issue #448
         user = 'exchangelib'
-    return 'exchangelib.cache.{user}.py{major}{minor}'.format(user=user, major=major, minor=minor)
+    return 'exchangelib.{version}.cache.{user}.py{major}{minor}'.format(
+        version=version, user=user, major=major, minor=minor
+    )
 
 
 AUTODISCOVER_PERSISTENT_STORAGE = os.path.join(tempfile.gettempdir(), shelve_filename())
@@ -130,6 +135,7 @@ def __setitem__(self, key, protocol):
         # Populate both local and persistent cache
         domain = key[0]
         with shelve_open_with_failover(self._storage_file) as db:
+            # Don't change this payload without bumping the cache file version in shelve_filename()
             db[str(domain)] = (protocol.service_endpoint, protocol.auth_type, protocol.retry_policy)
         self._protocols[key] = protocol
 
@@ -182,12 +188,9 @@ def discover(email, credentials=None, auth_type=None, retry_policy=None):
     and return a hopefully-cached Protocol to the callee.
     """
     log.debug('Attempting autodiscover on email %s', email)
-    if credentials is not None:
-        if not isinstance(credentials, BaseCredentials):
-            raise ValueError("'credentials' %r must be a Credentials instance" % credentials)
-    if retry_policy is None:
-        retry_policy = FailFast()
-    if not isinstance(retry_policy, RetryPolicy):
+    if not isinstance(credentials, (BaseCredentials, type(None))):
+        raise ValueError("'credentials' %r must be a Credentials instance" % credentials)
+    if not isinstance(retry_policy, (RetryPolicy, type(None))):
         raise ValueError("'retry_policy' %r must be a RetryPolicy instance" % retry_policy)
     domain = get_domain(email)
     # We may be using multiple different credentials and changing our minds on TLS verification. This key combination
@@ -202,6 +205,11 @@ def discover(email, credentials=None, auth_type=None, retry_policy=None):
             protocol = _autodiscover_cache[autodiscover_key]
             if not isinstance(protocol, AutodiscoverProtocol):
                 raise ValueError('Unexpected autodiscover cache contents: %s' % protocol)
+            # Reset auth type and retry policy if we requested non-default values
+            if auth_type:
+                protocol.config.auth_type = auth_type
+            if retry_policy:
+                protocol.config.retry_policy = retry_policy
             log.debug('Cache hit for domain %s credentials %s: %s', domain, credentials, protocol.service_endpoint)
             try:
                 # This is the main path when the cache is primed
@@ -221,7 +229,7 @@ def discover(email, credentials=None, auth_type=None, retry_policy=None):
             try:
                 # This eventually fills the cache in _autodiscover_hostname
                 return _try_autodiscover(hostname=domain, credentials=credentials, email=email,
-                                         retry_policy=retry_policy, auth_type=auth_type)
+                                         auth_type=auth_type, retry_policy=retry_policy)
             except AutoDiscoverRedirect as e:
                 if email.lower() == e.redirect_email.lower():
                     raise_from(AutoDiscoverCircularRedirect('Redirect to same email address: %s' % email), None)
@@ -234,47 +242,47 @@ def discover(email, credentials=None, auth_type=None, retry_policy=None):
     return discover(email=email, credentials=credentials, auth_type=auth_type, retry_policy=retry_policy)
 
 
-def _try_autodiscover(hostname, credentials, email, retry_policy, auth_type):
+def _try_autodiscover(hostname, credentials, email, auth_type, retry_policy):
     # Implements the full chain of autodiscover server discovery attempts. Tries to return autodiscover data from the
     # final host.
     try:
         return _autodiscover_hostname(hostname=hostname, credentials=credentials, email=email, has_ssl=True,
-                                      retry_policy=retry_policy, auth_type=auth_type)
+                                      auth_type=auth_type, retry_policy=retry_policy)
     except RedirectError as e:
         if not e.has_ssl:
             raise_from(AutoDiscoverFailed(
                 '%s redirected us to %s but only HTTPS redirects allowed' % (hostname, e.url)
             ), None)
         log.info('%s redirected us to %s', hostname, e.server)
-        return _try_autodiscover(hostname=e.server, credentials=credentials, email=email, retry_policy=retry_policy,
-                                 auth_type=auth_type)
+        return _try_autodiscover(hostname=e.server, credentials=credentials, email=email, auth_type=auth_type,
+                                 retry_policy=retry_policy)
     except AutoDiscoverFailed as e:
         log.info('Autodiscover on %s failed (%s). Trying autodiscover.%s', hostname, e, hostname)
         try:
             return _autodiscover_hostname(hostname='autodiscover.%s' % hostname, credentials=credentials, email=email,
-                                          has_ssl=True, retry_policy=retry_policy, auth_type=auth_type)
+                                          has_ssl=True, auth_type=auth_type, retry_policy=retry_policy)
         except RedirectError as e:
             if not e.has_ssl:
                 raise_from(AutoDiscoverFailed(
                     'autodiscover.%s redirected us to %s but only HTTPS redirects allowed' % (hostname, e.url)
                 ), None)
             log.info('%s redirected us to %s', hostname, e.server)
             return _try_autodiscover(hostname=e.server, credentials=credentials, email=email,
-                                     retry_policy=retry_policy, auth_type=auth_type)
+                                     auth_type=auth_type, retry_policy=retry_policy)
         except AutoDiscoverFailed:
             log.info('Autodiscover on %s failed (%s). Trying autodiscover.%s (plain HTTP)', hostname, e, hostname)
             try:
                 return _autodiscover_hostname(hostname='autodiscover.%s' % hostname, credentials=credentials,
-                                              email=email, has_ssl=False, retry_policy=retry_policy,
-                                              auth_type=auth_type)
+                                              email=email, has_ssl=False, auth_type=auth_type,
+                                              retry_policy=retry_policy)
             except RedirectError as e:
                 if not e.has_ssl:
                     raise_from(AutoDiscoverFailed(
                         'autodiscover.%s redirected us to %s but only HTTPS redirects allowed' % (hostname, e.url)
                     ), None)
                 log.info('autodiscover.%s redirected us to %s', hostname, e.server)
                 return _try_autodiscover(hostname=e.server, credentials=credentials, email=email,
-                                         retry_policy=retry_policy, auth_type=auth_type)
+                                         auth_type=auth_type, retry_policy=retry_policy)
             except AutoDiscoverFailed as e:
                 log.info('Autodiscover on autodiscover.%s (no TLS) failed (%s). Trying DNS records', hostname, e)
                 hostname_from_dns = _get_canonical_name(hostname='autodiscover.%s' % hostname)
@@ -284,15 +292,15 @@ def _try_autodiscover(hostname, credentials, email, retry_policy, auth_type):
                         hostname_from_dns = _get_hostname_from_srv(hostname='autodiscover.%s' % hostname)
                     # Start over with new hostname
                     return _try_autodiscover(hostname=hostname_from_dns, credentials=credentials, email=email,
-                                             retry_policy=retry_policy, auth_type=auth_type)
+                                             auth_type=auth_type, retry_policy=retry_policy)
                 except AutoDiscoverFailed as e:
                     log.info('Autodiscover on %s failed (%s). Trying _autodiscover._tcp.%s', hostname_from_dns, e,
                              hostname)
                     # Start over with new hostname
                     try:
                         hostname_from_dns = _get_hostname_from_srv(hostname='_autodiscover._tcp.%s' % hostname)
                         return _try_autodiscover(hostname=hostname_from_dns, credentials=credentials, email=email,
-                                                 retry_policy=retry_policy, auth_type=auth_type)
+                                                 auth_type=auth_type, retry_policy=retry_policy)
                     except AutoDiscoverFailed:
                         raise_from(AutoDiscoverFailed('All steps in the autodiscover protocol failed'), None)
 
@@ -320,7 +328,7 @@ def _get_auth_type_or_raise(url, email, hostname):
         raise_from(RedirectError(url='%s://%s' % ('https' if redirect_has_ssl else 'http', redirect_hostname)), None)
 
 
-def _autodiscover_hostname(hostname, credentials, email, has_ssl, retry_policy, auth_type):
+def _autodiscover_hostname(hostname, credentials, email, has_ssl, auth_type, retry_policy):
     # Tries to get autodiscover data on a specific host. If we are HTTP redirected, we restart the autodiscover dance on
     # the new host.
     url = '%s://%s/Autodiscover/Autodiscover.xml' % ('https' if has_ssl else 'http', hostname)