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

Generated Openapi / Swagger does not contain Authorization definitions #5977

Open
4 tasks done
Syndlex opened this issue Dec 15, 2022 · 3 comments
Open
4 tasks done
Assignees
Labels
technical debt Not necessarily broken, but could be done better/cleaner

Comments

@Syndlex
Copy link

Syndlex commented Dec 15, 2022

Summary

We used your generated Openapi documentation to generate our own client.

The Problem here is that you Openapi spec does not contain the Authorization definitions. For securitySchemes and api_key headers.

Steps to Reproduce

  1. Download your Open api Spec from https://github.com/TheThingsNetwork/lorawan-stack/blob/v3.23/api/api.swagger.json
  2. Load the spec into https://editor.swagger.io/
  3. generate client with editor
  4. clients are unable to mount authorization components

Current Result

the client is not able to call your api with the authorization.
Since the generated code is restricted it is not possible to go around this bug on a call by call basis.

Expected Result

possibility to use the authorization on every backendcall

Deployment

The Things Stack Community Edition

Proposed Fix

Change the generator to include the autorizations

This is describte within this issue grpc-ecosystem/grpc-gateway#309

https://github.com/grpc-ecosystem/grpc-gateway/blob/main/examples/internal/proto/examplepb/a_bit_of_everything.proto#L612
According to this example every RPC definition that needs a Authorization has to have a option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation)

I don't know grpc well enough here. Is it Possible to write this definition somewhere and reuse this Option?
It would still be a lot of manual Labor but a possible Solution to add the authorization definition to every endpoint that needs it.

Contributing

  • I can help by doing more research.
  • I can help by implementing a fix after the proposal above is approved.
  • I can help by testing the fix before it's released.

Code of Conduct

@Syndlex Syndlex added the needs/triage We still need to triage this label Dec 15, 2022
@NicolasMrad NicolasMrad added this to the 2023 Q1 milestone Dec 20, 2022
@NicolasMrad NicolasMrad added technical debt Not necessarily broken, but could be done better/cleaner and removed needs/triage We still need to triage this labels Dec 20, 2022
@johanstokking
Copy link
Member

Thanks @Syndlex for filing and for the pointer to declaring this.

We are currently working on a big refactor to get rid of gogoproto and use vanilla protobuf instead. The GRPC gateway is definitely part of this overhaul. We may be able to add these options easily, but as lots of things are moving on the proto side, I rather wait until we got rid of gogoproto. References #2798.

Just for my information; what client are you generating? Does this really stop you from using a client?

@Syndlex
Copy link
Author

Syndlex commented Dec 20, 2022

We are generating a Java client with the Apache-Http Libary as a Setting.
But the problem we are facing will include every possible client.

For people searching for a solution with self generated code:
It is possible to use the generated code by giving one API Key to the apache-Http-Client itself as a default header for all communication.

The Problem is that we build a multi-tanant system. Every call will have different API-key.
Without the mapping in the openapi-spec it is not possible to pass a key to the generated api.
We currently just use half of the generated API and build the API Keys into our definitions.

Using the default header Method would be a security nightmare.
I think your openapi and grpc approach is a create possibility for a ploygloth-api. :)

There are a lot of API Calls within the spec that are not documented on your homepage. At least I could not find them there. :)

Thanks for the Information @johanstokking

@johanstokking johanstokking added the blocked This can't continue until another issue or pull request is done label Dec 21, 2022
@johanstokking
Copy link
Member

I see. Yeah I suppose most generated clients will allow you to pass custom headers. I hope this is not blocking users, but I understand the annoyance.

Blocked on #2798

@NicolasMrad NicolasMrad modified the milestones: 2023 Q1, 2023 Q2 Apr 13, 2023
@NicolasMrad NicolasMrad removed the blocked This can't continue until another issue or pull request is done label Apr 13, 2023
@johanstokking johanstokking removed this from the 2023 Q2 milestone Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
technical debt Not necessarily broken, but could be done better/cleaner
Projects
None yet
Development

No branches or pull requests

3 participants