feat: add `to_json` method to google.oauth2.credentials.Credentials #367

ghost · 2019-08-28T14:21:57Z

This adds a Storage base interface class and a file module based on Storage to save and load credentials.
The credentials module was modified to rely on the newer Storage class while retaining compatibility.

busunkim96 · 2019-08-28T23:19:40Z

Hi @patkasper - apologies for it taking a long time to comment on this.

I brought up credentials storage in a weekly meeting and it looks like folks are generally opposed to the idea of this library handling credentials storage. It may be convenient to store credentials in memory or in a file, but it isn't great from a security perspective.

There was interest in adding a method like to_json to make it easier to convert a credentials object back into JSON. Could you perhaps rework this PR to add just that method?

ghost · 2019-08-30T16:24:35Z

I've removed the storage interface and I've kept the to_json method in google.oauth2's Credentials. Please let me know what you think.

I'm happy to add the storage interface + automated loading/storing to google-auth-oauthlib, should I go ahead with that?

Thanks.

busunkim96 · 2019-09-10T23:54:40Z

google/auth/_helpers.py

    return base64.urlsafe_b64encode(value).rstrip(b'=')
+
+
+def validate_file(filename):


I don't think this function is used anymore?

Indeed, that was a remnant from my earlier additions. Removing.

busunkim96 · 2019-09-10T23:56:29Z

google/oauth2/credentials.py

            data = json.load(json_file)
            return cls.from_authorized_user_info(data, scopes)
+
+    def to_json(self, strip=None, indent=None):


I'd prefer to remove indent. If the user would like to do more formatting they can do it with the return value from this function.

Ok, no problem.

busunkim96 · 2019-09-11T00:15:46Z

google/oauth2/credentials.py

        Args:
            info (Mapping[str, str]): The authorized user info in Google
-                format.
+                                      format.


Did you mean to change the indent here?

Funny, there are a few occurrences where the indent is not what I had locally. Not sure what happened.

Would you mind reverting these whitespace changes?

busunkim96 · 2019-09-11T00:45:24Z

google/auth/_helpers.py

+        warnings.warn(_MISSING_FILE_MESSAGE.format(filename))
+
+
+def parse_expiry(expiry):


Where is this function used?

google/oauth2/credentials.py

ghost

Changes coming!

ghost · 2019-09-16T12:18:50Z

google/auth/_helpers.py

    return base64.urlsafe_b64encode(value).rstrip(b'=')
+
+
+def validate_file(filename):


Indeed, that was a remnant from my earlier additions. Removing.

docs/reference/google.oauth2.rst

docs/reference/google.oauth2.file.rst

google/oauth2/credentials.py

ghost · 2019-09-16T12:23:54Z

google/oauth2/credentials.py

        Args:
            info (Mapping[str, str]): The authorized user info in Google
-                format.
+                                      format.


Funny, there are a few occurrences where the indent is not what I had locally. Not sure what happened.

google/oauth2/credentials.py

ghost · 2019-09-16T12:25:43Z

google/oauth2/credentials.py

            data = json.load(json_file)
            return cls.from_authorized_user_info(data, scopes)
+
+    def to_json(self, strip=None, indent=None):


Ok, no problem.

google/oauth2/credentials.py

busunkim96 · 2019-09-30T15:33:37Z

google/oauth2/credentials.py

        Args:
            info (Mapping[str, str]): The authorized user info in Google
-                format.
+                                      format.


Would you mind reverting these whitespace changes?

busunkim96 · 2019-09-30T15:34:30Z

google/oauth2/credentials.py

+        Args:
+            strip (Sequence[str]): Optional list of members to exclude from the
+                                   generated JSON.
+            indent (int): This parameter is passed to `json.dumps` to create


Please remove the documentation for this arg

busunkim96 · 2019-09-30T15:35:42Z

google/oauth2/credentials.py

            return cls.from_authorized_user_info(data, scopes)
+
+    def to_json(self, strip=None):
+        """Utility function that creates JSON repr. of a Credentials object.


Suggested change

"""Utility function that creates JSON repr. of a Credentials object.

