ory / elements

Ory Elements is a component library that makes building login, registration and account pages for Ory a breeze. Check out the components library on Chromatic https://www.chromatic.com/library?appId=63b58e306cfd32348fa48d50
https://ory.sh
Apache License 2.0
85 stars 44 forks source link

fix: unflatten form data into correct body format #104

Open DASPRiD opened 1 year ago

DASPRiD commented 1 year ago

The format for bodies with traits dictates that the traits come in through their own object. This becomes especially visible when using an SDK client compiled with a newer version of OpenAPI generator, as they only process properties which are defined in the API schema, which is not the case for flattened properties.

Fixes #93

Checklist

Benehiko commented 1 year ago

Still not sure why we need to unflatten data when the data is unflattened in Kratos.

Please take a look at some of the examples. These examples all work perfectly fine against Ory and are tested.

https://github.com/ory/elements/blob/main/examples/nextjs-spa/src/pages/login.tsx#L88-L102

Are you getting errors when trying Ory Elements against Ory Kratos? Could you please give me more information on what exactly isn't working.

DASPRiD commented 1 year ago

It is working fine at the moment, because the currently generated SDK does not verify the input parameters.

When a newer generator is used (which is planned, see https://github.com/ory/sdk/pull/256#issuecomment-1473892227), the input parameters are actually checked.

Alternatively, the API schema could be updated for these endpoint to allow additionalProperties: true, in which case unspecified flattened parameters would then be passed through by a newer SDK.

Benehiko commented 1 year ago

I see, thank you for the link and clarification. Could you rebase your branch so we can have e2e tests run on this branch?

I will then review and see to merge this :)

DASPRiD commented 1 year ago

Sure, I'll take care of that later tonight. Though what's your opinion on that manual unflattening? It does feel a little bit hacky, we could alternatively use the npm library flat to take care of generic unflattening.

It could look something like this then:

            const body = unflatten<unknown, UpdateBody>(
                Object.fromEntries(formData)
            );
Benehiko commented 1 year ago

I will need to look into the implementation a bit, but yes I agree that using a library might be better if the library is well tested and maintained.

aeneasr commented 1 year ago

@Benehiko please revisit

KimGressens commented 4 weeks ago

I was having the same issue but when looking at the request being sent by the browser (when posting the registration form) I discovered that it never send out the traits.email properties.

Thus the request was:

{
  "csrf_token" : "...",
  "method": "password",
  "password": "..."
}

instead of

{
  "csrf_token" : "...",
  "method": "password",
  "password": "...",
  "traits.email": "..."
}

The reason traits.email gets stripped is due to the use of TypeScript (I think the other comments also use TS). The client library from Ory ( @ory/kratos-client-fetch ) validates the request and a traits object is supported but other keys such as traits.email are removed before sending the request.

As a workaround I'm going to implement an unflatten of the keys to make it fit the TypeScript definitions (as the linked PR is doing) but it's understandable that the maintainers might never accept that PR.

The best option is probably to fix the openapi spec that generates the @ory/kratos-client-fetch library but I'm not knowledgeable enough to know if it's even fixable on that level.