Security: explicit string requirement for request parameters (client id, tokens etc.)

[jankapunkt]

Summary

This updates @node-oauth/formats to 1.1.0 which adds an explicit assertion for string types on every format check.
Before this, it was possible that the following values were passing format checks, such as .vschar:

null, undefined, true, {}, () => {}

Therefore, some tests were passing, although they used client ids or token as number values.
With this update this is not possible anymore, every value that needs to be a string is now explicitly required to be a string.

Please note security implications of this.

The fix is non-breaking in terms of the standard specifications or intended behavior.

Linked issue(s)

node-oauth/formats#3

Involved parts of the project

tests, isFormat (dependency)

Added tests?

yes, all upgraded

OAuth2 standard

relates to all involved RFCs that define an allowed character range for request parameters

[Copilot]

Pull request overview

This PR updates the OAuth2 server to use @node-oauth/formats v1.1.0, which enforces string types for format validation, and aligns all tests and redirect URI validation logic with the stricter type requirements.

Changes:

• Bumped @node-oauth/formats to ^1.1.0 and relaxed the mocha devDependency to a caret range, updating the lockfile accordingly.
• Updated unit and integration tests so client IDs, authorization codes, access tokens, and refresh tokens are passed and asserted as strings instead of numbers.
• Hardened AuthorizationCodeGrantType.validateRedirectUri to explicitly require a non-empty redirect_uri string before running URI format validation, preventing runtime errors with the new formats library while keeping RFC-compliant behavior.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
lib/grant-types/authorization-code-grant-type.js	Tightens redirect URI validation to guard against missing values before applying string-only URI checks from the updated formats library.
test/unit/handlers/token-handler_test.js	Adjusts unit tests for TokenHandler to use string client IDs and expectations consistent with string-only format checks.
test/unit/handlers/authorize-handler_test.js	Updates AuthorizeHandler unit tests to treat client_id as a string in request bodies and assertions.
test/unit/grant-types/authorization-code-grant-type_test.js	Ensures authorization codes and related values are strings in unit tests and updates PKCE-related tests accordingly.
test/integration/server_test.js	Aligns integration tests around the Server class with string client IDs and authorization codes.
test/integration/handlers/token-handler_test.js	Converts numeric client IDs, codes, and refresh tokens in token handler integration tests to strings to match new validation behavior.
test/integration/handlers/authorize-handler_test.js	Uses string client_id values consistently in authorize handler integration tests and keeps redirect URI error-path coverage in place.
test/integration/grant-types/refresh-token-grant-type_test.js	Switches client IDs and refresh tokens in refresh token grant integration tests from numbers to strings, preserving all behavioral expectations.
test/integration/grant-types/authorization-code-grant-type_test.js	Updates authorization-code grant integration tests for string codes/IDs and extends coverage around redirect URI validation under the new rules.
package.json	Bumps @node-oauth/formats to ^1.1.0 and changes mocha to ^11.7.5 to allow compatible patch/minor updates.
package-lock.json	Regenerates the lockfile to reflect the new @node-oauth/formats version, updated tooling (Rollup, Vue, Shiki, etc.), and removal of unused dev-time AI/docsearch-related dependencies.

---

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

[dhensby]

The change in the tests from numbers to strings makes me a little nervous as that can be indicative of some breaking behaviour change. I get that most of these tests are just mock values and perhaps were invalid to begin with, but are we confident this doesn't break existing behaviours in the wild?

Request bodies should be application/x-www-form-urlencoded format, right? So everything is a string - so assuming there's not funky middleware happening, the request bodies should validate without a problem; but there is also some change to function return values (eg: getAuthorizationCode() return values have changed).

Are these requirements reflected in the typings? edit: I see the typings already have these down as strings

[jankapunkt]

@dhensby I definitely would not rule out possible breaks. However, from what I saw the tests were not covering the real world scenario behavior and this is on most cases strings being used for client IDs and tokens.

In order to validate non breaking behavior we can publish a release candidate and communicate that changes are not likely breaking but maybe. RC are not installed by default and need explicit version being added to npm install.
In my experience there only few adopters who really make use of it, though.

but there is also some change to function return values (eg: getAuthorizationCode() return values have changed)

Can you please comment the lines that changed the return values? The PR was not intended to change anything beyond tests. The only change in the code is to verify redirectURL the same way it was done before.