Added self signed jwt service account credentials #503

kpayson64 · 2016-04-22T17:28:26Z

Initial self signed jwt credential support #252. Here is my thinking with this initial API:

-SelfSignedJWTCredentials class should not be on the OAuth2Client inheritance hierarchy, there are many oauth2 specific methods (like refresh) that don't apply to self signed credentials.
-SelfSignedJWTCredentials can be placed in the service_account module, SelfSignedJWTCredentials are highly coupled with service account credentials, self signed jwts can only be signed using service account credentials
-get_access_token() can be the external interface for generating tokens, it is consistent with the GoogleCredentials API, and there is nothing OAuth2 specific about the naming.
-The fields contained in the JWT are Google specific as far as I can tell, so this should be considered a Google specific class

oauth2client/service_account.py

+                              expires_in=self._time_limit)
+
+    @classmethod
+    def from_service_account_credentials(cls, creds, audience, 


theacodes · 2016-04-22T17:50:53Z

@kpayson64 Thanks for contributing this! :) 👍

kpayson64 · 2016-04-22T17:54:49Z

@jtattermusch

theacodes · 2016-04-22T18:08:50Z

@anthmgoogle @dhermes @nathanielmanistaatgoogle Care to chime in on API and naming conventions here?

I'd personally prefer this API (which closely matches pyjwt's):

service_account = oauth2client.service_account.ServiceAccount.from_json_keyfile_name('service-account.json')
jwt = oauth2client.jwt.encode(service_account)

Additional claims:

jwt = oauth2client.jwt.encode(service_account, {'sub': 'user@example.com'})

@nathanielmanistaatgoogle would probably be happy to see less classes. :D

theacodes · 2016-04-22T18:14:07Z

We could also expose this as ServiceAccount.encode_jwt, since a lot of this logic is already in ServiceAccount._generate_assertion. ¯_(ツ)_/¯

dhermes · 2016-04-22T18:19:12Z

@jonparrott That misses the mark. These credentials need to be able to be used as a swap-in replacement for credentials.

theacodes · 2016-04-22T18:21:17Z

@dhermes interesting. Can you expand more on that? JWTs are not credentials, they are signed claims that can be used for authorization. The service account private key is the credential.

theacodes · 2016-04-22T18:23:27Z

Is the desire to do something like this?

credentials = JwtCredentials(service_account_credentials)
credentials.authorize(http)
# http now sends the JWT in the Authorization header

If so, I don't see how this is distinct from just making the existing ServiceAccount accept a claims arg in its constructor.

dhermes · 2016-04-22T18:25:01Z

