fschmtt / keycloak-rest-api-client-php

PHP client to interact with Keycloak's Admin REST API.
MIT License
24 stars 18 forks source link

Create resource fails with error #94

Open jaapio opened 7 months ago

jaapio commented 7 months ago

We try to setup a new realm using this library. However this fails because the Resource representation api doesn't accept the id field when creating a resource. We get the error:

Client error: POST http://.../admin/realms/.../clients resulted in a 400 Bad Request response:\n
{"error":"Unrecognized field "id" (class org.keycloak.representations.idm.authorization.ResourceRepresentation), not m (truncated...).

Suggestion is to filter the id field when it's not set. So on fetch the object will contain an id, but when posting a new resource to create one it will not error. I saw that a feature exists to filter properties with a certain attribute. Would this be a useful approach for the resource id as well? So the code would look like this:

class Resource extends Representation
{
    public function __construct(
        #[OmmitNull]
        protected ?string $id,
//....

Implementing it this way would allow us to reuse the behavior on other fields of representations, in the same way.

This issue relates to #93

fschmtt commented 7 months ago

Hey, first of all, thanks for contributing!

If the Resource representation differs between Keycloak versions, there are both the Since and Until (PHP) attributes that can be used for filtering.

In the Keycloak docs there's an optional id parameter in v23 and v24.

Could be that the docs are wrong, though, but would be nice to have an integration test case that verifies the error.

fschmtt commented 7 months ago

I just looked it up in the source, it seems like the JSON property name is _id (prefixed with an underscore):

public class ResourceRepresentation {

    @JsonProperty("_id")
    private String id;
}

https://github.com/keycloak/keycloak/blob/3fffc5182ebf93225b3da40fce5d945e04155cdf/core/src/main/java/org/keycloak/representations/idm/authorization/ResourceRepresentation.java#L43-L44

Need to implement a solution to handle this πŸ˜… I think it will break with the current logic, because it doesn't "support" underscores in property names.

jaapio commented 7 months ago

What if we would add an attribute with an alternative name? Would that work? So we can translate on serialize and deserialize?

fschmtt commented 7 months ago

Yes, that would be an option I guess.

I currently don't have much spare time, so I can't look into it too soon πŸ™ˆ

Maybe we could built something similar and do e.g.

class Resource extends Representation
{
    public function __construct(
        #[JsonProperty(name: '_id')]
        protected ?string $id,
    )
}

I don't know if there are any existing PHP libraries that can already doo this πŸ€”

jaapio commented 7 months ago

The best one I know and use very often: https://symfony.com/doc/current/components/serializer.html

It's independent of the framework, so can be used standalone. Highly configurable and well maintained. The downside is, there is a lot of complexity, so we might run into issues that are harder to solve rather than having a simple implementation in this project

fschmtt commented 7 months ago

Agree, I would like to keep it as simple as possible πŸ‘

fschmtt commented 6 months ago

Hey, just to let you know, it's still on my radar, but I didn't get to work on that issue (yet) πŸ˜…

jaapio commented 6 months ago

No problem, we have a workaround for now. If I find the time we can try to contribute a fix. But also here time is limited

fschmtt commented 6 months ago

Maybe https://github.com/jolicode/automapper could be an option πŸ‘€

fschmtt commented 4 months ago

Hey, I've been playing around a little bit with the Symfony Serializer here: https://github.com/fschmtt/keycloak-rest-api-client-php/pull/116

Using the SerializedName attribute obviously works (for the Resource representation), so this would solve this issue.

I had to create a custom CollectionDenormalizer in order to normalize all collection classes, which also looks like it's working out so far πŸ˜„

Next I would need to add a custom normalizer (or encoder? πŸ€”) to handle the Since and Until attributes.

Feel free to leave your thoughts 😊