Closed NewtTheWolf closed 1 month ago
Hello @NewtTheWolf,
this is a very nice proposal. This helped me a lot as I needed an OpenApi spec for my API implemented with rocket using zitadel.
While this allows the usage it seems to not work without problems. I am unable to use the generated swagger documentation to login. Is this a problem you are aware of or does it work for you? If the later is the case it may be a problem of my zitadel configuration or project setup.
The other problem is that a typescript client generated from a spec made with this using the https://github.com/OpenAPITools/openapi-generator tool does not send the authorization header by default. If I add the access_token manually to the request headers everything works perfectly. This may also be a problem with the generation or my usage of it. Do you use something similar and can confirm that it works?
I would also propose to implement the second method get_responses
from the OpenApiFromRequest
trait.
This allows you to add additional statuses to what can be returned by an endpoint requiring this request guard.
A 401 Unauthorized
sounds like a addition that makes sense on every request with this guard.
But there may also be others that make sense like a bad request if no authorization header is being send.
Hello @FrTerstappen,
Is this a problem you are aware of or does it work for you?
My primary focus for my own project was to continue development without needing to create a New Type
, so it hasn’t been thoroughly tested yet. I'll make it a priority to test this issue and try to fix it as soon as possible.
Do you use something similar and can confirm that it works?
I haven't encountered this use case yet, but I will test it and try to reproduce the issue. If I can replicate it, I’ll work on a fix.
I would also propose to implement the second method get_responses from the OpenApiFromRequest trait.
I hadn’t thought of that—great idea! Implementing additional status codes, like a 401 Unauthorized or a bad request when no authorization header is sent, definitely makes sense.
My biggest challenge right now is understanding how to properly use the configuration that this client is intended for. This client is based on Code Config
, meaning that you configure it within your code instead of using an external configuration file like a .env
or a Rocket.toml
. Do you have any ideas on how to solve the configuration problem by any chance?
I’m considering adding two additional features, "rapidoc" and "swagger", to the project. The idea is that only one of these features would be active at a time:
Would this approach make sense, given the specific requirements for each tool? Any feedback or suggestions would be appreciated!
@FrTerstappen
Is this a problem you are aware of, or does it work for you?
I tested it today, and the login worked without any issues. You'll need to add the following to your Rocket.toml
:
[default]
authority = "https://auth.yourzitadel.de"
Also, make sure to add the redirect URL to http://127.0.0.1:8000/swagger/oauth2-redirect.html
. The /swagger
part and the port should be adjusted based on your specific configuration.
Just created GREsau/okapi#151 to ask for help regarding configuration access within the OpenApiFromRequest implementation in rocket_okapi.
@NewtTheWolf
Thank you for your feedback. I will take a deeper look at your changes and test them on Monday. But a first look at the changes looks good to me.
Now that you have confirmed that the login on the swagger docs should work I will invest some time to find out why it did not work for me. But seeing your configuration for the redirect url and having my doc mounted under a different path this may already be the problem.
Also, make sure to add the redirect URL to
http://127.0.0.1:8000/swagger/oauth2-redirect.html
. The/swagger
part and the port should be adjusted based on your specific configuration.
I'm not sure I understand what you want to use the two proposes features for. The decision to use rapidoc or swagger is made by the consumer of this library and there is no setup for included here.
Or do you want to adjust the schema for the login in the docs based on which is activated?
As I only use swagger at the moment I have not seen the need for adjustments but it seems like a sensible approach to have these two features to support the different authentication modes.
But if this is the case I would propose calling them like the authentication modes as that is a better description of what they do.
Something like rocket_okapi_oicd
which would include the rocket_okapi
feature.
I also do not know how to solve the problem of configuration.
It seems to be a use case not really supported by the okapi
crate.
As such it should be the right place to ask for a solution like you have done.
The current solution of specifying the authority in the Rocket.toml file works good enough for me. As it is a file I already have for other configuration that gets adjusted on deployment it is a nice fit for my use case.
@FrTerstappen
Thanks for your feedback, keep me updated!
I'm not sure I understand what you want to use the two proposed features for.
The main difference (in docs) between OAuth2 and OIDC is that OIDC does not work in Rapidoc and vice versa. My idea is to allow activating the feature that best suits your preferred documentation. In case you configure both docs in your Rocket app, you wouldn't be able to have both activated in zitadel-rust
. This is why I think it makes sense to have these features configurable.
The current solution of specifying the authority in the Rocket.toml file
I think it would also be a great idea to support a general function to configure the client, like with .with_config()
, because the Rocket.toml file is the intended place to configure your Rocket application. You could then use keys like oauth2.zitadel(.authority, client_id, client_secret, etc....)
.
What do others think about this approach?
@NewtTheWolf
Adjusting the redirect URL helped.
Having features for the different authentication schemes or for the different viewer for the OpenAPI spec (swagger, rapidoc, ...) seems like good idea. But this would need to be shown prominently in the documentation as it will lead to people using a non working combination of authentication and documentation viewer if not communicated.
I also tried your latest changes and while the additional response types show up I realized that you also specified a schema for the response. Does this match what is returned by the guard in an error case? The errors also look overly specific.
Bad Request - Multiple authorization headers found. Unauthorized - The request requires user authentication.
Looking at the errors used by the guard it seem like many errors are possible.
custom_error! {
/// Error type for guard related errors.
pub IntrospectionGuardError
MissingConfig = "no introspection config given to rocket managed state",
Unauthorized = "no HTTP authorization header found",
InvalidHeader = "authorization header is invalid",
WrongScheme = "Authorization header is not a bearer token",
Introspection{source: IntrospectionError} = "introspection returned an error: {source}",
Inactive = "access token is inactive",
NoUserId = "introspection result contained no user id",
}
I'm not sure if the best way is to add them all or to just specify a more generic version of the error, e.g. only Bad Request
The errors also do not reflect all status codes used by the guard.
For example in https://github.com/NewtTheWolf/zitadel-rust/blob/feature/rocket-okapi/src/rocket/introspection/guard.rs#L141 the guard return an internal server error.
I'm not sure what is best practice and how this is generally handled in rocket_okapi
but I feel like all codes used in the guard should be reflected in the possible responses.
@FrTerstappen,
I’m glad to hear that adjusting the redirect URL helped!
Regarding the error handling, I agree that generalizing the Bad Request
error makes sense.
I was thinking of proposing adding a struct
with JsonSchema
for errors definition. This could simplify the spec management and reduce boilerplate. What do you think of this approach? Do you think it might be too much?
I’ll handle the additional features in a separate pull request. For now, my focus will be on Swagger users since that’s what I primarily use.
@NewtTheWolf
It was actually changing the redirect URL in combination with adding a key to my app in zitadel. I'm setting up zitadel from the application when it gets deployed. The api call to add the frontend app gives me something that looks like the secret and was enough for my frontend to work. Making an extra request to generate a new key finally gave me something that also works in swagger. I think my problems come mostly from my zitadel setup and has little to do with swagger or the generation of the documentation.
I think having a struct with JsonSchema
for the errors sounds like a good idea.
That is also what I do in the rest of my project.
I think focusing on swagger and doing everything else in a follow up PR sounds like a good idea to move forward. It may even be worth it to remove the additional responses again and also add them in a separate PR later as this is probably a decision that the maintainer(s) need to make.
@buehler
At the moment, this is the best solution we can achieve. I believe this pull request is ready for a final review. I will keep track of the Issue on okapi
Everything else, like an error struct for doc generation, the generic 403 response, and Rapidoc, I would implement in separate pull requests.
I'll change the status to "Ready for Review" after a quick check to make sure the code is clean.
Hi @NewtTheWolf This seems to be reasonable :)
Draft: Add Optional Support for Rocket-Okapi in
IntrospectedUser
Overview
This pull request introduces the initial implementation of optional Rocket-Okapi support for the
IntrospectedUser
type within thezitadel-rust
project. The goal is to enable automated OpenAPI documentation generation for Rocket-based applications using this type.558
Current Implementation
I have added preliminary code to integrate Rocket-Okapi, which is currently functional but still requires further review and enhancements. Here are the specific changes made:
rocket_okapi
as an optional dependency.OpenApiFromRequest
forIntrospectedUser
, including security scheme setups and default responses.Status
The current implementation works as intended, enabling the
IntrospectedUser
to be utilized within the OpenAPI framework provided by Rocket-Okapi. However, this is still a draft, and the implementation is open for review and suggestions on improvements.Next Steps
Invitation for Feedback
I am actively seeking feedback on the current implementation and suggestions on how we can further refine this feature. Any input on additional aspects to consider, potential issues, or improvements would be greatly appreciated.
Thank you for reviewing this draft pull request. I look forward to your suggestions and contributions to finalize this feature.