Apply resource-wide security to every endpoint when no methods are listed

[dhruv-techdev]

Fixes #96

What was wrong

If a resource set a security scheme without listing methods, no endpoint got marked as secured. The docs say it should apply to all methods. Nothing errored, so a server generated from the spec could accept requests without any login.

What this PR changes

In openapi.py, the code used to save the security setting into rsrc["security"], which nothing reads. Now, when no per-method lists are given, it fills them in with every method:

security = {
    **security,
    "resource": RSRC_HTTP_METHODS,
    "instance": RSRC_INST_HTTP_METHODS,
    "instance_attrs": RSRC_ATTR_HTTP_METHODS,
}

The existing per-method code then secures every endpoint as expected.

Tests

Added two tests in test/spec/test_openapi.py:

• No method lists: every endpoint is secured. This fails before the fix.
• Method lists: only the listed methods are secured. Behaviour is unchanged.

I regenerated examples/addressbook/openapi.yaml, and it didn't change, since those resources already list their methods.

How to test

pytest test/spec/test_openapi.py -k security

Or by hand, with a resource that has only security.scheme set:

firestone generate -t Books -d Books -v 1 -r secure_books.yaml openapi | grep -A1 "security"

Before: the only match is securitySchemes:.
After: every endpoint shows security: - bearer_auth: [].

[dhruv-techdev]

Hi! This PR is ready for review whenever you get a chance. Happy to make any changes. Thanks! @ebourgeois

[ebourgeois]

Review of the fix for #96. The change itself works: the new tests pass and black and pylint are clean. The findings below are what it exposes or leaves open. Inline comments cover lines in the diff; these five are on lines the PR does not touch:

1. Specs with two schemes now break (firestone/spec/openapi.py:560). add_rsrc_components replaces components['securitySchemes'] per resource instead of merging. With resource foo using only bearer_auth and baz using only api_key (reproduced), securitySchemes contains only api_key but GET /foo has security: [{bearer_auth: []}]. The spec is invalid and validators/openapi-generator reject it; before this PR the same input produced a valid spec. Fix: components.setdefault('securitySchemes', {}).update(security['scheme']).

2. Regenerated Rust servers stop compiling (firestone/spec/server_rust.py:496). Cargo.toml is a kept scaffold and only gets subtle when the server is secured. A server generated earlier from a scheme-only resource has no subtle; regenerating now rewrites lib.rs with pub mod auth; and writes auth.rs with use subtle::Choice, while Cargo.toml is kept. cargo build fails on the unresolved import unless the user passes --force and loses their scaffold. (From reading the code; not compiled.)

3. Only the first scheme is ever applied (firestone/spec/openapi.py:283). list(security['scheme'].keys())[0] means scheme: {bearer_auth: ..., api_key: ...} secures every operation with bearer_auth only (reproduced); api_key is emitted but never accepted, so API-key clients get 401 everywhere instead of an OR of both schemes.

4. Docs are still wrong and the behaviour change is undocumented (docs/site/content/advanced-topics/security-schemes.md:32). The doc cited by the issue shows OpenAPI-style securitySchemes: and security: - BearerAuth: [] list syntax that resource.yaml rejects. Existing scheme-only users now get every operation secured, GET and HEAD included, so regenerated servers answer 401 until API_TOKENS is set, with no doc or release note.

5. Dead resource-wide security path in the template (firestone/schema/openapi.jinja2:11). The top-level security: block keyed on security_name can never render because generate() never passes security_name. The PR removes the last remnant of that path (rsrc['security'] = [...]) but leaves this branch, the same kind of misleading dead path that caused #96.

Verified by applying the diff and running the suite on Python 3.13: 216 passed, 18 failed, all traced to celpy missing in the test container, not to this PR.

[dhruv-techdev]

Thanks for the detailed review, @ebourgeois ! You're right on all of these.

I'll update this PR to:

• default each level to all methods when its list is missing, using one helper wherever security is read
• merge securitySchemes across resources instead of replacing them
• copy the method lists instead of sharing the module constants
• fix the per-level test and cover delete/head/patch, no methods block, nested resources, and multiple resources

For the Rust Cargo.toml, multi-scheme, docs and template points, would you prefer those in separate PRs?

[ebourgeois]

Thanks for the detailed review, @ebourgeois ! You're right on all of these.

I'll update this PR to:

• default each level to all methods when its list is missing, using one helper wherever security is read
• merge securitySchemes across resources instead of replacing them
• copy the method lists instead of sharing the module constants
• fix the per-level test and cover delete/head/patch, no methods block, nested resources, and multiple resources

For the Rust Cargo.toml, multi-scheme, docs and template points, would you prefer those in separate PRs?

Same PR is fine.

[dhruv-techdev]

Thanks @ebourgeois, I've pushed an update covering the whole review:

• Per-level defaults: a get_security() helper is used wherever security is read. A missing list secures every method at that level, and [] secures none, matching resource.yaml.
• Schemes: merged across resources, and several schemes on one resource are now alternatives (any one is accepted).
• Rust Cargo.toml: still never overwritten, but generate server now warns with the exact lines to add when the kept file lacks a dependency the regenerated code needs (e.g. subtle). I couldn't compile Rust locally, so I verified this from the generated files.
• Docs: rewrote security-schemes.md with the real syntax and rules (every example checked against firestone), and added an Unreleased changelog entry for the behaviour change.
• Template: removed the dead security_name block. The output is unchanged.
• Tests: per-level, empty list, no methods block, nested, multiple resources and multiple schemes, plus the Cargo.toml check.

black and pylint are clean, and the example openapi.yaml is unchanged.