Redocly / redocly-cli

⚒️ Redocly CLI makes OpenAPI easy. Lint/validate to any standard, generate beautiful docs, and more.
https://redocly.com/docs/cli/
MIT License
849 stars 129 forks source link

Ability to exclude some operations from security-defined rule #1569

Closed RomanHotsiy closed 1 month ago

RomanHotsiy commented 1 month ago

Is your feature request related to a problem? Please describe.

Very often API define a health endpoint which is not protected by any kind of auth.

Our security-defined rule always reports this as a problem.

Surely, it can be ignored with ignore file but this is a problem for users who constantly onboard new APIs. Configurable rule can't be used here as it doesn't cover security defined on different levels (operation vs root).

Describe the solution you'd like

Add support for exceptions config option to the rule:

rules:
  security-defined:
    severity: error
    exceptions:
      - path: '/health'
        method: GET

Describe alternatives you've considered

I used exceptions as we use this term in path-segment-plural rule. Let me know if you have better suggestions.

Another alternative is to have a simpler list of exceptions (only path)

rules:
  security-defined:
    severity: error
    exceptions:
      - '/health'

What do you think?

tatomyr commented 1 month ago

I like the first approach better as it allows fine-grained exceptions. However, I'd make the method property plural to be able to specify several methods. Also, we can make it optional and ignore an entire path if there are no methods specified.

adamaltman commented 1 month ago

FYI:

If you define security on the operation or path item as security: [] it means security is explicitly defined as no security, it won't trigger the rule.

I mention it in the docs here: https://redocly.com/docs/cli/rules/security-defined/#api-design-principles

tatomyr commented 1 month ago

@RomanHotsiy do you think we still need this adjustment? Is there a case that couldn't be easily covered by the empty security as @adamaltman suggests?

RomanHotsiy commented 1 month ago

Yes. Still makes sense.