Skip to content

Commit

Permalink
Fix issue where GAE Signer erroneously returns a tuple from sign() (#109
Browse files Browse the repository at this point in the history
)
  • Loading branch information
Jon Wayne Parrott committed Feb 16, 2017
1 parent 924191c commit 5b4e9c8
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 5 deletions.
26 changes: 22 additions & 4 deletions google/auth/app_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,30 @@


class Signer(object):
"""Signs messages using the App Engine app identity service.
"""Signs messages using the App Engine App Identity service.
This can be used in place of :class:`google.auth.crypt.Signer` when
running in the App Engine standard environment.
.. warning::
The App Identity service signs bytes using Google-managed keys.
Because of this it's possible that the key used to sign bytes will
change. In some cases this change can occur between successive calls
to :attr:`key_id` and :meth:`sign`. This could result in a signature
that was signed with a different key than the one indicated by
:attr:`key_id`. It's recommended that if you use this in your code
that you account for this behavior by building in retry logic.
"""
def __init__(self):
self.key_id = None

@property
def key_id(self):
"""Optional[str]: The key ID used to identify this private key.
.. note::
This makes a request to the App Identity service.
"""
key_id, _ = app_identity.sign_blob(b'')
return key_id

@staticmethod
def sign(message):
Expand All @@ -53,7 +70,8 @@ def sign(message):
bytes: The signature of the message.
"""
message = _helpers.to_bytes(message)
return app_identity.sign_blob(message)
_, signature = app_identity.sign_blob(message)
return signature


def get_project_id():
Expand Down
25 changes: 24 additions & 1 deletion tests/test_app_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,28 @@ def test_get_project_id_missing_apis():
assert excinfo.match(r'App Engine APIs are not available')


class TestSigner(object):
def test_key_id(self, app_identity_mock):
app_identity_mock.sign_blob.return_value = (
mock.sentinel.key_id, mock.sentinel.signature)

signer = app_engine.Signer()

assert signer.key_id == mock.sentinel.key_id

def test_sign(self, app_identity_mock):
app_identity_mock.sign_blob.return_value = (
mock.sentinel.key_id, mock.sentinel.signature)

signer = app_engine.Signer()
to_sign = b'123'

signature = signer.sign(to_sign)

assert signature == mock.sentinel.signature
app_identity_mock.sign_blob.assert_called_with(to_sign)


class TestCredentials(object):
def test_missing_apis(self):
with pytest.raises(EnvironmentError) as excinfo:
Expand Down Expand Up @@ -107,7 +129,8 @@ def test_refresh(self, now_mock, app_identity_mock):
assert not credentials.expired

def test_sign_bytes(self, app_identity_mock):
app_identity_mock.sign_blob.return_value = mock.sentinel.signature
app_identity_mock.sign_blob.return_value = (
mock.sentinel.key_id, mock.sentinel.signature)
credentials = app_engine.Credentials()
to_sign = b'123'

Expand Down

0 comments on commit 5b4e9c8

Please sign in to comment.