fschick / Keycloak.RestApiClient

Keycloak Admin REST API client (.NET)
52 stars 18 forks source link

Adding User to Org fails #11

Open JonasDev17 opened 2 weeks ago

JonasDev17 commented 2 weeks ago

When trying to add an existing user to an org with the function PostOrganizationsMembersByIdAsync, I always get the error that the user does not exist. I checked whether the endpoint works by looking at the network tab in my browser when using the Keycloak UI and I tried it using Postman and both work but something's different when using this client.

What I suspect is that the body is sent differently than how Keycloak expects it. Keycloak expects just an Id in the body but the type has to be application/json (basically invalid json)

image

image

JonasDev17 commented 2 weeks ago

I've done some more debugging but it does not look like the body is sent in a wrong format. The OpenAPI client takes the string can converts it to bytes (utf-8): httpRequestMessage.Content = new StringContent(customJsonCodec.Serialize(options.Data), new UTF8Encoding(), "application/json"); I did decode the bytes back to string and got the Guid I sent. Not sure whats wrong...

JonasDev17 commented 2 weeks ago

Ok I figured it out, the RestApiClient adds double quotes at the start and end of the Id.

fschick commented 2 weeks ago

Do you mean somethings like this { "Id": "bc724c54-618f-4455-9f79-382425b051c8", ... }? This is valid JSON, isn't it?

JonasDev17 commented 2 weeks ago

Do you mean somethings like this { "Id": "bc724c54-618f-4455-9f79-382425b051c8", ... }? This is valid JSON, isn't it?

No, the RestApiClient sends it as:

"bc724c54-618f-4455-9f79-382425b051c8"

while it should be sent as:

bc724c54-618f-4455-9f79-382425b051c8

I figured this out by debugging:

image

this is the binary content that is sent and when decoding it results in:

image

and when removing the 34 at the beginning and end, it results in the id without the quotes.

I then took those bytes without the quotes and converted them to a binary file and sent that binary file using postman and it worked.

fschick commented 2 weeks ago

Can you tell me where the Id is wrong? Is it a URL / query parameter? Than it should be without quotes.

Your code above httpRequestMessage.Content = new StringContent(customJsonCodec.Serialize(options.Data), new UTF8Encoding(), "application/json"); lets me assume it's for a body with POST/PUT/... In this case, the quotes are required.

Without, it's invalid JSON

{ "Id": bc724c54-618f-4455-9f79-382425b051c8 }

With, it's valid JSON

{ "Id": "bc724c54-618f-4455-9f79-382425b051c8" }
fschick commented 2 weeks ago

Can you post some steps to reproduce easily?

JonasDev17 commented 2 weeks ago

image

This is how Keycloak expects it. No json object or anything

JonasDev17 commented 2 weeks ago

The red underline is because it literally is invalid json but that's the way they made it...

fschick commented 2 weeks ago

It looks like a bug in Keycloak.

Either they should change the definition to match the expected content type from application/json to text/plain:

{
  "...": "...",
  "paths": {
    "/admin/realms/{realm}/organizations/{id}/members": {
      "get": {
        "...": "...",
      },
      "post": {
        "tags": ["Organizations"],
        "summary": "Adds the user with the specified id as a member of the organization",
        "description": "Adds, or associates, an existing user with the organization. If no user is found, or if it is already associated with the organization, an error response is returned",
        "requestBody": {
          "content": {
            "application/json": {
              "schema": {
                "type": "string"
              }
            }
          }
        }
      }
    }
  }
}

Or they should accept a JSON-encoded string like "95f019a8-977f-b7d5-f781-c1b2a3ef288c"

JonasDev17 commented 2 weeks ago

Ok, thanks I'll write a bug report over at their repo.

fschick commented 1 week ago

I think I closed the error prematurely. Waiting for a fix from Keycloak.

fschick commented 1 week ago

keycloak/keycloak#34512

JonasDev17 commented 1 week ago

The PR with the fix was merged. The OpenAPI definition won't change - the fix was to strip any double quotes for the affected endpoints.

fschick commented 1 week ago

Thank you for the info.

There are still other issues too like keycloak/keycloak#34512 and a few I fixed earlier. I'd like to review them and switch the Build to Nuke to be able to fix such shortcomings faster. Maybe I manage to add some tests too.

The top-notch solution would be if Keycloak had tests to compare the swagger.json with theris REST API. However, my Java skills are not sufficient for a pull request here.

JonasDev17 commented 1 week ago

Thank you for the info.

There are still other issues too like keycloak/keycloak#34512 and a few I fixed earlier. I'd like to review them and switch the Build to Nuke to be able to fix such shortcomings faster. Maybe I manage to add some tests too.

The top-notch solution would be if Keycloak had tests to compare the swagger.json with theris REST API. However, my Java skills are not sufficient for a pull request here.

The problem is they don't want any breaking changes which is understandable but thats why the problem has been solved the way it has but they said the following:

For the future we will discuss and pick more carefully the right content-type.