Surnet / swagger-jsdoc

Generates swagger/openapi specification based on jsDoc comments and YAML files.
MIT License
1.7k stars 228 forks source link

global security for openapi3 #124

Open lacivert opened 6 years ago

lacivert commented 6 years ago

I write security option as it was stated in Openapi docs; https://swagger.io/docs/specification/authentication/bearer-authentication/

Check "# 2) Apply the security globally to all operations" on the page about 'security'

So I put the 'security' key like this as 'root level', and 'securitySchemes' in components;

/**
 * @swagger
 * tags:
 *   name: Login
 *   description: Login controller
 * security:
 *   - bearerAuth: []
 * components:
 *   responses:
 *     UnauthorizedError:
 *       description: Access token is missing or invalid
 *   securitySchemes:
 *     bearerAuth:            # arbitrary name for the security scheme
 *       type: http
 *       scheme: bearer
 *       bearerFormat: JWT

But the api-docs.json file, I see the 'secutiry' in the 'paths' object;

"paths": {
    "/charts": {
      "get": {
        "description": "Returns the list of charts",
        "tags": [
          "Charts"
        ],
        "responses": {
          "200": {
            "description": "Success",
            "schema": {
              "type": "array",
              "items": {
                "$ref": "#/components/schemas/Chart"
              }
            }
          }
        }
      }
    },
    "security": {
      "0": {
        "bearerAuth": [

        ]
      }
    },
    "/login": {
       //...
     }
}

Is it a bug or do I write 'security' in wrong place?

lacivert commented 6 years ago

By the way, it works if I write security into the options.swaggerDefinition;

const options = {
    swaggerDefinition: {
        openapi: "3.0.0",
        info: {
            title: "My App", // Title (required)
            version: "1.0.0" // Version (required)
        },
        security: [
            {
                bearerAuth: []
            }
        ]
    },
    apis: ["./routes/*.js"] // Path to the API docs
};
kalinchernev commented 6 years ago

@lacivert thank you for following up on the issue and sharing your solution for the people after you! Indeed, root objects are to be added in the definition, especially when they are to follow a specific structure like the security one. Otherwise, the parser goes through comments and accumulates the others and attaches them with higher cardinality. (several items in an object)

I'm closing this one being fixed.

sibelius commented 3 years ago

can we rethink of definition everything inside .yml files?

kalinchernev commented 3 years ago

Could you please clarify?

On Thu, 4 Mar 2021, 15:58 Sibelius Seraphini, notifications@github.com wrote:

can we rethink of definition everything inside .yml files?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/Surnet/swagger-jsdoc/issues/124#issuecomment-790636797, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOVTFHKGOMZHHWQZJ7WGKLTB6GZXANCNFSM4FNP73JQ .

sibelius commented 3 years ago

We define our swagger in many .yml files and use this package to bundle all of them in a final version

I can define paths and components But defining security on .yml breaks because it thinks that security is a path instead of a top level field

I think all top level fields should be able to be defined in .yml files

So we could even remove the need for a config file

sibelius commented 3 years ago

I could have something like this

config.yml

openapi: '3.0.3',
info: 
  title: 'Title'
  description: 'Description'
  version: '0.0.1'
servers: 
  url: 'http://api.example.com.br/',
  description: 'Production server',
security: 
 AppID: [],
kalinchernev commented 3 years ago

We define our swagger in many .yml files and use this package to bundle all of them in a final version

Very good definition, I wish other users also understood this :D

I think all top level fields should be able to be defined in .yml files So we could even remove the need for a config file

Not quite, I think. We'll need some sort of a starting config file: otherwise what could be a good-enough starting point assumption?

At the same time, I think your suggestion could be possible to accommodate. This is the current starting point https://github.com/Surnet/swagger-jsdoc/blob/v7/src/specification.js#L21 What we can do (I think) is to move the definition loader back to main utils and use it as-is.

Essentially, options.definition becomes more loose, either an object, or a path to a file. If path to a file, load and use as a initial definition, else use the directly-set object.

What do you think?

kalinchernev commented 3 years ago

My previous comment was referring to 7.x RC. In 6.x you still have the CLI, it's accepting definitions in yaml.

See https://github.com/Surnet/swagger-jsdoc/blob/master/docs/CLI.md#definition-file

On Thu, Mar 4, 2021 at 5:48 PM Sibelius Seraphini notifications@github.com wrote:

I could have something like this

config.yml

openapi: '3.0.3',info: title: 'Title' description: 'Description' version: '0.0.1'servers: url: 'http://api.example.com.br/', description: 'Production server',security: AppID: [],

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/Surnet/swagger-jsdoc/issues/124#issuecomment-790760254, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOVTFEI6SCJF4FFFZO43TLTB62XRANCNFSM4FNP73JQ .

-- Kalin Chernev tel: +359878536542

sibelius commented 3 years ago

is the definitions deprecated on v3?

I think it would be worth to support at top level like this https://swagger.io/docs/specification/basic-structure/

and keep the paths handling like it is

it the top level of a yml is not a predefined one, it should be considered a path

kalinchernev commented 3 years ago

is the definitions deprecated on v3?

Definition is valid for all versions of swagger-jsdoc. I deprecated the CLI in 7.x, which contained the yaml loader.

I think it would be worth to support at top level like this https://swagger.io/docs/specification/basic-structure/

It is supported, from the so called swagger definition.

it the top level of a yml is not a predefined one, it should be considered a path

I don't understand this, please clarify

kalinchernev commented 3 years ago

Based on recurring questions and issues I'm re-opening. There's a clear need for an example and further work in terms of security definitions and swagger definition handling.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

baslr commented 2 years ago

I use version 3 and tried to use the key security in a yaml file but it did not worked. I looked into the code and I identified these parts that need work to support the security field. https://github.com/Surnet/swagger-jsdoc/blob/f9ce23b6c541b835037352a73ca21741652ffde1/src/specification.js#L36 This is not a valid key in version 3. See https://swagger.io/specification/ (version 3.0.3).

https://github.com/Surnet/swagger-jsdoc/blob/f9ce23b6c541b835037352a73ca21741652ffde1/src/specification.js#L55 Here the initialization of security is missing.

https://github.com/Surnet/swagger-jsdoc/blob/f9ce23b6c541b835037352a73ca21741652ffde1/src/specification.js#L161 Here the special case else if (property === 'security') is missing.

It should be clarified in the readme.md which parts of the specification are actually supported or which parts are not supported (Kind of matrix with field name x json definition and yaml files. Also if a workaround is available it should be mentioned.

Also I suggest one extra documentation chapter How to secure your API. Do not leave security behind.