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

Support bearerFormat option in security schemes #1848

Closed
kelnos opened this issue Mar 22, 2025 · 10 comments
Closed

Support bearerFormat option in security schemes #1848

kelnos opened this issue Mar 22, 2025 · 10 comments
Labels
feature 🚀 New feature or request

Comments

@kelnos
Copy link
Contributor

kelnos commented Mar 22, 2025

Description

For the http security scheme, with type bearer, there is an optional property bearerFormat that specifies the token that goes first in the Authorization header. This defaults to Bearer, but openapi-ts doesn't support setting this to something else.

I started work to add support for this, but I'm having trouble figuring out how to test my changes. How can I run openap-ts, pointing it at my local copy of client-fetch? I've tried passing the full path to the package in the --client option, as well as installing my local copy of client-fetch into node_modules and just specifying --client client-fetch, but neither worked.

@kelnos kelnos added the feature 🚀 New feature or request label Mar 22, 2025
@mrlubos
Copy link
Member

mrlubos commented Mar 22, 2025

I'd need to see what you're doing but I'm afraid you might be trying to execute a custom client which is not currently supported. Why not modify the existing client package? You only need to build it and then add a new test in openapi-ts for that client. You should be able to see how the other supported schemes are tested and do something similar. Happy to chat more if you need guidance

@mrlubos
Copy link
Member

mrlubos commented Mar 22, 2025

Here's a similar test for OpenAPI 3.1 for example

https://github.com/hey-api/openapi-ts/blob/main/packages/openapi-ts/test/3.1.x.test.ts#L515-L528

@kelnos
Copy link
Contributor Author

kelnos commented Mar 22, 2025

you might be trying to execute a custom client which is not currently supported. Why not modify the existing client package?

I was indeed modifying the existing client package, in packages/client-fetch. But I wasn't sure how to get openapi-ts to use that modified copy.

But I'll take a look at the tests and see about doing it that way. I guess I don't actually have to run the generator and examine the generated code; I just need to write a test for it and verify it works?

@mrlubos
Copy link
Member

mrlubos commented Mar 22, 2025

I always inspect the generated snapshot to verify it's doing what I want. There are a few parts to this feature:

  1. Ensure we parse the bearerFormat keyword. If not, relevant parsers need to be updated to process this keyword. I don't know off the top of my head which OpenAPI versions support it.
  2. Once we have the format parsed, ensure it's included in the generated SDK. We'll likely want to include it only if it's anything but the default/bearer so it doesn't modify the output otherwise.
  3. Steps 1 and 2 will be covered with a snapshot test similar to what I shared earlier. We need to have a snapshot for each supported OpenAPI version (1-3 tests total).
  4. Now that we have an updated SDK with the new field, we want our clients to use it. You'll most likely want to update getAuthToken() function https://github.com/hey-api/openapi-ts/blob/main/packages/client-core/src/auth.ts
  5. Step 4 will be covered with a unit test, there are other examples in the core package (1-2 tests total).
  6. Final step is to verify all clients support the new field. As long as they all import the core package and depend on getAuthToken(), they will. If not, we need to repeat steps 4 and 5 for each client.

Feel free to open a draft if you get stuck on anything and I can point you to the relevant parts.

@mrlubos
Copy link
Member

mrlubos commented Mar 22, 2025

In terms of building, I'm not sure if I have the dependencies configured with Turborepo, but basically if you modify the core package, it needs to be built, then the client needs to be built, and finally openapi-ts. That's how you get the core package change to the generated output.

@kelnos
Copy link
Contributor Author

kelnos commented Mar 22, 2025

Ok, I think #1849 is what we're looking for here? Please let me know what you think...

kelnos added a commit to kelnos/openapi-ts that referenced this issue Mar 22, 2025
@mrlubos
Copy link
Member

mrlubos commented Mar 22, 2025

For the http security scheme, with type bearer, there is an optional property bearerFormat that specifies the token that goes first in the Authorization header.

Let's take a step back on this actually. What's your source for this information? The spec says the following:

A hint to the client to identify how the bearer token is formatted. Bearer tokens are usually generated by an authorization server, so this information is primarily for documentation purposes.

It sounds like we have a different understanding here. I guess you're trying to implement a custom format?

@kelnos
Copy link
Contributor Author

kelnos commented Mar 22, 2025

Ah wow, yes, I completely misunderstood what bearerFormat is for. It looks like there is no way to specify anything other than the HTTP auth schemes registered with IANA, then.

Although, I guess the docs for scheme say:

The name of the HTTP Authorization scheme to be used in the Authorization header as defined in [RFC7235] Section 5.1. The values used SHOULD be registered in the IANA Authentication Scheme registry.

... which I suppose means you can put anything for scheme if you really want to, though it's not clear if all openapi client/server code generators will allow it, if they've decided to be strict about it.

Anyhow, sorry for the noise, I'll close this issue and the PR. have another one coming that adds cookie support to apiKey, so I'll get that one ready to submit instead.

@kelnos kelnos closed this as not planned Won't fix, can't repro, duplicate, stale Mar 22, 2025
@mrlubos
Copy link
Member

mrlubos commented Mar 22, 2025

Yeah. I guess the question is if this is a requirement for you? How are you generating your spec? If your API requires custom auth, let's get this done. I was going to suggest using scheme too, SHOULD leaves the interpretation to tooling and, well, we would support it. It seems pretty straightforward too, you'd want to handle any other value in the same function you modified in the pull request.

@kelnos
Copy link
Contributor Author

kelnos commented Mar 22, 2025

My API doesn't require it, no. I'm designing something new, so I can do it however I'd like, and this method (at first glance) seemed simple and reasonable. But after this discussion, I feel like it's a bit of an "abuse" of the Authorization header, so I'm fine implementing it in a different way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🚀 New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants