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

New saml 0530 registration resolver #3135

Closed
wants to merge 215 commits into from

Conversation

strehle
Copy link
Member

@strehle strehle commented Nov 15, 2024

No description provided.

swalchemist and others added 30 commits July 12, 2024 10:44
Co-authored-by: Peter Chen <[email protected]>
Co-authored-by: Bruce Ricard <[email protected]>
Co-authored-by: Danny Faught <[email protected]>
* Instead of calling fail(). We have a suspicion that there is a bug in
  the way the tests are running (most of them are somehow not running
  with "./gradlew test" and we have a theory that a combination of mixing
  junit4 imports and the junit5 fail() might be contributing.
* I was careful to use @ignore for tests importing the junit4 @test, and
  @disabled for tests using the junit5 @test.
* These annotations were added, with the idea that you can search for
  '@ignore("SAML' and '@disabled("SAML' to find the tests that need
  attention before we finish the SAML library conversion.
@ignore("SAML test fails")
@ignore("SAML test doesn't compile")
@ignore("SAML test setup doesn't compile")
@disabled("SAML test fails")
@disabled("SAML test doesn't compile")
* A few tests are set to ignore because they're failing for the right
  reasons, but more work is needed to finish that and get back to green.
  The goal is to start tracking these annotations instead of failing
  tests, so we can stay green.
* Tests now running:
    server module: 3,435 (in IntelliJ) (98 total ignored)
    uaa module: 67 (command line run of "./gradlew test" for all tests
    - still needs troubleshooting)

Co-authored-by: Danny Faught <[email protected]>
* Removed commented-out references to the outdated SAML extension library

Co-authored-by: Duane May <[email protected]>
- Adds back endpoint and incorporates forwarding for new pattern saml2 endpoints, Still has some wip elements WithHttpsNotRequired > samlMetadataReturnsOk still red RelyingPartyRegistration is hardcoded in xml, /saml/metadata/ with trailing slash not working missing parity with develop

[#186986697]

Co-authored-by: Peter Chen <[email protected]>
…NotRequired -> samlMetadataReturnsOk is green

- fixed one test but still WithHttpsRequired > samlMetadataReturnsOk is red after fixing this test -
HealthzShouldNotBeProtectedMockMvcTests > WithHttpsRequired > samlMetadataRedirects() FAILED
    java.lang.AssertionError: Range for response status value 200 expected:<REDIRECTION> but was:<SUCCESSFUL>

[#186986697]

Co-authored-by: Duane May <[email protected]>
Co-authored-by: Peter Chen <[email protected]>
Co-authored-by: Bruce Ricard <[email protected]>
Co-authored-by: Danny Faught <[email protected]>
* Instead of calling fail(). We have a suspicion that there is a bug in
  the way the tests are running (most of them are somehow not running
  with "./gradlew test" and we have a theory that a combination of mixing
  junit4 imports and the junit5 fail() might be contributing.
* I was careful to use @ignore for tests importing the junit4 @test, and
  @disabled for tests using the junit5 @test.
* These annotations were added, with the idea that you can search for
  '@ignore("SAML' and '@disabled("SAML' to find the tests that need
  attention before we finish the SAML library conversion.
@ignore("SAML test fails")
@ignore("SAML test doesn't compile")
@ignore("SAML test setup doesn't compile")
@disabled("SAML test fails")
@disabled("SAML test doesn't compile")
* A few tests are set to ignore because they're failing for the right
  reasons, but more work is needed to finish that and get back to green.
  The goal is to start tracking these annotations instead of failing
  tests, so we can stay green.
* Tests now running:
    server module: 3,435 (in IntelliJ) (98 total ignored)
    uaa module: 67 (command line run of "./gradlew test" for all tests
    - still needs troubleshooting)

Co-authored-by: Danny Faught <[email protected]>
- Adds back endpoint and incorporates forwarding for new pattern saml2 endpoints, Still has some wip elements WithHttpsNotRequired > samlMetadataReturnsOk still red RelyingPartyRegistration is hardcoded in xml, /saml/metadata/ with trailing slash not working missing parity with develop

[#186986697]

Co-authored-by: Peter Chen <[email protected]>
…NotRequired -> samlMetadataReturnsOk is green

- fixed one test but still WithHttpsRequired > samlMetadataReturnsOk is red after fixing this test -
HealthzShouldNotBeProtectedMockMvcTests > WithHttpsRequired > samlMetadataRedirects() FAILED
    java.lang.AssertionError: Range for response status value 200 expected:<REDIRECTION> but was:<SUCCESSFUL>

[#186986697]

Co-authored-by: Peter Chen <[email protected]>
- With the new SAML lib, SAML SP metadata generation relies on a relyingPartyRegistration,
which requires a valid SAML IDP
metadata. In the context of UAA external SAML IDP login, UAA does not know what the SAML IDP
metadata is, until the operator adds it via the /identity-providers endpoint. Also, some SAML
IDPs might require you to supply the SAML SP metadata first before you can obtain the
SAML IDP metadata. See relevant issue: spring-projects/spring-security#11369
- Previously, to solve this problem, the SAML SP metadata generation relies
on relyingPartyRegistration values in saml-providers.xml, which
hardcodes a SAML IDP metadata URL (point to some example Okta SAML instance);
this means that UAA's SP metadata generation relies on the
example Okta SAML instance to be running.
- This commit, instead, supplies a hardcoded dummy SAML IDP metadata here to unblock the SAML
SP metadata generation, at the advice of Spring Security team, so that UAA's functioning
does not rely on some external running Okta instance.
- code reference: https://github.com/spring-projects/spring-security-samples/blob/1b28351693d60f01a511cbcc18b64590452a3851/servlet/java-configuration/saml2/login/src/main/java/example/SecurityConfiguration.java#L62

[#186986697]

Co-authored-by: Peter Chen <[email protected]>
- A continuation of 65d1f0f
- This test is failing as early as
  e7beec7 due to the removal of SAML
  code, as this test is related the SAML feature

[#186986697]

Co-authored-by: Peter Chen <[email protected]>
* Has to be commented out of the erb file even when the test method used @disabled.

Co-authored-by: Peter Chen <[email protected]>
- A continuation of 65d1f0f
- This is a test recently added to develop branch, so
ignoring this here because the SAML feature is still being
built.

[#186986697]

Co-authored-by: Peter Chen <[email protected]>
- to reflect the fact that this IDP metadata just needs
to exist in its bare minimal form, where the specific fields
in it do not affect the SP metadata generation

[#186986697]

Co-authored-by: Peter Chen <[email protected]>
- previously some tests error with:
```
net.shibboleth.utilities.java.support.xml.XMLParserException: Unable to parse inputstream, it contained invalid XML
```
- this issue is fixed once we switch to loading
the idp saml metadata via a file (instead of an InputStream)

[186822654]

Co-authored-by: Danny Faught <[email protected]>
* We're reprioritizing the test to get this test to pass.

Co-authored-by: Bruce Ricard <[email protected]>
Co-authored-by: Duane May <[email protected]>
Co-authored-by: Duane May <[email protected]>
…ect request

- Tests are failing but they are behaving as expected with curl and browser for /saml/metadata /saml/metadata/example and /saml/metadata/example/

- /saml/metadata/ is not returning xml

- The dispatcher ordering along with position in the filter-mapping must be set properly.

[#186986697]

Co-authored-by: Bruce Ricard <[email protected]>
…VC Tests

- /saml/metadata/ is not returning xml

[#186986697]

Co-authored-by: Filip Hanik <[email protected]>
Co-authored-by: Alicia Yingling <[email protected]>
Co-authored-by: Duane May <[email protected]>
* Must be changed back to /saml/metadata later, removing "example".

Co-authored-by: Alicia Yingling <[email protected]>
Co-authored-by: Duane May <[email protected]>
Co-authored-by: Alicia Yingling <[email protected]>
Co-authored-by: Duane May <[email protected]>
…ault

- Updated to use direct GetMapping

[#186986697]

Co-authored-by: Filip Hanik <[email protected]>
…pulated in SPSSODescriptor

- Building out EntityDescriptor in the RelyingPartyRegistration which contains the SPSSODescriptor picked up by the resolve method

[#186986697]

Co-authored-by: Duane May <[email protected]>
- Need to fix credential type being empty

Caused by: java.lang.IllegalArgumentException: credentials types cannot be empty
....(SamlRelyingPartyRegistrationRepository.java:84)
[#186986697]

Co-authored-by: Duane May <[email protected]>
@strehle strehle added the DO NOT MERGE Internal Test or WIP, please DO NOT MERGE label Nov 15, 2024
strehle and others added 11 commits November 15, 2024 11:32
* Test saml bearer

* Fixes for SAML2 bearer flow

* reverted test
…0-registration-resolver

# Conflicts:
#	server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/ConfiguratorRelyingPartyRegistrationRepository.java
#	server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/Saml2BearerGrantAuthenticationConverter.java
and create the data based on real key data

Until now we do not deliver any keys in uaa.war.
…stration-resolver

# Conflicts:
#	dependencies.gradle
Changed, because of hack with defaults.
Signed-off-by: Duane May <[email protected]>
@strehle strehle closed this Nov 16, 2024
@strehle strehle deleted the new-saml-0530-registration-resolver branch November 16, 2024 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE Internal Test or WIP, please DO NOT MERGE
Projects
Development

Successfully merging this pull request may close these issues.

9 participants