microsoft / kiota

OpenAPI based HTTP Client code generator
https://aka.ms/kiota/docs
MIT License
2.94k stars 205 forks source link

Optional parameter to disable SSL verification #4176

Closed thgla closed 5 months ago

thgla commented 8 months ago

It would be nice to have an optional parameter for disabling SSL verification. Currently, generating the API client from an HTTPS source with an untrusted certificate, will lead to an error:

crit: Kiota.Builder.KiotaBuilder[0]
      error generating the client: Could not download the file at https://localhost:9003/swagger/v1/swagger.json, reason: The SSL connection could not be established, see inner exception.

The workaround I've been using so far: Downloading the swagger.json manually, and then scaffolding from the file instead. This is rather inconvenient.

The optional parameter could be like this:

kiota generate --openapi https://localhost:9003/swagger/v1/swagger.json --language csharp --disable-ssl-validation

There are several reasons why this would be useful:

baywet commented 8 months ago

Hi @thgla Thanks for using kiota and for reaching out. The only place where we instantiate the http client is here https://github.com/microsoft/kiota/blob/63f9f288cd839c06d59c4bc1846e134981de38bf/src/kiota/Handlers/BaseKiotaCommandHandler.cs#L35

With a custom SSL validator, we should be able to suppress errors for localhost, 127.0.0.1, [::1]

Is this something you'd be willing to submit a pull request for?

thgla commented 8 months ago

I can take a stab at this, might take a bit though. But after having glanced at the code base now, this doesn't seem too difficult

baywet commented 7 months ago

@thgla is this still something you're planning to work on?

thgla commented 7 months ago

Yeah, unless someone else will do it - I'm just a bit busy at the moment with other projects

rohitkrsoni commented 6 months ago

Hi, I would like to work on this issue, can I get this assigned and have some insights of where to start from, thanks

baywet commented 6 months ago

Hi @rohitkrsoni Thanks for volunteering! You should have all the details you need in this answer Let us know if you have additional questions.

rohitkrsoni commented 6 months ago

Thanks for assigning this issue to me, I am very willing and excited to contribute to this project. I cloned the repo and tried to run the project in VS but I am getting error in the command line.

image

I followed Microsoft documentation to setup my local provided here https://learn.microsoft.com/en-us/openapi/kiota/install?tabs=bash#build-from-source and https://github.com/microsoft/kiota/blob/main/CONTRIBUTING.md (All tests passed)

I tried using the bash command and run the kiota.exe manually but a cmd window opened and closed instantly Is there something I am missing?

One more thing (This might be a silly one) For some reason I can only see few files in VS Solution Explorer, probably because I am using solution view and I need to change to folder view?

image

Thanks

andrueastman commented 6 months ago

Hey @rohitkrsoni,

Looks like the launch config for Visual Studio isn't setup correctly on the main branch and still uses configuration of an older version of the library. (This has been setup though for VS code ).

What you would probably need to do is update the launch settings here https://github.com/microsoft/kiota/blob/b721efd15eea96f39d3544d1385b7f9030c18b21/src/kiota/Properties/launchSettings.json#L5

To be something like this so that launch runs correctly. (It would be great if you included the fix in the PR as well)

"generate --openapi https://raw.githubusercontent.com/microsoftgraph/msgraph-sdk-powershell/dev/openApiDocs/v1.0/Mail.yml -o {output path for generated files} -c GraphClient --log-level Information -l CSharp"

One more thing (This might be a silly one) For some reason I can only see few files in VS Solution Explorer, probably because I am using solution view and I need to change to folder view?

Most of the files should be under the kiota and Kiota.Builder projects. So, if you expand those you should see them in a fashion similar the folder structure.

rohitkrsoni commented 6 months ago

Thanks @andrueastman, for the clarification

Team, I have added a PR for the change, here is the link https://github.com/microsoft/kiota/pull/4617. Please review it and let me know if any other changes are required, thanks

darrelmiller commented 6 months ago

Hey folks, can you help me understand the requirements for this feature a little bit better. For context, security is an even more sensitive issue these days and a feature that disables SSL validation seems like an opportunity for something to go wrong.

I've seen two scenarios discussed so far where this feature might help:

For the first scenario, I would have thought the two easier solutions would be

For the second case, I don't really understand how it would happen that a company would set up an internal proxy that doesn't have a valid SSL configuration. That would make using a web browser within the company pretty much impossible. I've seen internal proxies use http on the local network and then HTTPS outside, if they don't have their own internal CA, but I don't understand how someone could setup internal SSL without a valid cert.

I'm fully prepared to accept I am misinformed. I just would like to get more clarification on this feature before we ship a security "foot gun". Thanks.

/cc @rohitkrsoni @thgla

thgla commented 6 months ago

@darrelmiller: In my mentioned scenario, the localhost API is using a self-signed certificate, but these aren't trusted by default. So I would have to establish a trust first before I can scaffold, which is possible.. but then I might as well just do the workaround I mentioned initally by downloading the swagger.json manually. The other scenario is not as likely I agree, but I have been in a situtation where I wanted to scaffold something while in a WSL/Ubuntu environment, which doesn't share the same trust store as the host Windows system. Bypassing validation entirely is admittedly not the best solution, it's preferable to just establish trust correctly, but for these one-off actions where I know I'm on the company network anyway it doesn't feel necessary to me.

So my thinking was that it's reasonable to assume a developer who explicitly adds --disable-ssl-verification understands the implications and risks of that, but just wants to "get it done" without needing to deal with workarounds or trying to establish a trusted chain. It's not that uncommon to see options for disabling SSL validation in developer tools, Git has one as well (git -c http.sslVerify=false ...)

baywet commented 6 months ago

@thgla Thanks for the additional information. In that scenario of talking from WSL to host, or vice versa, is the callback URL still localhost? (or a loopback address)

darrelmiller commented 6 months ago

I prefer the explicitness of the flag over a the hidden effect of an environment variable.

baywet commented 6 months ago

alright, trying to drive this to conclusion so we can do something with the PR here. @darrelmiller was the scenario from @thgla and the demonstration with git convincing enough for you to accept that change?

darrelmiller commented 5 months ago

@baywet Yeah, sorry, I should have been more explicit. The fact that git has this option was probably the most convincing argument for me. I am ok with us implementing it as a flag.

thgla commented 5 months ago

Awesome, thanks to @rohitkrsoni for the implementation!