[GitHub] [pulsar] tuteng commented on a change in pull request #11988: [Java Client] Make Audience Field Optional in OAuth2 Client Credentials

[GitBox <gi...@apache.org>]

tuteng commented on a change in pull request #11988:
URL: https://github.com/apache/pulsar/pull/11988#discussion_r706593281



##########
File path: site2/docs/security-oauth2.md
##########
@@ -28,7 +28,7 @@ The following table lists parameters supported for the `client credentials` auth
 | `type` | Oauth 2.0 authentication type. |  `client_credentials` (default) | Optional |
 | `issuerUrl` | URL of the authentication provider which allows the Pulsar client to obtain an access token | `https://accounts.google.com` | Required |
 | `privateKey` | URL to a JSON credentials file  | Support the following pattern formats: <br> <li> `file:///path/to/file` <li>`file:/path/to/file` <li> `data:application/json;base64,<base64-encoded value>` | Required |
-| `audience`  | An OAuth 2.0 "resource server" identifier for the Pulsar cluster | `https://broker.example.com` | Required |
+| `audience`  | An OAuth 2.0 "resource server" identifier for the Pulsar cluster | `https://broker.example.com` | Optional |

Review comment:
       Not sure if updating this field to be optional will cause confusion for users using other languages, one suggestion I have is to keep this field as required until other languages implement this change

##########
File path: site2/website/versioned_docs/version-2.8.1/security-oauth2.md
##########
@@ -29,7 +29,7 @@ The following table lists parameters supported for the `client credentials` auth
 | `type` | Oauth 2.0 authentication type. |  `client_credentials` (default) | Optional |
 | `issuerUrl` | URL of the authentication provider which allows the Pulsar client to obtain an access token | `https://accounts.google.com` | Required |
 | `privateKey` | URL to a JSON credentials file  | Support the following pattern formats: <br> <li> `file:///path/to/file` <li>`file:/path/to/file` <li> `data:application/json;base64,<base64-encoded value>` | Required |
-| `audience`  | An OAuth 2.0 "resource server" identifier for the Pulsar cluster | `https://broker.example.com` | Required |
+| `audience`  | An OAuth 2.0 "resource server" identifier for the Pulsar cluster. Required by some Identity Providers. | `https://broker.example.com` | Optional |

Review comment:
       Not sure if updating this field to be optional will cause confusion for users using other languages, one suggestion I have is to keep this field as required until other languages implement this change

##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/auth/oauth2/README.md
##########
@@ -46,7 +46,7 @@ The following parameters are supported:
 | `type` | Oauth 2.0 auth type. Optional. | default: `client_credentials`  |
 | `issuerUrl` | URL of the provider which allows Pulsar to obtain an access token. Required. | `https://accounts.google.com` |
 | `privateKey` | URL to a JSON credentials file (in JSON format; see below). Required. | See "Supported Pattern Formats" |
-| `audience`  | An OAuth 2.0 "resource server" identifier for the Pulsar cluster. Required. | `https://broker.example.com` |
+| `audience`  | An OAuth 2.0 "resource server" identifier for the Pulsar cluster. Required by some Identity Providers. Optional for client. | `https://broker.example.com` |

Review comment:
       `Optional for client.` => `Optional for java client.`?
   
   Just a note, we also need to make this field optional for other language clients that support oauth authentication (e.g. cpp, python, golang), otherwise, there may be inconsistent behavior, e.g. for java language this field is optional and for cpp this field is required, can you create several issues, in order for us to track this thing
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org