fix(bearer-auth): make auth-scheme case-insensitive#4659
fix(bearer-auth): make auth-scheme case-insensitive#4659yusukebe merged 2 commits intohonojs:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4659 +/- ##
=======================================
Coverage 91.43% 91.43%
=======================================
Files 173 173
Lines 11373 11373
Branches 3296 3296
=======================================
Hits 10399 10399
Misses 973 973
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| const realm = options.realm?.replace(/"/g, '\\"') | ||
| const prefixRegexStr = options.prefix === '' ? '' : `${options.prefix} +` | ||
| const regexp = new RegExp(`^${prefixRegexStr}(${TOKEN_STRINGS}) *$`) |
There was a problem hiding this comment.
Simply, only adding i to the regexp is not okay?
const regexp = new RegExp(`^${prefixRegexStr}(${TOKEN_STRINGS}) *$`, 'i')This means you can apply the patch to the main:
diff --git a/src/middleware/bearer-auth/index.ts b/src/middleware/bearer-auth/index.ts
index 3ecd2e3b..122ccc55 100644
--- a/src/middleware/bearer-auth/index.ts
+++ b/src/middleware/bearer-auth/index.ts
@@ -113,7 +113,7 @@ export const bearerAuth = (options: BearerAuthOptions): MiddlewareHandler => {
const realm = options.realm?.replace(/"/g, '\\"')
const prefixRegexStr = options.prefix === '' ? '' : `${options.prefix} +`
- const regexp = new RegExp(`^${prefixRegexStr}(${TOKEN_STRINGS}) *$`)
+ const regexp = new RegExp(`^${prefixRegexStr}(${TOKEN_STRINGS}) *$`, 'i')
const wwwAuthenticatePrefix = options.prefix === '' ? '' : `${options.prefix} `
const throwHTTPException = async (There was a problem hiding this comment.
Hi @yusukebe,
Using the i flag would make TOKEN_STRINGS matching case-insensitive as well.
While validation is ultimately done via equal = await options.verifyToken(match[1], c) using the captured original value, this change was intended to make only the prefix case-insensitive while keeping the token itself case-sensitive.
Which approach do you prefer? 🧐
There was a problem hiding this comment.
Hey! Thank you for the explanation!
Hmmm. This is a bit difficult problem. Honestly, both are okay, and I can fully understand what you intend. But the code in this PR is verbose. So how about using i flag, though, it is making TOKEN_STRINGS case-insensitive?
Just to confirm, the final behavior will not change, right?
There was a problem hiding this comment.
Yes, since verifyToken() compares the captured original value, the end result is the same. I've reverted the change, updated it, and added tests for this case 🙂
010fecb to
fafa38d
Compare
|
Looks good! Let's go with this. Thanks! |
The author should do the following, if applicable
bun run format:fix && bun run lint:fixto format the codeNote
It uses a case-insensitive token as a means to identify the authentication scheme, followed by additional information necessary for achieving authentication via that scheme.
https://datatracker.ietf.org/doc/html/rfc7235#section-2.1
This PR updates the
bearer-authmiddleware to handle auth scheme comparison case-insensitively