"""Utility function that creates JSON representation of a Credentials object.

busunkim96

@patkasper This looks good! I am doing some cleanup of the CI in another PR, but I will merge this once that is resolved.

Thank you for all the work you've put into this!

busunkim96 · 2019-11-20T17:28:15Z

👋 Hi @patkasper. Would you mind resolving the merge conflicts here?

busunkim96 · 2019-11-22T17:49:22Z

@patkasper Would you be able to run the code formatter?

pip install nox
nox -s blacken

If nox gives you trouble for any reason you can also do:

pip install black
black google tests

@michaelawyu If you're around next week could you help see this PR over the finish line? Since Patrick is an external contributor the CI does not trigger automatically. You'll have to add the label kokoro:force-run. Thank you!

nicain · 2021-09-04T02:56:17Z

@patkasper @busunkim96 I noticed this evening that I cannot round-trip a credential using "to_json" to serialize the object, and then reload.

I hit this when deserializing my credential and attempting to use it with a Dialogflow client; in that case, I received:

google.api_core.exceptions.PermissionDenied: 403 Your application has authenticated using end user credentials from the Google Cloud SDK or Google Cloud Shell which are not supported by the dialogflow.googleapis.com. We recommend configuring the billing/quota_project setting in gcloud or using a service account through the auth/impersonate_service_account setting. For more information about service accounts and how to use them in your application, see https://cloud.google.com/docs/authentication/. If you are getting this error with curl or similar tools, you may need to specify 'X-Goog-User-Project' HTTP header for quota and billing purposes. For more information regarding 'X-Goog-User-Project' header, please check https://cloud.google.com/apis/docs/system-parameters.

The hint here was the "billing quota"; when I compared the dict representations of the original and serialized-then-serialized clone of the original object, I saw that the "_quota_project_id" attribute was missing on the clone. Sure enough, after I added this in manually, my credential works.

Was the "quota_project_id" attribute intentionally excluded from the contents of the json blob, on serialization? If not, I am happy to work up a PR to include that attribute in the serialization (it is a kwarg of the initializaer of the Credentials class, so very easy to round-trip).

One other minor note; the serialized-then-serialized clone of the original credential cannot itself be serialized with to_json if the expiry kwarg is naively set to the expiry string from the serialization dict. This happened to me when I tried to naively deserialize by just "**" unpackinging the dictionary contents into the kwargs of the initializer. Then this line fails because self.expiry is a string, not a datetime. This should be pretty easy to fix with a isinstance or try-catch, and I am happy to provide a PR here as well.

What are your thoughts on these issues?

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 28, 2019

busunkim96 reviewed Sep 11, 2019

View reviewed changes

ghost commented Sep 16, 2019

View reviewed changes

busunkim96 suggested changes Sep 30, 2019

View reviewed changes

ghost changed the title ~~credential storage support~~ feat: add to_json method to google.oauth2.credentials.Credentials Oct 8, 2019

busunkim96 marked this pull request as ready for review October 8, 2019 17:53

ghost requested a review from busunkim96 October 11, 2019 09:58

busunkim96 approved these changes Oct 11, 2019

View reviewed changes

feat: add to_json method to google.oauth2.credentials.Credentials

0f65ac3

busunkim96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 21, 2019

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 21, 2019

fix: add test for to_json google/oauth2/credentials.py

bcd46eb

busunkim96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 22, 2019

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Nov 22, 2019

fix: ran black google tests

d29f118

busunkim96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 5, 2019

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 5, 2019

busunkim96 merged commit bfb1f8c into googleapis:master Dec 5, 2019

yoshi-automation mentioned this pull request Jan 22, 2020

[CHANGE ME] Re-generated to pick up changes in the API or client library generator. #429

Closed

		return base64.urlsafe_b64encode(value).rstrip(b'=')


		def validate_file(filename):

		warnings.warn(_MISSING_FILE_MESSAGE.format(filename))


		def parse_expiry(expiry):

	"""Utility function that creates JSON repr. of a Credentials object.
	"""Utility function that creates JSON representation of a Credentials object.

feat: add to_json method to google.oauth2.credentials.Credentials #367

feat: add to_json method to google.oauth2.credentials.Credentials #367

Uh oh!

Conversation

ghost commented Aug 28, 2019

Uh oh!

busunkim96 commented Aug 28, 2019

Uh oh!

ghost commented Aug 30, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

busunkim96 left a comment

Choose a reason for hiding this comment

Uh oh!

busunkim96 commented Nov 20, 2019

Uh oh!

busunkim96 commented Nov 22, 2019

Uh oh!

nicain commented Sep 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

feat: add `to_json` method to google.oauth2.credentials.Credentials #367

feat: add `to_json` method to google.oauth2.credentials.Credentials #367