add support of OAuth 2.0/OIDC 'code with PKCE' flow (back-end)

[laminelam]

https://issues.apache.org/jira/browse/SOLR-16897

This is the "back-end" part of 'code with PKCE' 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:

• I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
• I have created a Jira issue and added the issue ID to my pull request title.
• I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
• I have developed this patch against the main branch.
• I have run ./gradlew check.
• I have added tests for my changes.
• I have added documentation for the Reference Guide

[janhoy]

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.

[laminelam]

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.

[janhoy]

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.

[janhoy]

• Add refguide docs
• CHANGES entry
• Constant naming
• Backend part of CSP header

[janhoy]

Eager to get this into 9.4. Let me know if we can help finalizing these two PRs

[laminelam]

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.

[janhoy]

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?

[laminelam]

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.), 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.

[laminelam]

• Add refguide docs
• CHANGES entry
• Constant naming
• Backend part of CSP header

Will add 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.

[epugh]

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

[janhoy]

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.

[laminelam]

THANK YOU @janhoy for your extensive review and contribution.
I think the PR is now good for a merge.

[janhoy]

I fixed the merge conflict and CHANGES entry. Think this is good to go now. Great work!

[janhoy]

I'm going to force-push a squashed git-history in order for full test suite to run. Will preserve you as commit author..