feat: Implement JWT OAuth 2 Client Authentication as HttpExecuteInterceptor. #582

junying1 · 2020-12-29T01:01:04Z

Fixes #580 ☕️

Simple implementation modeled after ClientParametersAuthentication.

google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/JWTAuthentication.java

google-oauth-client/src/test/java/com/google/api/client/auth/oauth2/JWTAuthenticationTest.java

google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/JWTAuthentication.java

google-oauth-client/src/test/java/com/google/api/client/auth/oauth2/JWTAuthenticationTest.java

… request.

google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/JWTAuthentication.java

google-oauth-client/src/test/java/com/google/api/client/auth/oauth2/JWTAuthenticationTest.java

google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/JWTAuthentication.java

google-oauth-client/src/test/java/com/google/api/client/auth/oauth2/JWTAuthenticationTest.java

junying1

Requested change made.

google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/JWTAuthentication.java

elharo · 2021-01-11T13:11:55Z

google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/JWTAuthentication.java

+        } else {
+            String grantType = (String) data.get(GRANT_TYPE_KEY);
+            if (!grantType.equals(GRANT_TYPE_CLIENT_CREDENTIALS)) {
+                throw new IllegalArgumentException(GRANT_TYPE_KEY


I'm not sure a runtime exception is appropriate here. It's a gray area. At the very least, the exception needs to be documented.

Agree it's gray. The option I thought about was to just ignore whatever they try to set, since the GRANT_TYPE for JWT authentication has to be "client_credentials". Do you prefer that?

google-oauth-client/src/test/java/com/google/api/client/auth/oauth2/JWTAuthenticationTest.java

elharo · 2021-01-11T13:12:40Z

google-oauth-client/src/test/java/com/google/api/client/auth/oauth2/JWTAuthenticationTest.java

+import com.google.api.client.json.jackson2.JacksonFactory;
+import com.google.api.client.testing.http.HttpTesting;
+import com.google.api.client.testing.http.MockHttpTransport;
+import junit.framework.TestCase;


Implemented the same way as all the other tests in the package. Shouldn't it at least stay consistent? Are you saying don't extend TestCase at all? TestCase is in junit.framework.

google-oauth-client/src/test/java/com/google/api/client/auth/oauth2/JWTAuthenticationTest.java

elharo · 2021-01-11T13:15:04Z

google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/JWTAuthentication.java

+    }
+
+    public void intercept(HttpRequest request) {
+        Map<String, Object> data = Data.mapOf(UrlEncodedContent.getContent(request).getData());


This method seems to be mutating the internal state of some object. This is very suspicious.

The purpose of an authentication intercept pretty much requires it. It needs to add the appropriate authentication information into the HTTP request. It's the decorator pattern. In any case, it is implemented exactly like ClientParametersAuthentication already in the package.

elharo

All new code must follow the Google Java style guide. No exceptions.

We explicitly do not want consistency with existing code that does not follow the style guide.

junying1 · 2021-01-11T14:15:39Z

All new code must follow the Google Java style guide. No exceptions.

We explicitly do not want consistency with existing code that does not follow the style guide.

Understand. I made the changes requested style changes. However, for the test, are you saying don't extend TestCase any more?

yoshi-code-bot · 2021-01-11T14:18:48Z

google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/JWTAuthentication.java

+
+import java.io.IOException;
+import java.util.Map;
+
+/**
+ * Client credentials specified as URL-encoded parameters in the HTTP request body as specified in
+ * <a href="https://tools.ietf.org/html/rfc7523">JSON Web Token (JWT) Profile
+ *       for OAuth 2.0 Client Authentication and Authorization Grants</a>
+ *


Suggested change

import java.io.IOException;

import java.util.Map;

/**

* Client credentials specified as URL-encoded parameters in the HTTP request body as specified in

* <a href="https://tools.ietf.org/html/rfc7523">JSON Web Token (JWT) Profile

* for OAuth 2.0 Client Authentication and Authorization Grants</a>

*

* <a href="https://tools.ietf.org/html/rfc7523">JSON Web Token (JWT) Profile for OAuth 2.0 Client

* Authentication and Authorization Grants</a>

* <p>To use JWT authentication, grant_type must be "client_credentials". If

* AuthorizationCodeTokenRequest.setGrantType() is called, set it to

* JWTAuthentication.GRANT_TYPE_CLIENT_CREDENTIALS. It can also be left uncalled. Setting it to any

* other value causes an IllegalArgumentException.

yoshi-code-bot · 2021-01-11T14:18:49Z

google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/JWTAuthentication.java

+
+public class JWTAuthentication
+        implements HttpRequestInitializer, HttpExecuteInterceptor {
+
+  public static final String GRANT_TYPE_KEY = "grant_type";
+
+  /** Predefined value for grant_type when using JWT **/
+  public static final String GRANT_TYPE_CLIENT_CREDENTIALS = "client_credentials";
+


Suggested change

public class JWTAuthentication

implements HttpRequestInitializer, HttpExecuteInterceptor {

public static final String GRANT_TYPE_KEY = "grant_type";

/** Predefined value for grant_type when using JWT **/

public static final String GRANT_TYPE_CLIENT_CREDENTIALS = "client_credentials";

public class JWTAuthentication implements HttpRequestInitializer, HttpExecuteInterceptor {

/** Predefined value for grant_type when using JWT * */

/** Predefined value for client_assertion_type when using JWT * */

public static final String CLIENT_ASSERTION_TYPE =

"urn:ietf:params:oauth:client-assertion-type:jwt-bearer";

/** @param jwt JWT used for authentication */

yoshi-code-bot · 2021-01-11T14:18:49Z

google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/JWTAuthentication.java

+    } else {
+      String grantType = (String) data.get(GRANT_TYPE_KEY);
+      if (!grantType.equals(GRANT_TYPE_CLIENT_CREDENTIALS)) {
+        throw new IllegalArgumentException(GRANT_TYPE_KEY


Suggested change

throw new IllegalArgumentException(GRANT_TYPE_KEY

throw new IllegalArgumentException(

GRANT_TYPE_KEY

yoshi-code-bot · 2021-01-11T14:18:49Z

google-oauth-client/src/test/java/com/google/api/client/auth/oauth2/JWTAuthenticationTest.java

+
+package com.google.api.client.auth.oauth2;
+
+import com.google.api.client.http.GenericUrl;


Suggested change

import com.google.api.client.http.GenericUrl;

import static org.junit.Assert.assertThrows;

import com.google.api.client.http.GenericUrl;

yoshi-code-bot · 2021-01-11T14:18:49Z

google-oauth-client/src/test/java/com/google/api/client/auth/oauth2/JWTAuthenticationTest.java

+import junit.framework.TestCase;
+import static org.junit.Assert.assertThrows;


Suggested change

import junit.framework.TestCase;

import static org.junit.Assert.assertThrows;

import junit.framework.TestCase;

yoshi-code-bot · 2021-01-11T14:18:50Z

google-oauth-client/src/test/java/com/google/api/client/auth/oauth2/JWTAuthenticationTest.java

+            new ClientCredentialsTokenRequest(new MockHttpTransport(), new JacksonFactory(),
+                    new GenericUrl(HttpTesting.SIMPLE_GENERIC_URL.toString()));
+
+    JWTAuthentication auth =


Suggested change

new ClientCredentialsTokenRequest(new MockHttpTransport(), new JacksonFactory(),

new GenericUrl(HttpTesting.SIMPLE_GENERIC_URL.toString()));

JWTAuthentication auth =

new ClientCredentialsTokenRequest(

new MockHttpTransport(),

new JacksonFactory(),

new GenericUrl(HttpTesting.SIMPLE_GENERIC_URL.toString()));

JWTAuthentication auth = new JWTAuthentication(JWT);

yoshi-code-bot · 2021-01-11T14:18:50Z

google-oauth-client/src/test/java/com/google/api/client/auth/oauth2/JWTAuthenticationTest.java

+    JWTAuthentication auth =
+        new JWTAuthentication(JWT);


Suggested change

JWTAuthentication auth =

new JWTAuthentication(JWT);

JWTAuthentication auth = new JWTAuthentication(JWT);

yoshi-code-bot · 2021-01-11T14:18:50Z

google-oauth-client/src/test/java/com/google/api/client/auth/oauth2/JWTAuthenticationTest.java

+            new ClientCredentialsTokenRequest(new MockHttpTransport(), new JacksonFactory(),
+                    new GenericUrl(HttpTesting.SIMPLE_GENERIC_URL.toString()));
+
+    JWTAuthentication auth =


Suggested change

new ClientCredentialsTokenRequest(new MockHttpTransport(), new JacksonFactory(),

new GenericUrl(HttpTesting.SIMPLE_GENERIC_URL.toString()));

JWTAuthentication auth =

new ClientCredentialsTokenRequest(

new MockHttpTransport(),

new JacksonFactory(),

new GenericUrl(HttpTesting.SIMPLE_GENERIC_URL.toString()));

JWTAuthentication auth = new JWTAuthentication(JWT);

yoshi-code-bot · 2021-01-11T14:18:50Z

google-oauth-client/src/test/java/com/google/api/client/auth/oauth2/JWTAuthenticationTest.java

+
+    assertThrows(IllegalArgumentException.class, new ThrowingRunnable() {
+      @Override
+      public void run() throws Throwable {
+        request.executeUnparsed();
+      }
+    });
+  }
+
+  public void test_noJWT() {
+    assertThrows(RuntimeException.class, new ThrowingRunnable() {
+      @Override
+      public void run() {


Suggested change

assertThrows(IllegalArgumentException.class, new ThrowingRunnable() {

@Override

public void run() throws Throwable {

request.executeUnparsed();

}

});

}

public void test_noJWT() {

assertThrows(RuntimeException.class, new ThrowingRunnable() {

@Override

public void run() {

assertThrows(

IllegalArgumentException.class,

new ThrowingRunnable() {

@Override

public void run() throws Throwable {

request.executeUnparsed();

}

});

assertThrows(

RuntimeException.class,

new ThrowingRunnable() {

@Override

public void run() {

JWTAuthentication auth = new JWTAuthentication(null);

}

});

lesv · 2021-11-03T17:30:32Z

What's the status of this PR? Should it be closed?

junying1 · 2021-11-03T18:09:50Z

It's pending review to be merged. I made the changes as requested.

Implement JWT OAuth 2 Client Authentication as HttpExecuteInterceptor.

2629144

junying1 requested a review from a team as a code owner December 29, 2020 01:01

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Dec 29, 2020

yoshi-code-bot reviewed Dec 29, 2020

View reviewed changes

elharo suggested changes Dec 29, 2020

View reviewed changes

google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/JWTAuthentication.java Outdated Show resolved Hide resolved

elharo requested a review from silvolu December 29, 2020 14:19

elharo reviewed Dec 29, 2020

View reviewed changes

Assigned copyrigh to Google.

a5979ba

yoshi-code-bot reviewed Dec 29, 2020

View reviewed changes

junying1 added 2 commits December 29, 2020 09:41

Updated comment to present tense per Google style.

8cad037

Changed code usage example in comment to use GsonFactory per Google's…

122be49

… request.

yoshi-code-bot reviewed Dec 29, 2020

View reviewed changes

elharo changed the title ~~Implement JWT OAuth 2 Client Authentication as HttpExecuteInterceptor.~~ feat: Implement JWT OAuth 2 Client Authentication as HttpExecuteInterceptor. Dec 29, 2020

junying1 requested a review from elharo January 1, 2021 21:09

junying1 commented Jan 7, 2021

View reviewed changes

elharo suggested changes Jan 11, 2021

View reviewed changes

junying1 added 5 commits January 11, 2021 08:48

Took out (c) in copyright.

00b2c58

Took out (c) in copyright.

3d3dfc0

Sorted imports.

02c5093

Changed indent to 2 spaces per Google coding style standard.

ed06dcd

Indents for sample code in comment.

53cb120

elharo reviewed Jan 11, 2021

View reviewed changes

yoshi-code-bot reviewed Jan 11, 2021

View reviewed changes

Neenu1995 added the release-please:force-run To run release-please label Aug 12, 2021

release-please bot removed the release-please:force-run To run release-please label Aug 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Implement JWT OAuth 2 Client Authentication as HttpExecuteInterceptor. #582

feat: Implement JWT OAuth 2 Client Authentication as HttpExecuteInterceptor. #582

junying1 commented Dec 29, 2020

junying1 left a comment

elharo Jan 11, 2021

junying1 Jan 11, 2021

elharo Jan 11, 2021

junying1 Jan 11, 2021

elharo Jan 11, 2021

junying1 Jan 11, 2021

elharo left a comment

junying1 commented Jan 11, 2021

yoshi-code-bot Jan 11, 2021

yoshi-code-bot Jan 11, 2021

yoshi-code-bot Jan 11, 2021

yoshi-code-bot Jan 11, 2021

yoshi-code-bot Jan 11, 2021

yoshi-code-bot Jan 11, 2021

yoshi-code-bot Jan 11, 2021

yoshi-code-bot Jan 11, 2021

yoshi-code-bot Jan 11, 2021

lesv commented Nov 3, 2021

junying1 commented Nov 3, 2021

-import java.io.IOException;
-import java.util.Map;
-/**
- * Client credentials specified as URL-encoded parameters in the HTTP request body as specified in
- * <a href="https://tools.ietf.org/html/rfc7523">JSON Web Token (JWT) Profile
- *       for OAuth 2.0 Client Authentication and Authorization Grants</a>
- *
+ * <a href="https://tools.ietf.org/html/rfc7523">JSON Web Token (JWT) Profile for OAuth 2.0 Client
+ * Authentication and Authorization Grants</a>
+ * <p>To use JWT authentication, grant_type must be "client_credentials". If
+ * AuthorizationCodeTokenRequest.setGrantType() is called, set it to
+ * JWTAuthentication.GRANT_TYPE_CLIENT_CREDENTIALS. It can also be left uncalled. Setting it to any
+ * other value causes an IllegalArgumentException.

-public class JWTAuthentication
-        implements HttpRequestInitializer, HttpExecuteInterceptor {
-  public static final String GRANT_TYPE_KEY = "grant_type";
-  /** Predefined value for grant_type when using JWT **/
-  public static final String GRANT_TYPE_CLIENT_CREDENTIALS = "client_credentials";
+public class JWTAuthentication implements HttpRequestInitializer, HttpExecuteInterceptor {
+  /** Predefined value for grant_type when using JWT * */
+  /** Predefined value for client_assertion_type when using JWT * */
+  public static final String CLIENT_ASSERTION_TYPE =
+      "urn:ietf:params:oauth:client-assertion-type:jwt-bearer";
+  /** @param jwt JWT used for authentication */

	throw new IllegalArgumentException(GRANT_TYPE_KEY
	throw new IllegalArgumentException(
	GRANT_TYPE_KEY


		package com.google.api.client.auth.oauth2;

		import com.google.api.client.http.GenericUrl;

		import junit.framework.TestCase;
		import static org.junit.Assert.assertThrows;

	JWTAuthentication auth =
	new JWTAuthentication(JWT);
	JWTAuthentication auth = new JWTAuthentication(JWT);

feat: Implement JWT OAuth 2 Client Authentication as HttpExecuteInterceptor. #582

Are you sure you want to change the base?

feat: Implement JWT OAuth 2 Client Authentication as HttpExecuteInterceptor. #582

Conversation

junying1 commented Dec 29, 2020

junying1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elharo left a comment

Choose a reason for hiding this comment

junying1 commented Jan 11, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lesv commented Nov 3, 2021

junying1 commented Nov 3, 2021