Skip to content
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.

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

build(deps): Replace SAML Library #2908

Merged
merged 220 commits into from
Nov 26, 2024
Merged

build(deps): Replace SAML Library #2908

merged 220 commits into from
Nov 26, 2024

Conversation

Tallicia
Copy link
Contributor

@Tallicia Tallicia commented May 30, 2024

Replacing the other feature branch #2862 for new SAML library replacement effort.

Sonar
https://sonarcloud.io/summary/new_code?id=cloudfoundry-identity-parent&pullRequest=2908

@Tallicia Tallicia added in progress DO NOT MERGE Internal Test or WIP, please DO NOT MERGE labels May 30, 2024
@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/187710947

The labels on this github issue will be updated when the story is started.

@Tallicia Tallicia changed the title New saml 0530 New SAML 2024.05.30 - Not to merge but just for SAML feature branch testing May 30, 2024
@duanemay duanemay force-pushed the new-saml-0530 branch 5 times, most recently from 65b0d64 to e67a40a Compare June 14, 2024 22:11
@duanemay duanemay force-pushed the new-saml-0530 branch 9 times, most recently from a761b67 to 7de27a1 Compare June 24, 2024 15:54
@duanemay duanemay force-pushed the new-saml-0530 branch 2 times, most recently from 0d3a595 to f199f50 Compare July 5, 2024 22:17
@duanemay duanemay force-pushed the new-saml-0530 branch 2 times, most recently from 46248b9 to b6cb65b Compare July 9, 2024 18:49
@peterhaochen47 peterhaochen47 force-pushed the new-saml-0530 branch 2 times, most recently from a97457f to 745fff3 Compare July 10, 2024 17:04
@strehle strehle added accepted Accepted the issue and removed in_review The PR is currently in review DO NOT MERGE Internal Test or WIP, please DO NOT MERGE labels Nov 20, 2024
Copy link
Member

@strehle strehle left a comment

Choose a reason for hiding this comment

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

After a long time LGTM

  • thanks to broadcom
  • manually tested SAML flows

@strehle strehle requested a review from a team November 20, 2024 09:23
dependabot bot and others added 7 commits November 20, 2024 16:50
Bumps commons-io:commons-io from 2.17.0 to 2.18.0.

---
updated-dependencies:
- dependency-name: commons-io:commons-io
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
- Modified `cargo.local` to run with jacoco agent if a system property is set.
- Added a task to generate coverage report from the recorded jacoco data.
…en the configured entityID is a URL)

- maintain the existing behavior where a custom identity zone's saml entityID is defaulted
to either 1) `zoneSubdomain.uaaWideSamlEntityID` if `uaaWideSamlEntityID` is not a URL, or
2) if `uaaWideSamlEntityID` is a URL, integration the zoneSubdomain into the URL
(see tests for example).

- similar logic for saml entity alias (which is used in various saml sp urls, such as `AssertionConsumerService`)
except that the alias should not include url scheme (aka without `https://`), so that
the resulting saml sp urls are valid urls (e.g.: `https://zone1.uaa.com/saml/SSO/alias/[saml entity alias]`,
see tests for examples).

- reference on develop branch (old saml code):
  - doc: https://github.com/cloudfoundry/uaa/blob/65952b1b53b8d01cf93e68493a3f6ac85ad8a825/docs/login/Okta-README.md?plain=1#L73-L75
  - code: https://github.com/cloudfoundry/uaa/blob/cc5f76fba495e5d1b3fd755ac3a6ff137fc91878/server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/ZoneAwareMetadataGenerator.java#L53-L54

- problem statement:
without this commit, when
* a custom zone is created without a `zone.config.samlConfig.entityID` specified
* the default zone's `login.entityID` is configured to a URL, such as `https://uaa.com`
* the default zone's `login.saml.entityIDAlias` is not set, aka default to `login.entityID`
Then the resulting custom zone sp metadata has some discrepancies with the old saml
code's metadata:

For `AssertionConsumerService`:
- old (correct) value is: https://test-zone-before.uaa.com/saml/SSO/alias/test-zone-before.uaa.com
- new value is: https://test-zone.uaa.com/saml/SSO/alias/test-zone.http:/uaa.com
For `entityID`:
- old (correct) value is: http://test-zone-before.uaa.com
- new value is: test-zone.http://uaa.com

This results in the external SAML login for this zone not working.
…entityid-url-form

fix: default values of custom zone's saml entityID and saml alias (wh…
@strehle
Copy link
Member

strehle commented Nov 22, 2024

@duanemay pipeline runs after resolving merge conflicts.

Copy link
Member

@duanemay duanemay left a comment

Choose a reason for hiding this comment

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

🎉

@duanemay duanemay changed the title build(deps): bump opensaml2 from 2.6.6 to 4.0.1 build(deps): Replace SAML Library Nov 25, 2024
@strehle strehle merged commit 208a340 into develop Nov 26, 2024
22 checks passed
@strehle strehle deleted the new-saml-0530 branch November 26, 2024 21:26
@strehle strehle linked an issue Dec 5, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Accepted the issue scheduled
Projects
Development

Successfully merging this pull request may close these issues.

Spring Security SAML2 End of Life Wrong attributeMappings in SAML?
10 participants