hey-api / openapi-ts

✨ Turn your OpenAPI specification into a beautiful TypeScript client
https://heyapi.vercel.app
Other
994 stars 82 forks source link

use security scheme configuration #231

Open swurzinger opened 5 months ago

swurzinger commented 5 months ago

Describe the problem

OpenAPI allows to specify authentication/authorization details in so-called "security schemes" for each request. Examples are Basic Auth, Bearer Auth, API Keys, OAuth2 or OIDC. see https://swagger.io/docs/specification/authentication/

openapi-ts currently seems to ignore the specified authentication methods and currently always sends user/password (Basic Auth) or token (Bearer Auth), no matter whether the API supports the respective authentication type or whether the API requires authentication at all. It uses a global configuration which is used for every request, even if it doesn't require authentication.

For security reasons one might want to avoid sending the user/password (Basic Auth) or token (Bearer Auth) to every endpoint of the API.

Describe the proposed solution

Ideally openapi-ts should respect the security scheme configuration specified in the API specification and only use the authentication types which are specified for the respective request.

Alternatives considered

A possible alternativ solution is to use a custom interceptor, that knows about the authentication schemes and adds the authentication data, instead of using the global OpenAPIConfig provided by openapi-ts. Downside: authentication scheme mapping needs to maintained separately from OpenAPI spec

Importance

potential security issue; nice to have

Additional information

No response

jordanshatford commented 5 months ago

@swurzinger thanks for creating this issue. I was actually just thinking about this a couple days ago. Would be a great feature.

Note: we also have a ticket about passing schema to interceptors. That would allow for your alternative solution. However, I think we should properly support it internally.

swurzinger commented 5 months ago

Note: we also have a ticket about passing schema to interceptors. That would allow for your alternative solution. However, I think we should properly support it internally.

In general that's fine I guess, as authentication can be highly customized in many real-world applications. In my specific case that wouldn't currently work due to the missing request interceptor support for angular clients, though. Is it expected to use HttpInterceptor there?

jordanshatford commented 5 months ago

@swurzinger Yes, you would likely want to use that. I would like to provide better first class support, but I need to do more research into angular itself.

mrlubos commented 5 months ago

Thanks for the issue @swurzinger and as mentioned above, if you can provide any pointers on Angular clients, that'd be great, as neither one of us is familiar with that ecosystem at the moment

WhySoBad commented 2 months ago

Hey, I'd love to see a proper way to add authentication. In my mind it would be great if we could have an interceptor for auth which provides the name of the security scheme and the string returned from the interceptor is then placed in the right spot.

I think it fair to view the authentication types as two groups of types:

  1. The OIDC and OAuth types have to be executed and afterwards the token can be extracted without any user interaction
  2. The http and apiKey types rely on a token/key provided by the user and put them somewhere (cookie, header, query, etc.)

Having an interceptor for auth would support the 2nd group of types (http and apiKey) whilst the authentication for the other types could be implemented in another turn to run fully internally.

Let me know what you think of this idea

mrlubos commented 2 months ago

@WhySoBad sounds like there's something to it, can you show an example API how this would work?

WhySoBad commented 2 months ago

I'm imagining an API similar to the request interceptor described here https://heyapi.vercel.app/openapi-ts/interceptors.html

import { client } from '@hey-api/client-fetch';

client.interceptors.auth.use((scheme, request, options) => {
  if (scheme === "BasicAuth") {
    // get basic auth credentials and return them as a string
  } else if (scheme === "Bearer") {
    // get bearer token and return it as a string
  }
  return null;
});

When returning null or any other falsly value the request can be cancelled on the client side since it would fail anyways (as by the spec the endpoint needs auth).

mrlubos commented 2 months ago

@WhySoBad so I understand, you'd want to automate the credentials management?

WhySoBad commented 2 months ago

Uh I don't really know what you mean by credentials management. This should be a hook which is called before any request which has an authentication scheme registered.

mrlubos commented 2 months ago

Is it painful to write this manually yourself?

WhySoBad commented 2 months ago

Do you mean to implement it myself for this library or to have authentication without such a hook? For the 2nd I have to write down all urls which don't need authentication and check in the request interceptor whether the request is going to one of those locations.

mrlubos commented 2 months ago

Gotcha. That makes sense, thanks for the context!

swurzinger commented 2 months ago

I agree in several points:

For Angular there's request and response metadata. For this project's custom interceptors (which aren't supported for Angular) adding a new parameter to the signature as suggested by @WhySoBad is probably fine, although I wouldn't put it first and wrap it in options. I'm not sure whether it would make sense for the library to provide default implementations for e.g. BasicAuth or Bearer Auth as they are trivial to implement (just set a header) and the token acquisition/refresh logic needs a customized implementation anyway.

theogravity commented 1 month ago

Some also use a custom Authorization scheme type as well:

https://stackoverflow.com/questions/59694733/how-to-define-the-authorization-header-with-a-custom-prefix-in-openapi-3