Sharon Katz
d00963c330
**Related issue:** N/A # Checklist for submitter - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [x] Timeouts are implemented and retries are limited to avoid infinite loops - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually ## Summary Tightens validation in the conditional-access SCEP challenge middleware to reject enrollment requests that use a secret outside the expected scope. Adds unit test coverage for the middleware. ## Reproduction The `challengeMiddleware` in `ee/server/service/condaccess/scep.go` calls `ds.VerifyEnrollSecret()` but discards the returned secret (`_, err :=`), so it only checks that *some* valid enroll secret exists. It does not verify the secret's scope. Any valid secret from any scope passes the challenge. This was confirmed with a unit test using mock secrets scoped to different teams. Before the fix, the middleware accepted all of them indiscriminately. The server-side profile generation (`server/service/conditional_access_idp.go`) only ever embeds a global-scope secret as the SCEP challenge, so only global secrets should be accepted. ## How it was tested 1. **Unit tests (`scep_test.go`)** - table-driven test with 5 cases exercising the `challengeMiddleware` directly: - Empty challenge -> rejected ("missing challenge") - Unknown secret -> rejected ("invalid challenge") - Team-scoped secret (team A) -> rejected ("invalid challenge") *[new behavior]* - Team-scoped secret (team B) -> rejected ("invalid challenge") *[new behavior]* - Global secret (team_id = nil) -> accepted, signer invoked, cert returned 2. **Ran `go test -v ./ee/server/service/condaccess/`** - all tests pass. 3. **Ran `make lint-go-incremental`** - 0 issues. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved validation for conditional access SCEP enrollment, including rejecting team-scoped enrollment secrets. * Updated challenge middleware behavior to better handle enrollment secret verification outcomes and associated error messaging. * **Tests** * Added comprehensive coverage for conditional access SCEP challenge validation, including cases for missing, unknown, team-scoped, and global enrollment secrets, plus signer invocation expectations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 lines
68 B
Plaintext
2 lines
68 B
Plaintext
- Improved input validation for conditional access SCEP enrollment.
|