Skip to content

[3.0] Adjust minimum versions for security library #1043

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

[3.0] Adjust minimum versions for security library #1043

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d469bea
adjust minimum version
StefanHenke Jun 21, 2024
53cd9a2
Update migration.md
StefanHenke Jun 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions java/migration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,12 @@ This is a **preview** of the changes planned for CAP Java 3.0 (planned to be rel

CAP Java 3.0 increased some minimum required versions:

| Dependency | Minimum Version |
| --- | --- |
| @sap/cds-dk | ^7 |
| Maven | 3.6.3 |
| Cloud SDK | 5.9.0 |
| Dependency | Minimum Version |
|--------------------------------| --- |
| @sap/cds-dk | ^7 |
| Maven | 3.6.3 |
| SAP Cloud SDK | 5.10.0 |
| SAP BTP Spring Security Client | 3.5.1 |
Copy link
Contributor

@beckermarc beckermarc Jun 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this mainly because of this TokenClaims constant that we are using now? :D
If yes we might want to rethink that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No :) 3.5.0 is not properly doing the auto-configuration for the proof-token check in hybrid scenarios where xsuaa and identity bindings are used. Also a CAP application would incorrectly enforce the proof-token check in App2App scenarios.
Both issues has been fixed with 3.5.1.

Cloud SDK 5.10.0 we could discuss. I changed it mainly due to the fact that CAP 2.10.0 is already compiling against 5.10.0. It includes two fixes for zero-trust which we will support soon. So, we wouldn´t need to increase minimum version again.

Copy link
Contributor

@beckermarc beckermarc Jun 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think those are two different aspects. For me a minimum required version means: "Minimum version for successful compilation of the code". That a newer version of some library contains bug fixes (in internal code) for some scenarios I think doesn't really justify a new minimum version in my opinion. That is rather a new "recommended version". As for example people using plain XSUAA are totally fine with sticking to an older version and are not required to update now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For 3.5.1 you could say it is about bug fixes. But at least 3.5.0 includes the whole proof token check feature. If we stick to minimum version 3 of the security library, it could be interpreted as a kind of a security issue if people are using IAS with < 3.5.0. Applications wouldn´t even recognize that incoming certificates are not validated. On the other hand you can say it is in the responsible of the applications to update dependencies regularly once available.

If we don´t increase the minimum version, I at least have to revert the code change to use the new constant in TokenClaims as this would lead to runtime exceptions in IAS scenarios.

Copy link
Contributor

@beckermarc beckermarc Jun 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should document then rather with the IAS-related features that >3.5.1 is recommended/required. My main reason why I am a little hesitant here is that 95% of our current stakeholders don't use IAS as of today (obviously). For those we would (unnecessarily) increase the minimum version. This might also include stakeholders that rely on SAP Java Buildpack, which might be slow to update to 3.5.1.

Here I don't see a good reason why we need to force the newest version on our stakeholders. Not saying, that I wouldn't always recommend using the latest version anyway. If they don't manage the XSUAA version themselves CAP will anyway take care of bringing along the 3.5.1 version (if not downgraded by buildpack or so).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. Let´s do the following. I added a comment to the migration guide as part of PR #1047 that for proof of possession, 3.5.1 is needed. So, we can close this PR.


CAP Java 3.0 no longer supports @sap/cds-dk ^6.

Expand Down