Update Flurl.Http dependency to latest version - NewtonsoftJson deprecated

[CesarD]

I noticed that this library and the original one it's forked from are using Flurl.Http for handling the underlying API calls, but since beginning of this year Flurl.Http has released a newer major version that have removed the usage of NewtonsoftJson as the default JSON serializer. This creates a problem for those of us using Flurl.Http in the rest of our applications since if we're using the latest one, then KeycloakClient tries to declare internal variables of the NSJ serializer and throws because the type doesn't exist anymore in v4+.

Can we upgrade the dependency? I know it can be a breaking change, but Flurl.Http has also released a backwards compatibility package for legacy code that can be used for those that still need NSJ; perhaps we can make Keycloak.NET do something similar: use the new STJ by default and allow some extensibility to configure the alternative NSJ so those users that still want it can use it if needed.

[sps-campbellwray]

Hi @CesarD

I would definitely be open to upgrading to the latest Flurl version, I cannot promise that this will happen any time soon, as I mostly work on this project when I have downtime. If end up fixing this so that it works with your own application then PRs are always welcome.

On the note of backwards compatibility, we upgraded from Flurl 2.x to 3.x about 12 months ago, and there were no complaints, despite there also being breaking changes there, so I don't know how much thought we need to put into backwards compatibility.

Thanks, Campbell

[CesarD]

Thanks for the fast reply!

I started working on a fork of my own and I finished the modifications, though I'm still trying to figure out how this project tries to perform the tests. If you can lend me a hand with that, I could make the PR and make sure the tests pass for you to review it and approve it. The backwards compatibility I left it aside for a bit, but it still offers the original extensibility of assigning an ISerializer implementation of your own in case someone needs to.

Please let me know your thoughts.

Thanks!

[sps-campbellwray]

Thanks for the update, I have never been able to get the tests to run myself. This project is a fork of another repo, and we are the second owners of this fork, so the test environment configuration has been lost to time.

I think that the original author had a Keycloak realm that they would run the tests against, but even when I created a realm with the same name, and attempted to create some of the structures using data from the tests the most I could get was about 70% of the tests passing.

So long as the fork is working for you then I would be comfortable making a release with your changes, the library isn't that complicated and the Keycloak API itself is fairly stable, so there isn't much that can break.

[CesarD]

Yeah, my concern was more about the changes I had to make to port the previous implementation from the JsonConverter from NewtonsoftJson to the new one for System.Text.Json. I did some cleanup that shouldn't have changed behavior, but the stuff related to the serialization has indeed changed and that could break something... I'll try to run some more tests on my side and see if I find something that breaks and if not, will get the a PR for your review.

Beware: lot of files modified mainly due to having to add named arguments for the CancellationToken because Flurl.Http v4 changed their APIs signature by adding some options param and that forces you to either include its default value or add the named argument for the cancellation token. Also all models modified for the JsonPropertyName replacement and some cleanup of mine for formatting, eliminating redundancies, etc.

[CesarD]

@sps-campbellwray Ok, there's the PR ready for your review. I tried to summarize as briefly as possible all the modifications I'm squeezing in it.

Perhaps one of the most important ones is that I'm moving it into .NET 8. This is due to some deserialization for enums that had some issues in System.Text.Json that is being introduced in .NET 9 but there was a workaround for doing it in .NET 8 but not .NET 6.

Also, I managed to make the tests work. I'm now including an export JSON file for creating the realm (called Insurance) that the tests require to run. The only additional configuration required is to use the same admin user with same credentials in both the master and the Insurance realms (user: admin / pass: admin is what is stored in the export JSON). I could verify that all tests run correctly and also removed the Skip attribute to some that were already there, but there's still 4 tests that didn't work so I kept the Skip attribute on them.

Please let me know if you have any doubt or suggestions.

[CesarD]

Hey @sps-campbellwray any estimate on when a new release of the package might be available? Just to know because I might use the new implementation in a project of mine. Thanks!

[sps-campbellwray]

@CesarD I have just released it, thanks again for your help.

I have a couple of quick questions for you:

1. When importing the Insurance realm I was getting an error:

    Uncaught server error: java.lang.RuntimeException: Script upload is disabled

   After some research the suggested fix for this seems to be to remove the offending policies, which then allowed me to import the realm:

   

   I suspect that you simply forgot to check that the file imported successfully after you had created it, but I wanted to check before I committed this change to the repo.

2. I am getting a 403 error when running the GetResourcesOwnedByClientAsync test, I wanted to check if this was the same for you, or if it was perhaps a result of me removing part of the import file.

   type="PERMISSION_TOKEN_ERROR", realmId="9d9fe4dd-4c36-404f-9d94-4548fc5d022f", realmName="Insurance", clientId="admin-cli", userId="1ab0933e-9fc4-4662-9dd7-0a80105ae251", ipAddress="172.17.0.1", error="access_denied", reason="not_authorized", auth_method="oauth_credentials", audience="insurance-product", grant_type="urn:ietf:params:oauth:grant-type:uma-ticket"

[CesarD]

Hey! Let me double check on my side. I know I used the latest version of Keycloak, but let me check and get back to you in a bit.

[sps-campbellwray]

@CesarD thanks,

FYI the Keycloak version can be found at the bottom of the export, i.e. 25.0.4.

[CesarD]

Ok, I've just pushed some corrections to the realm export file. Unfortunately, for some reason I can't finish to figure out what was the configuration required for one of the tests, the one called GetResourcesOwnedByClientAsync, which performs an authentication flow that I'm unfamiliar with (grant_type: urn:ietf:params:oauth:grant-type:uma-ticket) to, apparently, obtain the resources owned by the client. For now, I've marked it for skipping it and left a comment on the Skip reason to finish to figure out the configuration required for it.

Please let me know if you need anything else. Hope this helps :)

[CesarD]

BTW, I've tested to export it, import it into a Keycloak copy deleting the Insurance real previously and then running the tests all over again and it worked. All this with latest KC version: 25.0.6.