feat(swagger-ui): add support for oauth config #134

jgramoll · 2022-05-09T15:50:32Z

@juhaku not sure what you had in mind for supporting this. Welcome to any feedback

jgramoll · 2022-05-09T15:51:58Z

Fixes #131

juhaku

Yeah this is pretty much something I planned actually add. Later I will refactor this a bit and add the swagger-ui other configuration options as well but that is another PR then.

Overall pretty good. Some minor things to check through though :)

juhaku · 2022-05-09T16:56:11Z

utoipa-swagger-ui/src/oauth.rs

+const END_MARKER: &str = "//</editor-fold>";
+
+// https://github.com/swagger-api/swagger-ui/blob/master/docs/usage/oauth2.md
+#[derive(Default, Clone, Debug, Serialize)]


The Debug trait could be placed under debug feature flag to reduce possible clutter for people who does not wish to have that there by default.

Suggested change

#[derive(Default, Clone, Debug, Serialize)]

#[derive(Default, Clone, Serialize)]

#[cfg(feature = "debug", derive(Debug))]

juhaku · 2022-05-09T16:59:05Z

utoipa-swagger-ui/src/lib.rs

@@ -467,6 +502,7 @@ async fn serve_swagger_ui(path: web::Path<String>, data: web::Data<Config<'_>>)
 pub struct Config<'a> {
    /// [`Url`]s the Swagger UI is serving.
    urls: Vec<Url<'a>>,
+    oauth: Option<oauth::Config>,


This could field could have a short doc comment on it.

juhaku · 2022-05-09T17:12:24Z

utoipa-swagger-ui/src/lib.rs

+            .app_data(Data::new(Config {
+                urls: urls,
+                oauth: self.oauth,
+            }))


This block should also be added for rocket framework. Look at line 309 :)

juhaku · 2022-05-09T20:47:29Z

utoipa-swagger-ui/src/oauth.rs

+// https://github.com/swagger-api/swagger-ui/blob/master/docs/usage/oauth2.md
+#[derive(Default, Clone, Debug, Serialize)]
+#[serde(rename_all = "camelCase")]
+pub struct Config {


And this should also have a proper doc comment along with doc comments on the options.
You can see the docs with cargo doc --workspace --no-deps --features serde_json,json,actix-web,rocket,actix_extras and browse to the docs in targer folder e.g. file:///home/me/development/utoipa/target/doc/utoipa/index.html.

juhaku · 2022-05-09T21:05:50Z

utoipa-swagger-ui/src/lib.rs

@@ -481,6 +517,7 @@ impl<'a> Config<'a> {
    pub fn new<I: IntoIterator<Item = U>, U: Into<Url<'a>>>(urls: I) -> Self {
        Self {
            urls: urls.into_iter().map(|url| url.into()).collect(),
+            oauth: None,
        }
    }


I would add here a method with_oauth_config something like this.

pub fn with_oauth_config<I: IntoIterator<Item = U>, U: Into<Url<'a>>>(urls: I, oauth_config: oauth::Config) -> Self { Self { urls: urls.into_iter().map(|url| url.into()).collect(), oauth: Some(oauth_config), } }

This Config type is non_exhaustive for backwards compatibility purposes

Outside of the defining crate, types annotated with non_exhaustive have limitations that preserve backwards compatibility when new fields or variants are added.

Non-exhaustive types cannot be constructed outside of the defining crate:

Non-exhaustive variants (struct or enum variant) cannot be constructed with a StructExpression (including with functional update syntax).
enum instances can be constructed.
https://doc.rust-lang.org/reference/attributes/type_system.html#the-non_exhaustive-attribute

It is in Rust a good idea to define outside looking API with non_exhaustive and instead provide constructors new() or / and accessor fields or builder pattern to construct objects. This way the API implementator is at control and not every change to the API nor adding / changing / deleting one field will be a breaking change for API consumer.

juhaku · 2022-05-09T21:06:05Z

utoipa-swagger-ui/src/oauth.rs

@@ -0,0 +1,81 @@
+use std::collections::HashMap;


This could also have some module level comments like

//! Implements Swagger UI [oauth configuration](https://github.com/swagger-api/swagger-ui/blob/master/docs/usage/oauth2.md) options.

jgramoll

Thanks for the great feedback

jgramoll · 2022-05-10T15:19:54Z

utoipa-swagger-ui/src/lib.rs

+            .app_data(Data::new(Config {
+                urls: urls,
+                oauth: self.oauth,
+            }))


jgramoll · 2022-05-10T15:28:00Z

utoipa-swagger-ui/src/oauth.rs

@@ -0,0 +1,81 @@
+use std::collections::HashMap;


jgramoll · 2022-05-10T15:28:14Z

utoipa-swagger-ui/src/lib.rs

@@ -467,6 +502,7 @@ async fn serve_swagger_ui(path: web::Path<String>, data: web::Data<Config<'_>>)
 pub struct Config<'a> {
    /// [`Url`]s the Swagger UI is serving.
    urls: Vec<Url<'a>>,
+    oauth: Option<oauth::Config>,


jgramoll · 2022-05-10T15:28:26Z

utoipa-swagger-ui/src/lib.rs

@@ -481,6 +517,7 @@ impl<'a> Config<'a> {
    pub fn new<I: IntoIterator<Item = U>, U: Into<Url<'a>>>(urls: I) -> Self {
        Self {
            urls: urls.into_iter().map(|url| url.into()).collect(),
+            oauth: None,
        }
    }


jgramoll · 2022-05-10T15:28:39Z

utoipa-swagger-ui/src/oauth.rs

+const END_MARKER: &str = "//</editor-fold>";
+
+// https://github.com/swagger-api/swagger-ui/blob/master/docs/usage/oauth2.md
+#[derive(Default, Clone, Debug, Serialize)]


jgramoll · 2022-05-10T15:31:25Z

utoipa-swagger-ui/src/oauth.rs

+// https://github.com/swagger-api/swagger-ui/blob/master/docs/usage/oauth2.md
+#[derive(Default, Clone, Debug, Serialize)]
+#[serde(rename_all = "camelCase")]
+pub struct Config {


juhaku · 2022-05-10T16:40:48Z

Thanks for the great feedback

Welcome you can try running tests locally with scripts/test.sh utoipa-swagger-ui to get started with. And further use the commands found from that file to run specific tests if needed as well.

juhaku

Super 🚀

feat(swagger-ui): add support for oauth config

3780f4b

fix format_swagger_config_oauth test to unwrap result

a1fe982

juhaku requested changes May 9, 2022

View reviewed changes

juhaku linked an issue May 9, 2022 that may be closed by this pull request

Support pkce with oauth2 #131

Closed

jgramoll commented May 10, 2022

View reviewed changes

doc(swagger-ui): add more oauth docs

f5984ee

jgramoll force-pushed the swagger-ui-oauth branch from b286f72 to f5984ee Compare May 10, 2022 16:41

juhaku approved these changes May 10, 2022

View reviewed changes

juhaku merged commit 5cfd77f into juhaku:master May 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(swagger-ui): add support for oauth config #134

feat(swagger-ui): add support for oauth config #134

jgramoll commented May 9, 2022

jgramoll commented May 9, 2022

juhaku left a comment

juhaku May 9, 2022

jgramoll May 10, 2022

juhaku May 9, 2022

jgramoll May 10, 2022

juhaku May 9, 2022

jgramoll May 10, 2022

juhaku May 9, 2022

jgramoll May 10, 2022

juhaku May 9, 2022

jgramoll May 10, 2022

juhaku May 9, 2022

jgramoll May 10, 2022

jgramoll left a comment

jgramoll May 10, 2022

jgramoll May 10, 2022

jgramoll May 10, 2022

jgramoll May 10, 2022

jgramoll May 10, 2022

jgramoll May 10, 2022

juhaku commented May 10, 2022

juhaku left a comment

	#[derive(Default, Clone, Debug, Serialize)]
	#[derive(Default, Clone, Serialize)]
	#[cfg(feature = "debug", derive(Debug))]

feat(swagger-ui): add support for oauth config #134

feat(swagger-ui): add support for oauth config #134

Conversation

jgramoll commented May 9, 2022

jgramoll commented May 9, 2022

juhaku left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgramoll left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juhaku commented May 10, 2022

juhaku left a comment

Choose a reason for hiding this comment