For APIs like Cloud Bigtable, the JWT can be used as the access token (so-called scopless credentials). Hence, we want a class to allow this, and maybe have some built-in retry behavior which regenerates the local token (this PR doesn't quite get there either).

dhermes · 2016-04-22T18:26:53Z

@jonparrott Sorry we posted at essentially the same moment.

The difference is that ServiceAccountCredentials uses the JWT to exchange for a bearer token (which requires a round trip).

theacodes · 2016-04-22T18:31:02Z

Ah, I see, I should get more sleep before discussing auth.

So, I think I'd prefer "both". We should separate the logic of generating a JWT vs using the jwt.

I suggest we:

Add oauth2client.jwt.encode that accepts a signer and set of claims and returns a JWT.
Make this new class use it to generate its access token.
Make ServiceAccount use it to generate its assertion.

This covers all use-cases well and prevents duplicate code.

kpayson64 · 2016-04-22T18:38:49Z

@jonparrott
I think the oauth2client.jwt.encode you are suggesting is essentially crypt.make_signed_jwt.

@dhermes
Do we want the new class to implement client.Credentials? Could you be more specific with the functionality that is missing?

oauth2client/service_account.py

@@ -461,3 +462,56 @@ def create_delegated(self, sub):
        result._private_key_pkcs12 = self._private_key_pkcs12
        result._private_key_password = self._private_key_password
        return result
+
+
+class SelfSignedJWTCredentials(object):


theacodes · 2016-04-22T21:38:48Z

Okay, after chatting with @jboeuf the API should shake out like this:

class JWTAccessCredentials(object):
    def __init__(self, service_account, additional_claims=None)
         pass

    def get_access_token(self, additional_claims=None)
          pass

    def create_with_claims(claims):
          pass

This allows for it to satisfy the oauth2client.client.Credentials interface as well as work for gRPC by allowing gRPC to explicitly call get_access_token(additional_claims={'aud': service_url}) without having to create tons of instances.

kpayson64 · 2016-04-26T15:08:24Z

This latest commit has the new API. Also, I have made JWTAccessCredentials inherit GoogleCredentials . I'm holding off on unit tests/documentation until I have an API nailed down.

oauth2client/service_account.py

+
+    def __init__(self, service_account_creds, additional_claims=None, access_token=None,
+                 token_expiry=None):
+        super(JWTAccessCredentials, self).__init__(access_token, None, None, None, token_expiry,


theacodes · 2016-04-26T17:57:40Z

API surface looks good, I completed a round of reviews. Please ping when you're ready for another review.

Side note: your file seems to have mixed indentation, please use 4 spaces.

kpayson64 · 2016-04-26T20:14:01Z

@jonparrott
I've updated the pr from the feedback.

oauth2client/service_account.py

+
+
+def _datetime_to_secs(utc_time):
+   # time_delta.total_seconds() not supported in Python 2.6


kpayson64 · 2016-05-05T17:54:52Z

@jonparrott
Is the following use case of this library deprecated?

http = httplib2.Http()
creds.authorize(http)

If so, is there any expected use case of JWT tokens outside of gRPC I should consider?

theacodes · 2016-05-05T18:06:50Z

@kpayson64 no, but in general we want to move away from httplib2 (or rather, support urllib3) as soon as possible. If you're writing a net-new project and want thread-safety and proper https, urllib3 is a much more sane way to go. see #128 and gcloud-python#1364.

nathanielmanistaatgoogle · 2016-05-05T18:16:03Z

I think I really like the API change in the current draft - we're adding one new method to one existing class and that's it for changes to the public surface area of the entire library?

Is this the shape this change should have going forward or are there still design pressures pushing this in the direction of "no, a new public class is the right thing for this feature"?

kpayson64 · 2016-05-05T18:24:09Z

@jonparrott
Sounds good, I can try to produce a POC using urllib3.

A side note, AFAICT credentials are not thread safe, and I assumed this was because the underlying httplib2 is not thread safe. Should the new code be thread safe?

theacodes · 2016-05-05T18:28:07Z

Credentials are thread-safe as long as their storage is thread-safe.

On Thu, May 5, 2016 at 11:24 AM kpayson64 notifications@github.com wrote:

@jonparrott https://github.com/jonparrott
Sounds good, I can try to produce a POC using urllib3.

A side note, AFAICT credentials are not thread safe, and I assumed this
was because the underlying httplib2 is not thread safe. Should the new code
be thread safe?

—
You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
#503 (comment)

kpayson64 · 2016-05-06T20:11:07Z

It seems like we have converged on an API. I think the one unresolved issue is if JWTAccessCredentials should be public. It should be a trivial change to make JWTAccessCredentials public in the future, so I think we should leave it private for now.

If we are comfortable with this API, could @jonparrott @nathanielmanistaatgoogle do a final review?

oauth2client/client.py

-                headers = {}
-            else:
-                headers = dict(headers)
+            headers = self._initialize_headers(headers)


kpayson64 · 2016-05-11T22:27:01Z

I have addressed the feedback.

oauth2client/client.py

-        from oauth2client.service_account import ServiceAccountCredentials
-        return ServiceAccountCredentials.from_json_keyfile_dict(
+        from oauth2client.service_account import _JWTAccessCredentials
+


nathanielmanistaatgoogle · 2016-05-12T15:45:27Z

Change content looks good!

Please squash commits.

Please compose a commit message with at least a paragraph explaining what's going on. Is this a Google-proprietary feature? Is this a Google-specific feature? Is there a publicly available open and unencumbered standard being implemented in this change that you can cite and to which you can link?

Newer Google APIs can accept JWTs signed using ServiceAccountCredentials for authentication. (See https://jwt.io/). The new behavior for GoogleCredentials.get_application_default() will attempt to use a signed JWT if ServiceAccountCredentials are available and no scope is specified. Upon specifying a scope, OAuth2 authentication will be used.

kpayson64 · 2016-05-16T17:50:13Z

I've squashed the commits and updated the commit message.

nathanielmanistaatgoogle · 2016-05-18T14:20:17Z

I'm satisfied; @dhermes, @jonparrott, and @jboeuf: please either also make a final review or explicitly decline to do so?

dhermes · 2016-05-18T15:25:59Z

I decline 😀 I trust you

theacodes · 2016-05-18T15:26:52Z

I'm satisfied.

On Wed, May 18, 2016, 8:26 AM Danny Hermes notifications@github.com wrote:

I decline [image: 😀] I trust you

—
You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
#503 (comment)

jboeuf · 2016-05-18T16:08:16Z

LGTM. Thanks! Please CC me on the PR that will do the integration with grpc.

nathanielmanistaatgoogle · 2016-05-18T17:14:16Z

Woohoo!

kpayson64 · 2016-05-18T17:24:07Z

@jonparrott
What is the process for releasing a new version?

theacodes · 2016-05-18T17:24:54Z

I probably won't have time to cut a release this week. @nathanielmanistaatgoogle may be able to.

theacodes · 2016-05-19T18:06:23Z

I'd like to see #506 merged before we release.

ensonic · 2017-09-07T08:18:13Z

Can you make _JWTAccessCredentials public? Or how else would/show I do something like below. Please also not the hack to set 'aud'.

from oauth2client.service_account import _JWTAccessCredentials as JWTAccessCredentials
import httplib2

httplib2.debuglevel = 1

credentials = JWTAccessCredentials.from_json_keyfile_name(
    '/path/to/service-account-credentials.json', scopes=['https://www.googleapis.com/auth/datastore'])
# the class method does not pass through he kwargs to __init__()
# this needs to match the service name in the api-config:
credentials._kwargs['aud'] = 'myapi.endpoints.myproject.cloud.goog'

http_auth = credentials.authorize(httplib2.Http(disable_ssl_certificate_validation=True))
resp, content = http_auth.request('https://example.com/v1/myapi', 'GET')

Or should people manually do this as e.g. in:
https://github.com/GoogleCloudPlatform/python-docs-samples/blob/master/endpoints/getting-started/clients/google-jwt-client.py

googlebot added the cla: yes label Apr 22, 2016

dhermes assigned nathanielmanistaatgoogle Apr 22, 2016

dhermes reviewed Apr 22, 2016
View reviewed changes

oauth2client/service_account.py

expires_in=self._time_limit)

@classmethod

def from_service_account_credentials(cls, creds, audience,

This comment was marked as spam.

Sign in to view

jboeuf reviewed Apr 22, 2016
View reviewed changes

theacodes reviewed Apr 26, 2016
View reviewed changes

nathanielmanistaatgoogle reviewed May 11, 2016
View reviewed changes

oauth2client/client.py

headers = {}

else:

headers = dict(headers)

headers = self._initialize_headers(headers)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

nathanielmanistaatgoogle reviewed May 12, 2016
View reviewed changes

oauth2client/client.py

from oauth2client.service_account import ServiceAccountCredentials

return ServiceAccountCredentials.from_json_keyfile_dict(

from oauth2client.service_account import _JWTAccessCredentials

This comment was marked as spam.

Sign in to view

kpayson64 force-pushed the ss_jwt_creds branch from 94ffe98 to 82cde04 Compare May 16, 2016 17:46

nathanielmanistaatgoogle merged commit f0ba603 into googleapis:master May 18, 2016

theacodes mentioned this pull request May 20, 2016

Release v2.1.0 #509

Merged

theacodes mentioned this pull request Jun 6, 2016

SignedJwtAssertionCredentials has been removed: Why? #401

Closed

nathanielmanistaatgoogle mentioned this pull request Jun 28, 2016

Adding Python 3.5 capability #531

Merged

nathanielmanistaatgoogle mentioned this pull request Nov 1, 2016

Add grpc transport googleapis/google-auth-library-python#67

Merged

google-admin unassigned nathanielmanistaatgoogle Sep 11, 2018



		def _datetime_to_secs(utc_time):
		# time_delta.total_seconds() not supported in Python 2.6

Added self signed jwt service account credentials #503

Added self signed jwt service account credentials #503

Conversation

kpayson64 commented Apr 22, 2016

This comment was marked as spam.

theacodes commented Apr 22, 2016

kpayson64 commented Apr 22, 2016

theacodes commented Apr 22, 2016

theacodes commented Apr 22, 2016 • edited

dhermes commented Apr 22, 2016

theacodes commented Apr 22, 2016

theacodes commented Apr 22, 2016

dhermes commented Apr 22, 2016

dhermes commented Apr 22, 2016

theacodes commented Apr 22, 2016

kpayson64 commented Apr 22, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

theacodes commented Apr 22, 2016

kpayson64 commented Apr 26, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

theacodes commented Apr 26, 2016

kpayson64 commented Apr 26, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

kpayson64 commented May 5, 2016

theacodes commented May 5, 2016

nathanielmanistaatgoogle commented May 5, 2016

kpayson64 commented May 5, 2016

theacodes commented May 5, 2016

kpayson64 commented May 6, 2016

This comment was marked as spam.

This comment was marked as spam.

kpayson64 commented May 11, 2016

This comment was marked as spam.

nathanielmanistaatgoogle commented May 12, 2016

kpayson64 commented May 16, 2016

nathanielmanistaatgoogle commented May 18, 2016

dhermes commented May 18, 2016

theacodes commented May 18, 2016

jboeuf commented May 18, 2016

nathanielmanistaatgoogle commented May 18, 2016

kpayson64 commented May 18, 2016

theacodes commented May 18, 2016

theacodes commented May 19, 2016

ensonic commented Sep 7, 2017 • edited

theacodes commented Apr 22, 2016 •

edited

ensonic commented Sep 7, 2017 •

edited