added security definition description #1174

pmorelli92 · 2022-04-12T18:06:10Z

Describe the PR

Added description capabilities for the security definitions types.

Additional context

I know this field is not mandatory, but another tool I am working with, relies on me adding the descriptions, otherwise it will break. I would need pretty please a tag if this gets merged 🙏

ubogdan · 2022-04-13T10:41:05Z

@pmorelli92 I perfectly understand the use case, please open a feature request https://github.com/swaggo/swag/issues examining this implementation requirement and I will follow you with a CR.

pmorelli92 · 2022-04-13T10:50:05Z

Done @ubogdan #1175

ubogdan

Well done.
We need to have a clear readme so our users will understand how to use it.

parser.go

ubogdan · 2022-04-16T07:05:20Z

README.md

+| securitydefinitions.oauth2.application  | [OAuth2 application](https://swagger.io/docs/specification/authentication/oauth2/) auth.       | tokenUrl, scope, sec.def.description                   | // @securitydefinitions.oauth2.application OAuth2Application |
+| securitydefinitions.oauth2.implicit     | [OAuth2 implicit](https://swagger.io/docs/specification/authentication/oauth2/) auth.          | authorizationUrl, scope, sec.def.description           | // @securitydefinitions.oauth2.implicit OAuth2Implicit       |
+| securitydefinitions.oauth2.password     | [OAuth2 password](https://swagger.io/docs/specification/authentication/oauth2/) auth.          | tokenUrl, scope, sec.def.description                   | // @securitydefinitions.oauth2.password OAuth2Password       |
+| securitydefinitions.oauth2.accessCode   | [OAuth2 access code](https://swagger.io/docs/specification/authentication/oauth2/) auth.       | tokenUrl, authorizationUrl, scope, sec.def.description | // @securitydefinitions.oauth2.accessCode OAuth2AccessCode   |


Please update sec.def.description with "description" for the parameter column and add also the "description" feature in the example column.

I added the example on the table below where the parameter annotation | example lives as I cannot do line breaks on the example cell of the mentioned table. Hope that is ok.

@ubogdan , as sec.def.description is a "line" parameter such as header, in, scopes etc; I had to name it in the mentioned way because if I named it description it will override (or take) the description at the general API info level.

I can of course change it to x-description as well as an alternative :)

See the following definition

swag/example/celler/main.go

Lines 31 to 56 in 050b0aa

// @securityDefinitions.basic BasicAuth

// @securityDefinitions.apikey ApiKeyAuth

// @in header

// @name Authorization

// @securitydefinitions.oauth2.application OAuth2Application

// @tokenUrl https://example.com/oauth/token

// @scope.write Grants write access

// @scope.admin Grants read and write access to administrative information

// @securitydefinitions.oauth2.implicit OAuth2Implicit

// @authorizationUrl https://example.com/oauth/authorize

// @scope.write Grants write access

// @scope.admin Grants read and write access to administrative information

// @securitydefinitions.oauth2.password OAuth2Password

// @tokenUrl https://example.com/oauth/token

// @scope.read Grants read access

// @scope.write Grants write access

// @scope.admin Grants read and write access to administrative information

// @securitydefinitions.oauth2.accessCode OAuth2AccessCode

// @tokenUrl https://example.com/oauth/token

// @authorizationUrl https://example.com/oauth/authorize

// @scope.admin Grants read and write access to administrative information

I expect to see something like

// @securitydefinitions.oauth2.application OAuth2Application // @tokenUrl https://example.com/oauth/token // @scope.write Grants write access // @scope.admin Grants read and write access to administrative information // @description Perform OAUTH2 at application level

I can do that :)

You made it @security.definition.description instead of @description ...

Sorry, did not see your edited comment 😅
I will try to make it work with just description but as I said before, when I tried it overlapped with the @description that you put to the general API info. I will debug and see if I can solve that.

The current way it works is the following, given these lines:

// @description this API is a demo // @securitydefinitions.oauth2.application OAuth2Application // @description Perform OAUTH2 at application level

The code will:

Range every comment

For the first line it will set the API description to this API is a demo

For the second line it will range all comments starting from comments[i+1:] in other words skipping the current. This will effectively go through the @description and set it correctly.

For the third line it will set the API description (and override) to Perform OAUTH2 at application level

The issue is that the parseSecAttr function is iterating all the lines below until it finds another @securitydefinitions.. But then, the lines that parseSecAttr iterated, will be iterated back again by the main for buckle.

This is why I changed the name to something different. I can potentially see how to make the main buckle not go through the lines that the security definition already did, but that might introduce lots of changes.

@ubogdan check it now, with the changes I have done it is working correctly and the main buckle does not re-iterate the lines that the security definitions already iterated :)

codecov · 2022-04-19T06:33:16Z

Codecov Report

Merging #1174 (d991a85) into master (050b0aa) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

❗ Current head d991a85 differs from pull request most recent head a117495. Consider uploading reports for the commit a117495 to get more accurate results

@@            Coverage Diff             @@
##           master    #1174      +/-   ##
==========================================
- Coverage   94.77%   94.71%   -0.06%     
==========================================
  Files          10       10              
  Lines        2449     2460      +11     
==========================================
+ Hits         2321     2330       +9     
- Misses         67       68       +1     
- Partials       61       62       +1

Impacted Files	Coverage Δ
parser.go	`92.41% <100.00%> (-0.15%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 050b0aa...a117495. Read the comment docs.

ubogdan

LGTM

pmorelli92 · 2022-04-19T12:54:47Z

@ubogdan I discovered a bug when you could have more than one security definition so I did the fix and added test cases for it.

ubogdan

LGTM

ubogdan · 2022-04-19T13:09:42Z

@pmorelli92 Thanks for your contribution.

added security definition description

d07b87b

pmorelli92 mentioned this pull request Apr 12, 2022

Make description not mandatory on security definition lunarway/SwaggerSwiftML#10

Closed

pmorelli92 added 2 commits April 12, 2022 20:09

unexported functions

607ae63

removed test generated file

e8fe7c1

pmorelli92 mentioned this pull request Apr 13, 2022

Add support for description on security definition #1175

Closed

ubogdan requested changes Apr 16, 2022

View reviewed changes

code format and readme

e7c7e2f

removed test generated file

f73b609

pmorelli92 requested a review from ubogdan April 19, 2022 06:57

pmorelli92 added 5 commits April 19, 2022 12:48

renamed sec.def to security.definition

2a6b9a3

support for description on security definition

cad3127

added tryAddDescription tests

ee9d582

fixed typo

c1a4f7d

added another test case scenario

36d1424

ubogdan previously approved these changes Apr 19, 2022

View reviewed changes

added test for multiple security definitions

a117495

pmorelli92 dismissed ubogdan’s stale review via a117495 April 19, 2022 12:50

pmorelli92 requested a review from ubogdan April 19, 2022 12:54

ubogdan approved these changes Apr 19, 2022

View reviewed changes

ubogdan merged commit d209f71 into swaggo:master Apr 19, 2022

pmorelli92 deleted the sec-def-description branch April 19, 2022 13:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added security definition description #1174

added security definition description #1174

pmorelli92 commented Apr 12, 2022 •

edited

Loading

ubogdan commented Apr 13, 2022

pmorelli92 commented Apr 13, 2022

ubogdan left a comment

ubogdan Apr 16, 2022

pmorelli92 Apr 19, 2022

pmorelli92 Apr 19, 2022

pmorelli92 Apr 19, 2022

ubogdan Apr 19, 2022 •

edited

Loading

pmorelli92 Apr 19, 2022

ubogdan Apr 19, 2022

pmorelli92 Apr 19, 2022

pmorelli92 Apr 19, 2022

pmorelli92 Apr 19, 2022

codecov bot commented Apr 19, 2022 •

edited

Loading

ubogdan left a comment

pmorelli92 commented Apr 19, 2022

ubogdan left a comment

ubogdan commented Apr 19, 2022

	// @securityDefinitions.basic BasicAuth

	// @securityDefinitions.apikey ApiKeyAuth
	// @in header
	// @name Authorization

	// @securitydefinitions.oauth2.application OAuth2Application
	// @tokenUrl https://example.com/oauth/token
	// @scope.write Grants write access
	// @scope.admin Grants read and write access to administrative information

	// @securitydefinitions.oauth2.implicit OAuth2Implicit
	// @authorizationUrl https://example.com/oauth/authorize
	// @scope.write Grants write access
	// @scope.admin Grants read and write access to administrative information

	// @securitydefinitions.oauth2.password OAuth2Password
	// @tokenUrl https://example.com/oauth/token
	// @scope.read Grants read access
	// @scope.write Grants write access
	// @scope.admin Grants read and write access to administrative information

	// @securitydefinitions.oauth2.accessCode OAuth2AccessCode
	// @tokenUrl https://example.com/oauth/token
	// @authorizationUrl https://example.com/oauth/authorize
	// @scope.admin Grants read and write access to administrative information

added security definition description #1174

added security definition description #1174

Conversation

pmorelli92 commented Apr 12, 2022 • edited Loading

ubogdan commented Apr 13, 2022

pmorelli92 commented Apr 13, 2022

ubogdan 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

ubogdan Apr 19, 2022 • 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

codecov bot commented Apr 19, 2022 • edited Loading

Codecov Report

ubogdan left a comment

Choose a reason for hiding this comment

pmorelli92 commented Apr 19, 2022

ubogdan left a comment

Choose a reason for hiding this comment

ubogdan commented Apr 19, 2022

pmorelli92 commented Apr 12, 2022 •

edited

Loading

ubogdan Apr 19, 2022 •

edited

Loading

codecov bot commented Apr 19, 2022 •

edited

Loading