Skip to content

Commit cc6fd00

Browse files
committed
Provide actionable messages for 2 dpapi errors
1 parent b674b6a commit cc6fd00

File tree

2 files changed

+42
-10
lines changed

2 files changed

+42
-10
lines changed

msal_extensions/persistence.py

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,19 +56,31 @@ def _auto_hash(input_string):
5656

5757

5858
# We do not aim to wrap every os-specific exception.
59-
# Here we define only the most common one,
60-
# otherwise caller would need to catch os-specific persistence exceptions.
61-
class PersistenceNotFound(IOError): # Use IOError rather than OSError as base,
59+
# Here we standardize only the most common ones,
60+
# otherwise caller would need to catch os-specific underlying exceptions.
61+
class PersistenceError(IOError): # Use IOError rather than OSError as base,
62+
"""The base exception for persistence."""
6263
# because historically an IOError was bubbled up and expected.
6364
# https://github.com/AzureAD/microsoft-authentication-extensions-for-python/blob/0.2.2/msal_extensions/token_cache.py#L38
6465
# Now we want to maintain backward compatibility even when using Python 2.x
6566
# It makes no difference in Python 3.3+ where IOError is an alias of OSError.
67+
def __init__(self, err_no=None, message=None, location=None): # pylint: disable=useless-super-delegation
68+
super(PersistenceError, self).__init__(err_no, message, location)
69+
70+
71+
class PersistenceNotFound(PersistenceError):
6672
"""This happens when attempting BasePersistence.load() on a non-existent persistence instance"""
6773
def __init__(self, err_no=None, message=None, location=None):
6874
super(PersistenceNotFound, self).__init__(
69-
err_no or errno.ENOENT,
70-
message or "Persistence not found",
71-
location)
75+
err_no=errno.ENOENT,
76+
message=message or "Persistence not found",
77+
location=location)
78+
79+
class PersistenceEncryptionError(PersistenceError):
80+
"""This could be raised by persistence.save()"""
81+
82+
class PersistenceDecryptionError(PersistenceError):
83+
"""This could be raised by persistence.load()"""
7284

7385

7486
class BasePersistence(ABC):
@@ -177,7 +189,12 @@ def __init__(self, location, entropy=''):
177189

178190
def save(self, content):
179191
# type: (str) -> None
180-
data = self._dp_agent.protect(content)
192+
try:
193+
data = self._dp_agent.protect(content)
194+
except OSError as exception:
195+
raise PersistenceEncryptionError(
196+
err_no=getattr(exception, "winerror", None), # Exists in Python 3 on Windows
197+
message="Unable to encrypt data. You may consider disable encryption.")
181198
with os.fdopen(_open(self._location), 'wb+') as handle:
182199
handle.write(data)
183200

@@ -186,7 +203,6 @@ def load(self):
186203
try:
187204
with open(self._location, 'rb') as handle:
188205
data = handle.read()
189-
return self._dp_agent.unprotect(data)
190206
except EnvironmentError as exp: # EnvironmentError in Py 2.7 works across platform
191207
if exp.errno == errno.ENOENT:
192208
raise PersistenceNotFound(
@@ -199,6 +215,14 @@ def load(self):
199215
"DPAPI error likely caused by file content not previously encrypted. "
200216
"App developer should migrate by calling save(plaintext) first.")
201217
raise
218+
try:
219+
return self._dp_agent.unprotect(data)
220+
except OSError as exception:
221+
raise PersistenceDecryptionError(
222+
err_no=getattr(exception, "winerror", None), # Exists in Python 3 on Windows
223+
message="Unable to decrypt data. You may have to delete the file.",
224+
location=self._location,
225+
)
202226

203227

204228
class KeychainPersistence(BasePersistence):

msal_extensions/windows.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,14 @@ def raw(self):
3939
_MEMCPY(blob_buffer, pb_data, cb_data)
4040
return blob_buffer.raw
4141

42+
_err_description = {
43+
# Keys came from real world observation, values came from winerror.h (https://errors)
44+
-2146893813: "Key not valid for use in specified state.",
45+
-2146892987: "The requested operation cannot be completed. "
46+
"The computer must be trusted for delegation and "
47+
"the current user account must be configured to allow delegation. "
48+
"See also https://docs.microsoft.com/en-us/windows/security/threat-protection/security-policy-settings/enable-computer-and-user-accounts-to-be-trusted-for-delegation",
49+
}
4250

4351
# This code is modeled from a StackOverflow question, which can be found here:
4452
# https://stackoverflow.com/questions/463832/using-dpapi-with-python
@@ -82,7 +90,7 @@ def protect(self, message):
8290
_LOCAL_FREE(result.pbData)
8391

8492
err_code = _GET_LAST_ERROR()
85-
raise OSError(256, '', '', err_code)
93+
raise OSError(None, _err_description.get(err_code), None, err_code)
8694

8795
def unprotect(self, cipher_text):
8896
# type: (bytes) -> str
@@ -111,4 +119,4 @@ def unprotect(self, cipher_text):
111119
finally:
112120
_LOCAL_FREE(result.pbData)
113121
err_code = _GET_LAST_ERROR()
114-
raise OSError(256, '', '', err_code)
122+
raise OSError(None, _err_description.get(err_code), None, err_code)

0 commit comments

Comments
 (0)