feat(basic-auth): added custom response message option

[marceloverdijk]

The author should do the following, if applicable

• Add tests
• Run tests
• bun run format:fix && bun run lint:fix to format the code
• Add TSDoc/JSDoc to document the code

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.78%. Comparing base (3500404) to head (6029ba3).
Report is 13 commits behind head on next.

Additional details and impacted files

@@           Coverage Diff           @@
##             next    #3371   +/-   ##
=======================================
  Coverage   95.77%   95.78%           
=======================================
  Files         152      152           
  Lines        9187     9202   +15     
  Branches     2818     2823    +5     
=======================================
+ Hits         8799     8814   +15     
  Misses        388      388           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[yusukebe]

@marceloverdijk Thank you for the PR!

Regarding the invalidUserMessage function, I think allowing returning string | object is not a good design. It might be useful for some users, but this should be more flexible and reduce the if and type of statement to detect the types.

I think the following is best:

type CustomMessageFunction = (c: Context) => Response | Promise<Response>

Returning Response is the most flexible, and the user can fully customize the response. Enable to this, you can change it as the following:

diff --git a/src/middleware/basic-auth/index.ts b/src/middleware/basic-auth/index.ts
index b9b9f359..f69cc049 100644
--- a/src/middleware/basic-auth/index.ts
+++ b/src/middleware/basic-auth/index.ts
@@ -9,19 +9,21 @@ import type { MiddlewareHandler } from '../../types'
 import { auth } from '../../utils/basic-auth'
 import { timingSafeEqual } from '../../utils/buffer'

+type CustomMessageFunction = (c: Context) => Response | Promise<Response>
+
 type BasicAuthOptions =
   | {
       username: string
       password: string
       realm?: string
       hashFunction?: Function
-      invalidUserMessage?: string | object | Function
+      invalidUserMessage?: CustomMessageFunction
     }
   | {
       verifyUser: (username: string, password: string, c: Context) => boolean | Promise<boolean>
-      invalidUserMessage?: string | object | Function
+      invalidUserMessage?: CustomMessageFunction
     }

 /**
@@ -73,10 +75,6 @@ export const basicAuth = (
     options.realm = 'Secure Area'
   }

-  if (!options.invalidUserMessage) {
-    options.invalidUserMessage = 'Unauthorized'
-  }
-
   if (usernamePasswordInOptions) {
     users.unshift({ username: options.username, password: options.password })
   }
@@ -104,23 +102,15 @@ export const basicAuth = (
     }
     // Invalid user.
     const status = 401
-    const headers = {
-      'WWW-Authenticate': 'Basic realm="' + options.realm?.replace(/"/g, '\\"') + '"',
+    ctx.status(401)
+    ctx.header('WWW-Authenticate', 'Basic realm="' + options.realm?.replace(/"/g, '\\"') + '"')
+
+    const defaultCustomMessage = (c: Context) => {
+      return c.text('Unauthorized')
     }
-    const responseMessage =
-      typeof options.invalidUserMessage === 'function'
-        ? await options.invalidUserMessage(ctx)
-        : options.invalidUserMessage
-    const res =
-      typeof responseMessage === 'string'
-        ? new Response(responseMessage, { status, headers })
-        : new Response(JSON.stringify(responseMessage), {
-            status,
-            headers: {
-              ...headers,
-              'content-type': 'application/json; charset=UTF-8',
-            },
-          })
+
+    const customResponse = options.invalidUserMessage ?? defaultCustomMessage
+    const res = await customResponse(ctx)
     throw new HTTPException(status, { res })
   }
 }

I'd like to hear other's opinions: cc @usualoma

[marceloverdijk]

@yusukebe yes that looks like a very good approach.
Shall I update the PR, or wait for @usualoma ?

[marceloverdijk]

@yusukebe I created this alternative PR with the changes you suggested: #3380

[marceloverdijk]

@yusukebe as we decided to close #3380 , let's discuss here what is needed to get #3371 going again.

[yusukebe]

Hi @marceloverdijk

I think almost all are good. I've left a comment. Check it.

[yusukebe]

LGTM!

[yusukebe]

Hi @marceloverdijk

Looks good to me! Let's go with this. I'll merge this into the next later.