10227: RelayState max length not respected (fix2)#13757
10227: RelayState max length not respected (fix2)#13757hmlnarik merged 3 commits intokeycloak:mainfrom
Conversation
|
Closes #10227 RelayState max length not respected |
|
@ahus1: Can you approve running the workflows? |
|
Hi @evtr, thanks for this PR. Two things I note here:
Please let me know if you can think of a way to write such a test without Mockito, or if you need help writing such a SAML test case. |
|
The objects that are mocked in the tests mocks very extensive interfaces. I can create TestClientModel and TestRealmModel classes but there are many methods to override. Is that ok? |
|
@ahus1: I changed the unit tests to not using mockito |
|
@ahus1: I don't really understand what is failing here? Can you help? |
|
Hi @evtr - failing Quickstart tests are unrelated to your changes. I'll figure out what needs to be done there. I'll also reach out to some maintainers in a separate channel to find out whether to go with the manually created mocks, or with a SAML integration tests. |
|
My thoughts one the SAML integration tests. Perhaps I shouldn't create new tests but choose one of the "sunshine" scenarios that are already in the test suite and add an assertion on the relaystate length somehow? |
|
@hmlnarik - this PR adds a class IdentityBrokerStateTestHelpers.java to avoid using a mocking framework like Mockito as I know that adding such dependencies is discouraged. Could you please provide a hint if this is the way to go, or if the contributor should go for an Arquillian based integration test. Thanks! |
Closes #10227 RelayState max length not respected