Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing optional params after updating to 10.9.3+ #514

Closed
Zehir opened this issue Aug 19, 2023 · 7 comments
Closed

Missing optional params after updating to 10.9.3+ #514

Zehir opened this issue Aug 19, 2023 · 7 comments

Comments

@Zehir
Copy link
Contributor

Zehir commented Aug 19, 2023

Hello,

I was on 10.9.2 with not issue and I get this issue on 10.9.3 and 10.9.4.

I have this definition :

export const globalParameters = makeParametersObject({
  // The API Key is optional because it's will be loaded by the API Key plugin
  apiKey: {
    name: 'apiKey',
    description: 'API Key',
    type: 'Path',
    schema: z.string().optional(),
  },
})

(The API Key plugin if needed can be found here)

  makeEndpoint({
    alias: 'getConfig',
    description: 'Get gateway configuration',
    method: 'get',
    path: '/api/:apiKey/config',
    response: z.toLongForGithubStuff()
    parameters: [
      globalParameters.apiKey,
    ],
  }),

With version 10.9.2 the getConfig signature was :

const config = await client.getConfig()

image

But with both version 10.9.3 and 10.9.4 I get this error :
image

Typescript is happy only if I provide some apiKey param but it's should be optional here.

      const config = await client.client.getConfig({
        params: {
          apiKey: '',
        },
      })

I don't from where is the string | number type coming from. During my tests sometime it's was string | number | boolean. When installing back version 10.9.2 the issue is gone and typescript display the old correct signature.

If you need to test my setup you can use my repo from this commit.
The call is located here and the definition here and here.

@ecyrbe
Copy link
Owner

ecyrbe commented Aug 20, 2023

path parameters can't be optional, that's why you get this error. before that we did not force it.

@Zehir
Copy link
Contributor Author

Zehir commented Aug 20, 2023

Is there a workaround ? The rest api I use have optional path parameters (yes I know...)
Here I want /api//config when apiKey is undefined

@ecyrbe
Copy link
Owner

ecyrbe commented Aug 20, 2023

Here you where lying to zodios by saying it was optional and hacked into injecting it with a plugin, i could allow undefined back.

But hear me just a second.

API keys should not leak in the URL, for the reason that URL are usually logged in reverse proxies like nginx etc, it's a security issue and bad practice.
The better approach is to put it as a header parameter if you have control over the backend.
with header, your api key will not leak, or you can even silence it with reverse proxy.

I encourage you to change the design of your API, but if it's not doable, i'll add support for optional path parameters

@Zehir
Copy link
Contributor Author

Zehir commented Aug 20, 2023

It's even worse that you think, the API key is sent with http but I can't control it and it's on a local network so it's almost "secure"

https://www.sitebase.be/generate-phillips-hue-api-token/
https://developers.meethue.com/develop/hue-api/7-configuration-api/#get-configuration (need free account)

I can't control the API I use so I have no choice to use what I have

@ecyrbe
Copy link
Owner

ecyrbe commented Aug 20, 2023

ok, i'll test adding optional back.

@ecyrbe
Copy link
Owner

ecyrbe commented Aug 20, 2023

@Zehir version v10.9.5 is out and should fix your issue. can you confirm this is the case ?

@Zehir
Copy link
Contributor Author

Zehir commented Aug 20, 2023

@ecyrbe It's working thanks
image

@Zehir Zehir closed this as completed Aug 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants