feat: enable swagger-ui token #267

jannyHou · 2019-09-09T19:52:16Z

The spike PR for loopbackio/loopback-next#2027
Details are explained in the readme file.

hacksparrow · 2019-09-10T05:27:30Z

With the limitations and the next plans explained in the docs, this is looking good. Excellent job @jannyHou 👍

bajtos

Great work, @jannyHou. I especially like the screenshot you are adding to README 👍

I'd like to discuss the implementation details though, please see my second comment below.

bajtos · 2019-09-10T09:26:06Z

README.md

+`swagger-ui` module is built with authorization component, which will show up by
+adding the security schema and operation security spec in the OpenAPI spec.
+
+You can check the swagger


Suggested change

You can check the swagger

You can check the OpenAPI

packages/shopping/src/index.ts

Signed-off-by: jannyHou <[email protected]>

bajtos · 2019-09-13T13:10:26Z

packages/shopping/src/application.ts

+          ...SECURITY_SCHEMA_SPEC,
+        },
+        security: SECURITY_SPEC,
+      }),


I believe it should be fine to call this.api() just with Security parts, because the code gathering spec from controllers & routes will merge that new spec with the object supplied to this.api().

Have you tried to call this.api() from the constructor?

export class ShoppingApplication extends BootMixin( ServiceMixin(RepositoryMixin(RestApplication)), ) { constructor(options?: ApplicationConfig) { super(options); this.api({ components: { ...SECURITY_SCHEMA_SPEC, }, security: SECURITY_SPEC, }); // etc. }

In my opinion, the security spec does not depend on the app being started, therefore it should be available even before app.start() is called. Thoughts?

Should we perhaps add a story to improve app.api() to intelligently merge the new spec with any spec provided by previous calls of app.api()?

@bajtos Now app.api() takes in an entire spec in type OpenAPISpec, not Partial<OpenAPISpec>, that's why it's not possible to merge a partial spec into the generated server's spec.

Do we want to change its behaviour to take in a partial OpenAPI spec? If so I can create a story for it :), see comment loopbackio/loopback-next#2027 (comment)

Well, what are the required fields? Isn't it just a paths object that can be empty?

this.api({ components: { ...SECURITY_SCHEMA_SPEC, }, security: SECURITY_SPEC, paths: { // will be filled by LoopBack from controller metadata } });

@bajtos Not really, the problem is the openapi and info fields, they are required and especially the info object, you cannot provide an empty value like '' or {}.

See my last commit, to call this.api(), I have to provide valid value for openapi and info, but the rest server doesn't overwrite them later.

The good spec filled by rest server:

The one generated by calling this.api() with "empty" openapi and info fields:

We can improve this by defining and checking the empty value of openapi and info, like

// in rest server if (spec.openapi == '') {// generate openapi field } if (spec.info.title == '' && spec.info.version == '') {// generate info field}

Thought? If this is the right direction for improvement I can create an issue (or just submit a PR)

I see, thank you for explaining the details!

I think that ideally, I'd like:

app.api() require fields like info to force the user to provide these required fields. The current implementation is good for that 👍

Have a way for extensions to contribute additional OpenAPI metadata. This could be a new method in Application or RestServer, but we can also use binding-based approach where the app collects OpenAPI metadata from bindings with a given tag (or other metadata-based specification). I think this is out of scope of your current work (both on the spike and on enabling auth in swagger-ui), so let's create a spike issue to research different options.

As for this spike PR, I think the current proposal (as shown in the committed code) is good enough and no further changes are necessary 👍

/cc @raymondfeng

We're already exploring the idea to create an extension point to enhance the generated OpenAPI spec so that the application or other extensions can add code to transform the spec.

bajtos · 2019-09-13T13:11:32Z

packages/shopping/src/controllers/user.controller.ts

@@ -83,6 +95,7 @@ export class UserController {
  }

  @get('/users/{userId}', {
+    security: SECURITY_SPEC_OPERATION,


IIUC, this is configuring basicAuth schema. Is that intentional? I thought that we use bearerAuth schema everywhere. Or is this just for the spike?

@bajtos Oh this is just a demo, for spike only, I just want to verify that a operation level security policy(which is different than the global bearerAuth) works.

I thought that we use bearerAuth schema everywhere

Exactly 💯

agnes512

I don't know much about the implement part. But the README writeup is pretty straightforward~! Just a few comments.

agnes512 · 2019-09-13T18:13:28Z

README.md

+![set-token](/imgs/set-token.png)
+
+Paste the token you just copied in the field, then click "Authorize". The token
+will be be hidden:


-> will be hidden

agnes512 · 2019-09-13T18:16:09Z

README.md

+  }
+```
+
+The "Authorize" dialog will show up your new entry as:


your new entry -> the new added entry better?

Signed-off-by: jannyHou <[email protected]>

bajtos

LGTM with a small comment to consider.

bajtos · 2019-09-16T15:32:46Z

packages/shopping/src/application.ts

@@ -47,6 +48,13 @@ export class ShoppingApplication extends BootMixin(
 ) {
  constructor(options?: ApplicationConfig) {
    super(options);
+    this.api({
+      openapi: '3.0.0',
+      info: {title: '', version: ''},


Can you fill title and version with something more meaningful please? Personally, I'd use metadata from package.json.

import * as package from '../package.json'; // ... info: {title: package.name, version: package.version}

updated 👌

raymondfeng · 2019-09-16T16:14:39Z

@jannyHou Please fix the code-lint issue.

Signed-off-by: jannyHou <[email protected]>

jannyHou · 2019-09-16T19:17:08Z

Follow up stories see loopbackio/loopback-next#2027 (comment)

jannyHou · 2019-10-08T19:43:23Z

packages/shopping/src/application.ts

+    this.api({
+      openapi: '3.0.0',
+      info: {title: pkg.name, version: pkg.version},
+      paths: {},


servers: [{url: '/'}]

or config it when create the rest app:

https://loopback.io/doc/en/lb4/Server.html#customize-how-openapi-spec-is-served

jannyHou · 2019-10-22T16:21:03Z

Closed as the tutorial is added :)

jannyHou mentioned this pull request Sep 9, 2019

propagate configured AuthenticationStrategy to OpenApiSpec loopbackio/loopback-next#3669

Closed

2 tasks

jannyHou force-pushed the spike/swagger-ui-token branch from 6fe8163 to 9c4a77c Compare September 9, 2019 19:54

dhmlau mentioned this pull request Sep 9, 2019

[Spike] Token based authentication in API Explorer loopbackio/loopback-next#2027

Closed

3 tasks

bajtos reviewed Sep 10, 2019

View reviewed changes

feat: enable swagger-ui token

5f9b027

Signed-off-by: jannyHou <[email protected]>

jannyHou force-pushed the spike/swagger-ui-token branch from e106cbe to ee75b64 Compare September 12, 2019 16:29

bajtos reviewed Sep 13, 2019

View reviewed changes

agnes512 approved these changes Sep 13, 2019

View reviewed changes

feat: add operation security

6283c6b

Signed-off-by: jannyHou <[email protected]>

jannyHou force-pushed the spike/swagger-ui-token branch from ee75b64 to 6283c6b Compare September 13, 2019 19:24

jannyHou mentioned this pull request Sep 13, 2019

Enable the authorize button from swagger-ui in the shopping example with tutorial loopbackio/loopback-next#3740

Closed

5 tasks

bajtos approved these changes Sep 16, 2019

View reviewed changes

raymondfeng approved these changes Sep 16, 2019

View reviewed changes

fixup!: improve api method

9e0bd5f

Signed-off-by: jannyHou <[email protected]>

jannyHou force-pushed the spike/swagger-ui-token branch from 434a7d2 to 9e0bd5f Compare September 16, 2019 18:04

jannyHou mentioned this pull request Oct 1, 2019

Contribute OpenAPI spec pieces from extensions loopbackio/loopback-next#3854

Closed

3 tasks

fix: typo

cec5ad8

raymondfeng approved these changes Oct 2, 2019

View reviewed changes

jannyHou commented Oct 8, 2019

View reviewed changes

jannyHou closed this Oct 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: enable swagger-ui token #267

feat: enable swagger-ui token #267

jannyHou commented Sep 9, 2019

hacksparrow commented Sep 10, 2019

bajtos left a comment

bajtos Sep 10, 2019

bajtos Sep 13, 2019

bajtos Sep 13, 2019

jannyHou Sep 13, 2019 •

edited

Loading

bajtos Sep 16, 2019

jannyHou Sep 16, 2019

bajtos Sep 16, 2019

raymondfeng Sep 16, 2019

bajtos Sep 13, 2019

jannyHou Sep 13, 2019

agnes512 left a comment

agnes512 Sep 13, 2019

agnes512 Sep 13, 2019 •

edited

Loading

bajtos left a comment

bajtos Sep 16, 2019

jannyHou Sep 16, 2019

raymondfeng commented Sep 16, 2019

jannyHou commented Sep 16, 2019

jannyHou Oct 8, 2019

jannyHou Oct 8, 2019

jannyHou commented Oct 22, 2019

feat: enable swagger-ui token #267

feat: enable swagger-ui token #267

Conversation

jannyHou commented Sep 9, 2019

hacksparrow commented Sep 10, 2019

bajtos 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

jannyHou Sep 13, 2019 • edited Loading

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

agnes512 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agnes512 Sep 13, 2019 • edited Loading

Choose a reason for hiding this comment

bajtos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raymondfeng commented Sep 16, 2019

jannyHou commented Sep 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jannyHou commented Oct 22, 2019

jannyHou Sep 13, 2019 •

edited

Loading

agnes512 Sep 13, 2019 •

edited

Loading