Fix: Return error from attempted security scheme instead of first defined scheme

[aleksandar-kovacic]

Problem

When multiple security schemes are defined (e.g., BearerAuth OR ApiKeyAuth) and a user provides credentials for only one scheme, the error returned was always from the first defined security scheme rather than the one actually attempted.

Example

Given this OpenAPI security configuration:

security:
  - BearerAuth: []
  - ApiKeyAuth: []

Before this fix

User sends request with an invalid ApiKey but no Bearer token.
ApiKey validation runs and fails with "Invalid API key".
❌ Error returned: "Authorization header required" (from BearerAuth, which wasn't attempted).

After this fix

User sends request with an invalid ApiKey but no Bearer token.
ApiKey validation runs and fails with "Invalid API key"
✅ Error returned: "Invalid API key provided" (from ApiKeyAuth, which was actually attempted)

Solution

Added an attempted flag to track which security schemes received credentials. The error selection logic now prioritizes errors from schemes where authentication was actually attempted over schemes where credentials were missing.

Closes #1118

[Copilot]

Pull request overview

This PR adjusts the OpenAPI security middleware’s error selection when multiple OR security schemes are defined, so that the returned error reflects the scheme that was actually attempted (i.e., had credentials provided) rather than always returning the first scheme’s missing-credentials error.

Changes:

• Add an attempted flag to security handler results and prefer attempted-scheme errors when choosing which error to return.
• Track whether AuthValidator passed before invoking a handler to determine “attempted”.
• Add a regression test and a new OpenAPI path (/bearer_or_apikey) to validate the behavior.

Reviewed changes

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

File	Description
src/middlewares/openapi.security.ts	Adds attempted tracking and updates error selection to prefer attempted-scheme failures.
test/security.handlers.spec.ts	Adds an integration test covering bearer-vs-apikey OR error selection.
test/resources/security.yaml	Adds /bearer_or_apikey path with OR security requirements for the new test.

---

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

[Copilot]

The attempted doc comment says “credentials were provided and handler was called”, but in executeHandlers it’s really derived from AuthValidator.validate() passing. Since validateOauth2 / validateOpenID are currently no-ops, those schemes will be marked as attempted even when no credentials are present. Either update the comment to match the current meaning, or refine the attempted detection so it truly reflects credential presence across all scheme types.

Suggested change

	attempted?: boolean; // true if credentials were provided and handler was called
	attempted?: boolean; // true if AuthValidator.validate() passed for this scheme and the handler was called

[Copilot]

SecurityHandlerResult.error is typed as string, but this code stores the caught exception object there and later reads e.error.message. Updating the type to something like unknown/any or a structured error shape would make the contract accurate and prevent consumers from assuming it’s a string.

Suggested change

	error?: string;
	error?: any;

[Copilot]

This comment states “validation passed and credentials were provided”, but AuthValidator.validate() currently does not validate oauth2/openIdConnect credentials at all (those methods are TODO/no-op). That means validatorPassed can become true even when no credentials are present for those scheme types. Consider rewording this comment (and/or renaming validatorPassed) to avoid implying stronger guarantees than the code provides.

Suggested change

	// If we reach here, validation passed and credentials were provided
	// If we reach here, AuthValidator did not report validation errors for this scheme

[Copilot]

The new attempted-error prioritization can mask configuration/5xx errors. For an OR security list where one scheme fails with a 5xx (e.g., missing/invalid handler or security scheme definition) and another scheme was attempted and failed with 401, this logic will now prefer the 401 and hide the 5xx. Consider prioritizing 5xx errors (or otherwise non-authn configuration errors) ahead of attempted-401s so misconfigurations still surface correctly.

Suggested change

	// Prioritize errors where authentication was actually attempted
	const attemptedErrors = errors.filter(e => e.attempted);
	const errorToThrow = attemptedErrors.length > 0 ? attemptedErrors[0] : errors[0];
	// Prefer server/configuration errors (5xx) so misconfigurations surface
	const serverErrors = errors.filter(
	(e) => typeof e.status === 'number' && e.status >= 500 && e.status < 600,
	);
	let errorToThrow;
	if (serverErrors.length > 0) {
	errorToThrow = serverErrors[0];
	} else {
	// Otherwise, prioritize errors where authentication was actually attempted
	const attemptedErrors = errors.filter((e) => e.attempted);
	errorToThrow = attemptedErrors.length > 0 ? attemptedErrors[0] : errors[0];
	}

[Copilot]

Pull request overview

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

---

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