feat: manage double security - #5
Conversation
|
|
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.
There was a problem hiding this comment.
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
parseSecurityReqto split@securityarguments 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.
| // 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 == "" { |
d08d304 to
56f80ed
Compare
| } | ||
| req[name] = scopes | ||
| return req |
56f80ed to
37d72c2
Compare
There was a problem hiding this comment.
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
| {map[string][]string{"A": nil}, "A && "}, // empty part skipped | ||
| {nil, ""}, // no schemes: nil, never {} ("no auth") | ||
| {nil, " && "}, // separators only: nil as well | ||
| } |
37d72c2 to
94a4a5a
Compare
| if name == "" { | ||
| continue // malformed scope-only part like "[write]" - no scheme to require | ||
| } | ||
| req[name] = scopes | ||
| } |
| 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) | ||
| } | ||
| } | ||
| } | ||
|
|
implement [ENG-8921]
94a4a5a to
f242e36
Compare
There was a problem hiding this comment.
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] = scopesoverwrites 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
}
implement ENG-8921
manage double authentication / security