Skip to content

feat: manage double security - #5

Merged
cowan-macady merged 1 commit into
masterfrom
security-options
Jul 24, 2026
Merged

feat: manage double security#5
cowan-macady merged 1 commit into
masterfrom
security-options

Conversation

@cowan-macady

@cowan-macady cowan-macady commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

implement ENG-8921

manage double authentication / security

Copilot AI review requested due to automatic review settings July 23, 2026 10:30
@deepsource-io

deepsource-io Bot commented Jul 23, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in b6d74dc...f242e36 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
Go Jul 23, 2026 11:35a.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the swag-annotation parser to support “double authentication” by allowing a single @security directive to express multiple required security schemes combined with &&, aligning with OpenAPI’s “AND” semantics within a single Security Requirement Object.

Changes:

  • Extend parseSecurityReq to split @security arguments on && and collect multiple schemes into one requirement map.
  • Improve robustness by trimming whitespace per security-scheme segment before parsing optional scope lists.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread gen/parse.go
Comment on lines +404 to +408
// One or more schemes combined with && (all required together, swag syntax):
// "OAuth2Application[write, admin]", "ApiKeyAuth" or "ApiKeyAuth && BearerAuth".
for part := range strings.SplitSeq(args, "&&") {
part = strings.TrimSpace(part)
if part == "" {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Copilot AI review requested due to automatic review settings July 23, 2026 10:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread gen/parse.go
Comment on lines 423 to 424
}
req[name] = scopes
return req

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Copilot AI review requested due to automatic review settings July 23, 2026 11:04

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

gen/parse.go:426

  • parseSecurityReq can produce a Security Requirement Object with an empty scheme name for malformed inputs like "[write]" (name becomes ""), which then gets appended and results in invalid OpenAPI output. Treat empty scheme names as malformed and avoid emitting any requirement in that case.
				}
			}
		}
		req[name] = scopes

Comment thread gen/internal_test.go
Comment on lines +128 to +131
{map[string][]string{"A": nil}, "A && "}, // empty part skipped
{nil, ""}, // no schemes: nil, never {} ("no auth")
{nil, " && "}, // separators only: nil as well
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Copilot AI review requested due to automatic review settings July 23, 2026 11:23

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread gen/parse.go
Comment on lines +426 to +430
if name == "" {
continue // malformed scope-only part like "[write]" - no scheme to require
}
req[name] = scopes
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Comment thread gen/internal_test.go
Comment on lines +133 to +139
for _, c := range cases {
if got := parseSecurityReq(c.in); !reflect.DeepEqual(got, c.want) {
t.Errorf("parseSecurityReq(%q) = %#v, want %#v", c.in, got, c.want)
}
}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

implement [ENG-8921]
Copilot AI review requested due to automatic review settings July 23, 2026 11:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

gen/parse.go:430

  • When the same security scheme is repeated in a single && expression (e.g. OAuth2[read] && OAuth2[write]), req[name] = scopes overwrites the earlier entry, silently dropping previously parsed scopes. This can produce an incomplete Security Requirement Object from a valid-looking annotation. Consider merging scopes for duplicate scheme names (and optionally deduping) instead of overwriting.
		if name == "" {
			continue // malformed scope-only part like "[write]" - no scheme to require
		}
		req[name] = scopes
	}

@cowan-macady
cowan-macady requested a review from chahalarora July 23, 2026 11:40
@cowan-macady
cowan-macady merged commit fa03b53 into master Jul 24, 2026
9 checks passed
@cowan-macady
cowan-macady deleted the security-options branch July 24, 2026 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants