p2-inc / keycloak-orgs

Single realm, multi-tenancy for SaaS apps
https://phasetwo.io
Other
389 stars 66 forks source link

Organization creation not matching the API docs #47

Closed hisorange closed 1 year ago

hisorange commented 1 year ago

Hey there,

Was playing around with the organization creation when I found 2 issue, but gonna merge them as it all related.

  1. The API docs describes the organization creation expects an id property, but this is ignored and a new one generated here

  2. Is kinda the same, the docs says the request will return with the created organization's JSON representation, but it responds with a redirection code to the newly created resource. Which is fine, just makes the process a bit finicky as either the client has to follow redirects, or parse the response's headers for the ID.

Luckily the location header allows me to parse the created organization's ID, but I think either correcting the docs, or implementing a "PUT /realms/%/org/:orgId" could solve the issue.

Anyways, nothing serious and the plugin is awesome, have a great day! ^.^

xgp commented 1 year ago

Thanks. I think it's possible to update the docs to make the id not required in the request object. We use OpenAPI https://github.com/p2-inc/phasetwo-docs/blob/master/openapi.yaml for everything, and generate the API docs from there, so I need to figure out how to do it there.

the docs says the request will return with the created organization's JSON representation

Can you show me where it says that? I can't find it.

it responds with a redirection code to the newly created resource

We decided to make this consistent with how Keycloak responds to resource creation methods.

hisorange commented 1 year ago

Haha never mind me. You are right.

Screenshot 2023-03-12 at 13 37 15

The UI misled my thoughts, when I was looking at the screen it shows the request body on the left (purple) as a table, and then my brain assumed the right side is the output (orange).

But upon reading the openApi code I seen my mistake and double checked the UI, and there is a clear "Responses" section (green).

At first I sent a UUID and realised that the id does not match the id I sent, this is when I started to dig in.

I will send a PR to patch the openApi's id claim as it will be discarded in the process, and that's kinda solves this issue.