From 5b4e9c87983d76393195ac4d2af5379f7219053a Mon Sep 17 00:00:00 2001
From: Jon Wayne Parrott <jonwayne@google.com>
Date: Wed, 15 Feb 2017 16:44:00 -0800
Subject: [PATCH] Fix issue where GAE Signer erroneously returns a tuple from
 sign() (#109)

---
 google/auth/app_engine.py | 26 ++++++++++++++++++++++----
 tests/test_app_engine.py  | 25 ++++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/google/auth/app_engine.py b/google/auth/app_engine.py
index ec89dc0ea..8b17e76ce 100644
--- a/google/auth/app_engine.py
+++ b/google/auth/app_engine.py
@@ -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):
@@ -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():
diff --git a/tests/test_app_engine.py b/tests/test_app_engine.py
index 117533ebf..dd410d9d2 100644
--- a/tests/test_app_engine.py
+++ b/tests/test_app_engine.py
@@ -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:
@@ -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'