You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "laminelam (via GitHub)" <gi...@apache.org> on 2023/08/17 20:46:36 UTC
[GitHub] [solr] laminelam opened a new pull request, #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
laminelam opened a new pull request, #1792:
URL: https://github.com/apache/solr/pull/1792
https://issues.apache.org/jira/browse/SOLR-16897
This is the "back-end" part of '[code with PKCE](https://github.com/apache/solr/pull/1791)' contribution.
The back-end code is mainly for configuration. This is where the different options are configured.
This PR adds _tokenEndpoint_ and _authorizationFlow_ options.
# Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the `main` branch.
- [x] I have run `./gradlew check`.
- [x] I have added tests for my changes.
- [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] laminelam commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "laminelam (via GitHub)" <gi...@apache.org>.
laminelam commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1311976271
##########
solr/CHANGES.txt:
##########
@@ -147,6 +149,8 @@ Bug Fixes
* SOLR-16916: Use of the JSON Query DSL should ignore the defType parameter
(Christina Chortaria, Max Kadel, Ryan Laddusaw, Jane Sandberg, David Smiley)
+* SOLR-16900: Add/fix wrong/missing "solr.home" to zkcli scripts (Lamine Idjeraoui. Review by Eric Pugh & Jason Gerlowski)
Review Comment:
done :)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1301667152
##########
solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTIssuerConfig.java:
##########
@@ -64,12 +72,18 @@ public class JWTIssuerConfig {
private WellKnownDiscoveryConfig wellKnownDiscoveryConfig;
private String clientId;
private String authorizationEndpoint;
+ private String tokenEndpoint;
+ private String authorizationFlow;
private Collection<X509Certificate> trustedCerts;
public static boolean ALLOW_OUTBOUND_HTTP =
Boolean.parseBoolean(System.getProperty("solr.auth.jwt.allowOutboundHttp", "false"));
public static final String ALLOW_OUTBOUND_HTTP_ERR_MSG =
"HTTPS required for IDP communication. Please use SSL or start your nodes with -Dsolr.auth.jwt.allowOutboundHttp=true to allow HTTP for test purposes.";
+ private static final String DEFAULT_AUTHORIZATION_FLOWS =
Review Comment:
Nit: use singular form for the constant
```suggestion
private static final String DEFAULT_AUTHORIZATION_FLOW =
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1296170780
##########
solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java:
##########
@@ -847,9 +851,11 @@ protected String generateAuthDataHeader() {
Map<String, Object> data = new HashMap<>();
data.put(
JWTIssuerConfig.PARAM_AUTHORIZATION_ENDPOINT, primaryIssuer.getAuthorizationEndpoint());
Review Comment:
```suggestion
"authorization_endpoint", primaryIssuer.getAuthorizationEndpoint());
```
See comment in line below
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy merged pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy merged PR #1792:
URL: https://github.com/apache/solr/pull/1792
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1311948300
##########
solr/CHANGES.txt:
##########
@@ -147,6 +149,8 @@ Bug Fixes
* SOLR-16916: Use of the JSON Query DSL should ignore the defType parameter
(Christina Chortaria, Max Kadel, Ryan Laddusaw, Jane Sandberg, David Smiley)
+* SOLR-16900: Add/fix wrong/missing "solr.home" to zkcli scripts (Lamine Idjeraoui. Review by Eric Pugh & Jason Gerlowski)
Review Comment:
Unrelated edit?
##########
solr/CHANGES.txt:
##########
@@ -107,6 +107,8 @@ Improvements
* SOLR-16927: Allow SolrClientCache clients to use Jetty HTTP2 clients (Alex Deparvu, David Smiley)
+* SOLR-16897: Add support of OAuth 2.0/OIDC 'code with PKCE' flow - back-end (Lamine Idjeraoui. Review and contribution by janhoy. Review by Kevin Risden)
Review Comment:
Simply list the contributors, no need to qualify what each one contributed.
Github will set author and add `Co-authored-by: John Doe <jo...@doe.com>` for code contributors in final merge message.
Also, no ned for two CHANGES entris even if you split it in two PRs with two commits. Suggest to add both JIRA numbers in same line:
```suggestion
* SOLR-16896, SOLR-16897: Add support of OAuth 2.0/OIDC 'code with PKCE' flow (Lamine Idjeraoui, janhoy, Kevin Risden)
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] laminelam commented on pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "laminelam (via GitHub)" <gi...@apache.org>.
laminelam commented on PR #1792:
URL: https://github.com/apache/solr/pull/1792#issuecomment-1697196130
> Eager to get this into 9.4. Let me know if we can help finalizing these two PRs
@janhoy
I should push a new commit sometime today. I have changed little bit your CSP patch, I just need to add a test class for LoadAdminUiServlet.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1292849716
##########
solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTIssuerConfig.java:
##########
@@ -315,6 +327,24 @@ public JWTIssuerConfig setAuthorizationEndpoint(String authorizationEndpoint) {
return this;
}
+ public String getTokenEndpoint() {
+ return tokenEndpoint;
+ }
+
+ public JWTIssuerConfig setTokenEndpoint(String tokenEndpoint) {
+ this.tokenEndpoint = tokenEndpoint;
+ return this;
+ }
+
+ public String getAuthorizationFlow() {
+ return authorizationFlow;
+ }
+
+ public JWTIssuerConfig setAuthorizationFlow(String authorizationFlow) {
+ this.authorizationFlow = authorizationFlow;
Review Comment:
Should we not validate the flow value early and fail if not either of the two allowed values? I'm thinking we can also let this default to `implicit` here in the backend instead of in the frontend?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #1792:
URL: https://github.com/apache/solr/pull/1792#issuecomment-1702639741
I'm going to force-push a squashed git-history in order for full test suite to run. Will preserve you as commit author..
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] laminelam commented on pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "laminelam (via GitHub)" <gi...@apache.org>.
laminelam commented on PR #1792:
URL: https://github.com/apache/solr/pull/1792#issuecomment-1699933444
> > > Eager to get this into 9.4. Let me know if we can help finalizing these two PRs
> >
> >
> > @janhoy I should push a new commit sometime today. I have changed little bit your CSP patch, I just need to add a test class for LoadAdminUiServlet.
>
> Cool. In my patch i completely disabled CSP response for all other endpoints than the `AdminUIServlet`. I cannot see a reason to return that header for other endpoints. The only I can think of is if you have a solr package that exposes another UI on some path, it could be nice to protect that UI with CSP rules, so if it would be possible to have some fallback CSP header for all other requests perhaps?
Yes I was thinking the same thing, we don't need this header for non browser targeted servlets (API endpoint, etc), so it will be just ignored by other endpoints. Now, I am not sure of a scenario where we would need a different UI servlet exposed on a different path. Let me think about it and get back to you hopefully by tomorrow.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] laminelam commented on pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "laminelam (via GitHub)" <gi...@apache.org>.
laminelam commented on PR #1792:
URL: https://github.com/apache/solr/pull/1792#issuecomment-1699940578
> * Add refguide docs
> * CHANGES entry
> * Constant naming
> * Backend part of CSP header
Will the CHANGEs entry.
Here are the main changes to the patch:
- In line with the security best practice of granting minimal necessary permissions, adjusted the _registerTokenEndpointForCsp_() method to incorporate the precise URL for the /token endpoint rather than a more general host:port based URL.
- Thought providing the capability of passing the URLs as a comma-separated list is less confusing for the end-user/operator compared to using a prefix-based system property name. This approach also enables more concise implementation.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] laminelam commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "laminelam (via GitHub)" <gi...@apache.org>.
laminelam commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1297744193
##########
solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTIssuerConfig.java:
##########
@@ -315,6 +327,24 @@ public JWTIssuerConfig setAuthorizationEndpoint(String authorizationEndpoint) {
return this;
}
+ public String getTokenEndpoint() {
+ return tokenEndpoint;
+ }
+
+ public JWTIssuerConfig setTokenEndpoint(String tokenEndpoint) {
+ this.tokenEndpoint = tokenEndpoint;
+ return this;
+ }
+
+ public String getAuthorizationFlow() {
+ return authorizationFlow;
+ }
+
+ public JWTIssuerConfig setAuthorizationFlow(String authorizationFlow) {
+ this.authorizationFlow = authorizationFlow;
Review Comment:
I've added a validation test to _setAuthorizationFlow_. The reason I am not doing it earlier is:
- I am following the same logic/style than the other methods (like _setJwksUrl_)
- Adding it earlier would lead to too much verbose code for such basic test.
I have also set _implict_ as default. Will change the front-end code too.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] laminelam commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "laminelam (via GitHub)" <gi...@apache.org>.
laminelam commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1297745407
##########
solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java:
##########
@@ -847,9 +851,11 @@ protected String generateAuthDataHeader() {
Map<String, Object> data = new HashMap<>();
data.put(
JWTIssuerConfig.PARAM_AUTHORIZATION_ENDPOINT, primaryIssuer.getAuthorizationEndpoint());
Review Comment:
Okay, I am leaving as it is both _PARAM_AUTHORIZATION_ENDPOINT_ and _PARAM_TOKEN_ENDPOINT_ for now, is that what you meant?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] rmuir commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1311497182
##########
solr/server/etc/jetty.xml:
##########
@@ -99,6 +100,7 @@
</New>
</Arg>
</Call>
Review Comment:
> @rmuir You added the CSP stuff to `jetty.xml`. Do you see a reason to keep sending the header for any and all API requests to Solr that are not from a browser? I don't.
I set these headers across the board for all responses, pure API servers too. For something doing OAuth authorization code flow I would definitely set it. I don't understand "not from a browser" as this flow usually involves one?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] laminelam commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "laminelam (via GitHub)" <gi...@apache.org>.
laminelam commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1311961728
##########
solr/CHANGES.txt:
##########
@@ -147,6 +149,8 @@ Bug Fixes
* SOLR-16916: Use of the JSON Query DSL should ignore the defType parameter
(Christina Chortaria, Max Kadel, Ryan Laddusaw, Jane Sandberg, David Smiley)
+* SOLR-16900: Add/fix wrong/missing "solr.home" to zkcli scripts (Lamine Idjeraoui. Review by Eric Pugh & Jason Gerlowski)
Review Comment:
Yes not related to this PR.
But I forgot to add a change entry of another already merged PR, so I added it here to avoid a new PR just to add a change entry.
If you think it's not good idea never mind we can remove it, it's not that important.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1310867905
##########
solr/solr-ref-guide/modules/deployment-guide/pages/jwt-authentication-plugin.adoc:
##########
@@ -85,6 +85,8 @@ jwk ; As an alternative to `jwksUrl` you may provide a static J
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
authorizationEndpoint; The URL for the Id Provider's authorization endpoint ; Auto configured if `wellKnownUrl` is provided
+tokenEndpoint; The URL for the Id Provider's token endpoint ; Auto configured if `wellKnownUrl` is provided
+authorizationFlow; Specifies the OAuth 2.0 flow to be used. Supported flows are 'implicit_flow' and 'code_pkce' (for authorization code with 'Proof Key for Code Exchange'). Note: 'implicit_flow' is deprecated and it is highly recommended to use 'code_pkce' instead. ; implicit_flow
Review Comment:
`implicit_flow` --> `implicit`?
##########
solr/server/etc/jetty.xml:
##########
@@ -99,6 +100,7 @@
</New>
</Arg>
</Call>
Review Comment:
Do we need a fallback CSP header for all API endpoints?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1296161226
##########
solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java:
##########
@@ -847,9 +851,11 @@ protected String generateAuthDataHeader() {
Map<String, Object> data = new HashMap<>();
data.put(
JWTIssuerConfig.PARAM_AUTHORIZATION_ENDPOINT, primaryIssuer.getAuthorizationEndpoint());
+ data.put(JWTIssuerConfig.PARAM_TOKEN_ENDPOINT, primaryIssuer.getTokenEndpoint());
Review Comment:
In the `X-Solr-AuthData` the keys should be "snake_case", so this should be `token_endpoint`, not `tokenEndpoint`. The frontend part will probably need to adapt to this as well.
```suggestion
data.put("token_endpoint", primaryIssuer.getTokenEndpoint());
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] laminelam closed pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "laminelam (via GitHub)" <gi...@apache.org>.
laminelam closed pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
URL: https://github.com/apache/solr/pull/1792
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1296174532
##########
solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java:
##########
@@ -847,9 +851,11 @@ protected String generateAuthDataHeader() {
Map<String, Object> data = new HashMap<>();
data.put(
JWTIssuerConfig.PARAM_AUTHORIZATION_ENDPOINT, primaryIssuer.getAuthorizationEndpoint());
Review Comment:
For this line I realize that it's been like this since 9.0, so let's not change that in this PR...
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] laminelam commented on pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "laminelam (via GitHub)" <gi...@apache.org>.
laminelam commented on PR #1792:
URL: https://github.com/apache/solr/pull/1792#issuecomment-1682979852
> Looks overall good.
>
> Please do a stab on RefGuide docs. The ref guide should encourage the use of code flow and warn that implicit flow support is deprecated.
>
> Perhaps we can also print a WARN log in backend if `implicit` is in use (either explicitly or implicitly) so that users upgrading to 9.4 without reading the release notes will at least get a warn log that they should switch.
Thank you @janhoy for the review.
I am planning to add a new entry to the ref guide.
BTW, I have added a WARN message.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1311708090
##########
solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java:
##########
@@ -60,20 +66,49 @@ public void doGet(HttpServletRequest _request, HttpServletResponse _response) th
if (in != null && cores != null) {
response.setCharacterEncoding("UTF-8");
response.setContentType("text/html");
+ String connectSrc = generateCspConnectSrc();
+ response.setHeader(
+ HttpHeaders.CONTENT_SECURITY_POLICY,
+ "default-src 'none'; base-uri 'none'; connect-src "
+ + connectSrc
+ + "; form-action 'self'; font-src 'self'; frame-ancestors 'none'; img-src 'self' data:; media-src 'self'; style-src 'self' 'unsafe-inline'; script-src 'self'; worker-src 'self';");
// We have to close this to flush OutputStreamWriter buffer
try (Writer out =
new OutputStreamWriter(
CloseShieldOutputStream.wrap(response.getOutputStream()), StandardCharsets.UTF_8)) {
- Package pack = SolrCore.class.getPackage();
String html =
new String(in.readAllBytes(), StandardCharsets.UTF_8)
- .replace("${version}", pack.getSpecificationVersion());
+ .replace("${version}", getSolrCorePackageSpecVersion());
out.write(html);
}
} else {
response.sendError(404);
}
}
}
+
+ /**
+ * Retrieves the specification version of the SolrCore package.
+ *
+ * <p>This method is primarily introduced to facilitate unit testing
+ *
+ * @return The specification version of the SolrCore class's package or Unknown if it's
+ * unavailable.
+ */
+ protected String getSolrCorePackageSpecVersion() {
Review Comment:
Precommit checks fail due to this. Change to private:
```suggestion
private String getSolrCorePackageSpecVersion() {
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #1792:
URL: https://github.com/apache/solr/pull/1792#issuecomment-1688216572
> BTW, I have added a WARN message.
Thanks, the validation and warn log looks good đź‘Ť
Will do a final review when refguide docs are in. Also remember CHANGES.txt.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #1792:
URL: https://github.com/apache/solr/pull/1792#issuecomment-1697241385
> > Eager to get this into 9.4. Let me know if we can help finalizing these two PRs
>
> @janhoy I should push a new commit sometime today. I have changed little bit your CSP patch, I just need to add a test class for LoadAdminUiServlet.
Cool. In my patch i completely disabled CSP response for all other endpoints than the `AdminUIServlet`. I cannot see a reason to return that header for other endpoints. The only I can think of is if you have a solr package that exposes another UI on some path, it could be nice to protect that UI with CSP rules, so if it would be possible to have some fallback CSP header for all other requests perhaps?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #1792:
URL: https://github.com/apache/solr/pull/1792#issuecomment-1700901263
We do have two packages "in the wild" that come with a UI, the YASA package https://github.com/yasa-org/yasa and Splainer, https://github.com/o19s/splainer/tree/main/solr-splainer-package. Will these packages need to be updated?
If we want to make this change, and that ripples down to packages, then we should do it now before more Packages are developed ;-).
We do have a test (commented out) that validates deploying Packages that might be useful: https://github.com/apache/solr/blob/main/solr/packaging/test/test_packages.bats#L69
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #1792:
URL: https://github.com/apache/solr/pull/1792#issuecomment-1697017083
Eager to get this into 9.4. Let me know if we can help finalizing these two PRs
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1311274572
##########
solr/server/etc/jetty.xml:
##########
@@ -99,6 +100,7 @@
</New>
</Arg>
</Call>
Review Comment:
@rmuir You added the CSP stuff to `jetty.xml`. Do you see a reason to keep sending the header for any and all API requests to Solr that are not from a browser? I don't.
@chatman The only other UI I can think of being served by Solr backend are those that are part of a package installed by package manager. Those UIs are 100% stand alone, can we also let it be the UI author's responsibility to secure it with proper headers? I think so.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] laminelam commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "laminelam (via GitHub)" <gi...@apache.org>.
laminelam commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1311961728
##########
solr/CHANGES.txt:
##########
@@ -147,6 +149,8 @@ Bug Fixes
* SOLR-16916: Use of the JSON Query DSL should ignore the defType parameter
(Christina Chortaria, Max Kadel, Ryan Laddusaw, Jane Sandberg, David Smiley)
+* SOLR-16900: Add/fix wrong/missing "solr.home" to zkcli scripts (Lamine Idjeraoui. Review by Eric Pugh & Jason Gerlowski)
Review Comment:
Yes not related to this PR.
But I forgot to add a change entry of another already merged PR, so I added it here to avoid a new PR just to add a change entry.
If you think it's not a good idea never mind we can remove it, it's not that important.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1311966439
##########
solr/CHANGES.txt:
##########
@@ -147,6 +149,8 @@ Bug Fixes
* SOLR-16916: Use of the JSON Query DSL should ignore the defType parameter
(Christina Chortaria, Max Kadel, Ryan Laddusaw, Jane Sandberg, David Smiley)
+* SOLR-16900: Add/fix wrong/missing "solr.home" to zkcli scripts (Lamine Idjeraoui. Review by Eric Pugh & Jason Gerlowski)
Review Comment:
Yea, remove it from this PR, and do a separate commit (and PR) for the missing changes entry, with `SOLR-16900: ..` in the commit message - that will make it ping the JIRA issue to keep track of the commit history.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1311360400
##########
solr/server/etc/jetty.xml:
##########
@@ -99,6 +100,7 @@
</New>
</Arg>
</Call>
Review Comment:
Ok, think I found a nice generic way of doing it:
```patch
diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
--- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java (revision 7f9a791eaa194001d7e50647c37535705179cc09)
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java (date 1693473677585)
@@ -23,6 +23,7 @@
import static org.apache.solr.util.tracing.TraceUtils.setTracer;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.net.HttpHeaders;
import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.util.List;
@@ -234,6 +235,11 @@
request = wrappedRequest.get();
}
+ // Set fallback CSP header. Can still be overwritten by individual Handlers
+ response.setHeader(
+ HttpHeaders.CONTENT_SECURITY_POLICY,
+ "default-src 'none'; base-uri 'none'; connect-src 'self'; form-action 'self'; font-src 'self'; frame-ancestors 'none'; img-src 'self' data:; media-src 'self'; style-src 'self' 'unsafe-inline'; script-src 'self'; worker-src 'self';");
+
var span = getSpan(request);
if (getCores().getAuthenticationPlugin() != null) {
if (log.isDebugEnabled()) {
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1311699269
##########
solr/server/etc/jetty.xml:
##########
@@ -99,6 +100,7 @@
</New>
</Arg>
</Call>
Review Comment:
Ok, turned out to be much simpler. Just leaving the `HeaderPatternRule` in `jetty.xml` worked fine. The `LoadAdminUiServlet` simply overwrite the header, and everything is just as it has been, no extra code. I pushed the fix, we can close this concern.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1311324407
##########
solr/server/etc/jetty.xml:
##########
@@ -99,6 +100,7 @@
</New>
</Arg>
</Call>
Review Comment:
If there's an easy way to provide a "fallback" if the header is not set, we could do that to avoid this discussion. I'm aware of Jetty's generic response rewrite plugins. We also have our `SolrDispatchFilter` but I find no single code point where one could manipulate response header before response is sent.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1297811474
##########
solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java:
##########
@@ -847,9 +851,11 @@ protected String generateAuthDataHeader() {
Map<String, Object> data = new HashMap<>();
data.put(
JWTIssuerConfig.PARAM_AUTHORIZATION_ENDPOINT, primaryIssuer.getAuthorizationEndpoint());
Review Comment:
Yea, let’s re-visit this later..
##########
solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java:
##########
@@ -847,9 +851,11 @@ protected String generateAuthDataHeader() {
Map<String, Object> data = new HashMap<>();
data.put(
JWTIssuerConfig.PARAM_AUTHORIZATION_ENDPOINT, primaryIssuer.getAuthorizationEndpoint());
Review Comment:
Yea, let’s re-visit this later..
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] laminelam commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "laminelam (via GitHub)" <gi...@apache.org>.
laminelam commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1297744193
##########
solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTIssuerConfig.java:
##########
@@ -315,6 +327,24 @@ public JWTIssuerConfig setAuthorizationEndpoint(String authorizationEndpoint) {
return this;
}
+ public String getTokenEndpoint() {
+ return tokenEndpoint;
+ }
+
+ public JWTIssuerConfig setTokenEndpoint(String tokenEndpoint) {
+ this.tokenEndpoint = tokenEndpoint;
+ return this;
+ }
+
+ public String getAuthorizationFlow() {
+ return authorizationFlow;
+ }
+
+ public JWTIssuerConfig setAuthorizationFlow(String authorizationFlow) {
+ this.authorizationFlow = authorizationFlow;
Review Comment:
I've added a validation test to _setAuthorizationFlow_. The reason I am not doing it earlier is:
- I am following the same logic/style than the other methods (like _setJwksUrl_)
- Adding it earlier would lead to too much verbose code for such basic test.
I have also set _implict_ as default. Will change the front code too.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] janhoy commented on pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #1792:
URL: https://github.com/apache/solr/pull/1792#issuecomment-1701134559
> We do have two packages "in the wild" that come with a UI
As stated anove, we never really needed to disable the old CSP header config in `jetty.xml`. Turns out the `LoadAdminUiServlet` simply replaces the header, which is nice.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] laminelam commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "laminelam (via GitHub)" <gi...@apache.org>.
laminelam commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1311932809
##########
solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java:
##########
@@ -60,20 +66,49 @@ public void doGet(HttpServletRequest _request, HttpServletResponse _response) th
if (in != null && cores != null) {
response.setCharacterEncoding("UTF-8");
response.setContentType("text/html");
+ String connectSrc = generateCspConnectSrc();
+ response.setHeader(
+ HttpHeaders.CONTENT_SECURITY_POLICY,
+ "default-src 'none'; base-uri 'none'; connect-src "
+ + connectSrc
+ + "; form-action 'self'; font-src 'self'; frame-ancestors 'none'; img-src 'self' data:; media-src 'self'; style-src 'self' 'unsafe-inline'; script-src 'self'; worker-src 'self';");
// We have to close this to flush OutputStreamWriter buffer
try (Writer out =
new OutputStreamWriter(
CloseShieldOutputStream.wrap(response.getOutputStream()), StandardCharsets.UTF_8)) {
- Package pack = SolrCore.class.getPackage();
String html =
new String(in.readAllBytes(), StandardCharsets.UTF_8)
- .replace("${version}", pack.getSpecificationVersion());
+ .replace("${version}", getSolrCorePackageSpecVersion());
out.write(html);
}
} else {
response.sendError(404);
}
}
}
+
+ /**
+ * Retrieves the specification version of the SolrCore package.
+ *
+ * <p>This method is primarily introduced to facilitate unit testing
+ *
+ * @return The specification version of the SolrCore class's package or Unknown if it's
+ * unavailable.
+ */
+ protected String getSolrCorePackageSpecVersion() {
Review Comment:
done
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] laminelam commented on pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "laminelam (via GitHub)" <gi...@apache.org>.
laminelam commented on PR #1792:
URL: https://github.com/apache/solr/pull/1792#issuecomment-1701423640
THANK YOU @janhoy for your extensive review and contribution.
I think the PR is now good for a merge.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] laminelam commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "laminelam (via GitHub)" <gi...@apache.org>.
laminelam commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1311961728
##########
solr/CHANGES.txt:
##########
@@ -147,6 +149,8 @@ Bug Fixes
* SOLR-16916: Use of the JSON Query DSL should ignore the defType parameter
(Christina Chortaria, Max Kadel, Ryan Laddusaw, Jane Sandberg, David Smiley)
+* SOLR-16900: Add/fix wrong/missing "solr.home" to zkcli scripts (Lamine Idjeraoui. Review by Eric Pugh & Jason Gerlowski)
Review Comment:
Yes not related to this PR.
But I forgot to add a change entry of another already merged PR, so I added it here to avoid a new PR just to add a change entry.
If you think it's not good idea never mind we can remove it.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] laminelam commented on a diff in pull request #1792: add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)
Posted by "laminelam (via GitHub)" <gi...@apache.org>.
laminelam commented on code in PR #1792:
URL: https://github.com/apache/solr/pull/1792#discussion_r1310887235
##########
solr/solr-ref-guide/modules/deployment-guide/pages/jwt-authentication-plugin.adoc:
##########
@@ -85,6 +85,8 @@ jwk ; As an alternative to `jwksUrl` you may provide a static J
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
authorizationEndpoint; The URL for the Id Provider's authorization endpoint ; Auto configured if `wellKnownUrl` is provided
+tokenEndpoint; The URL for the Id Provider's token endpoint ; Auto configured if `wellKnownUrl` is provided
+authorizationFlow; Specifies the OAuth 2.0 flow to be used. Supported flows are 'implicit_flow' and 'code_pkce' (for authorization code with 'Proof Key for Code Exchange'). Note: 'implicit_flow' is deprecated and it is highly recommended to use 'code_pkce' instead. ; implicit_flow
Review Comment:
> `implicit_flow` --> `implicit`?
Good catch.
Fixed :-)
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org