Copilot commented on code in PR #4337: URL: https://github.com/apache/solr/pull/4337#discussion_r3142778593
########## solr/solr-ref-guide/modules/deployment-guide/pages/jwt-authentication-plugin.adoc: ########## @@ -42,27 +42,25 @@ The simplest possible `security.json` for registering the plugin without configu } ---- -The plugin will by default require a valid JWT token for all traffic. - -If the `blockUnknown` property is set to `false` as in the above example, it is possible to start configuring the plugin using unauthenticated REST API calls, which is further described in section <<Editing JWT Authentication Plugin Configuration>>. +By default (`blockUnknown=false`), requests without a JWT are allowed through, which makes it possible to configure the plugin using unauthenticated REST API calls as described in <<Editing JWT Authentication Plugin Configuration>>. Set `blockUnknown=true` to require a valid JWT for all requests. Review Comment: The intro/example uses `"blockUnknown":"false"` (a JSON string) while later examples use a boolean (`true`). Since this section now emphasizes the default behavior, it would be clearer and more correct JSON to use an unquoted boolean (`false`) or omit `blockUnknown` entirely to demonstrate the default. ########## solr/solr-ref-guide/modules/deployment-guide/pages/jwt-authentication-plugin.adoc: ########## @@ -84,7 +82,7 @@ clientId ; Client identifier for use with OpenID Connect. Required t jwksUrl ; A URL to a https://tools.ietf.org/html/rfc7517#section-5[JWKs] endpoint. Must use https protocol. Optionally an array of URLs in which case all public keys from all URLs will be consulted when validating signatures. ; Auto configured if `wellKnownUrl` is provided jwk ; As an alternative to `jwksUrl` you may provide a static JSON object containing the public key(s) of the issuer. The format is either JWK or JWK Set, see https://tools.ietf.org/html/rfc7517#appendix-A[RFC7517] for examples. ; iss ; Unique issuer id as configured on the IdP. Incoming tokens must have a matching `iss` claim. Also used to resolve issuer when multiple issuers configured. ; Auto configured if `wellKnownUrl` is provided -aud ; Validates that the `aud` (audience) claim equals this string ; Uses `clientId` if configured +aud ; Validates that the `aud` (audience) claim equals this string ; If not set, audience validation is skipped entirely Review Comment: The `aud` row says that if `aud` is not set, audience validation is skipped entirely. In code, audience validation is only skipped when *no* issuers configure `aud` at all; if any issuer sets `aud`, the JwtConsumer enforces that expected-audience list for all tokens. Consider rephrasing the default to reflect this global behavior (e.g., skipped only when no `aud` values are configured). -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
