fix: auth typings and reduced type ambiguity (#4562)

* fix: revert helper that broke auth

* fix: remove type ambiguity, add TS docs

Author: damusix

lib/types/request.d.ts

@@ -89,6 +89,8 @@ export interface RequestAuth<
      * set to false.
      */
     isAuthorized: boolean;
+    /** true if the request has been authenticated via the `server.inject()` `auth` option, otherwise `undefined`. */
+    isInjected?: boolean | undefined;
     /** the route authentication mode. */
     mode: AuthMode;
     /** the name of the strategy used. */
@@ -250,7 +252,7 @@ export interface RequestLog {
 }
 
 export interface RequestQuery {
-    [key: string]: any;
+    [key: string]: string | string[] | undefined;
 }
 
 /**
@@ -269,9 +271,9 @@ export interface InternalRequestDefaults {
 
     Payload: stream.Readable | Buffer | string | object;
     Query: RequestQuery;
-    Params: Record<string, any>;
+    Params: Record<string, string>;
     Pres: Record<string, any>;
-    Headers: Record<string, any>;
+    Headers: Record<string, string | string[] | undefined>;
     RequestApp: RequestApplicationState;
 
     AuthUser: UserCredentials;
@@ -303,11 +305,7 @@ export type ReqRef = Partial<Record<keyof ReqRefDefaults, unknown>>;
 /**
  * Utilities for merging request refs and other things
  */
-export type MergeType<T, U> = {
-    [K in keyof T]: K extends keyof U
-        ? U[K]
-        : T[K];
-};
+export type MergeType<T, U> = Omit<T, keyof U> & U;
 
 export type MergeRefs<T extends ReqRef> = MergeType<ReqRefDefaults, T>;
 
@@ -441,7 +439,7 @@ export interface Request<Refs extends ReqRef = ReqRefDefaults> extends Podium {
     /**
      * Same as pre but represented as the response object created by the pre method.
      */
-    readonly preResponses: Record<string, any>;
+    readonly preResponses: Record<string, unknown>;
 
     /**
      * By default the object outputted from node's URL parse() method.
@@ -474,7 +472,7 @@ export interface Request<Refs extends ReqRef = ReqRefDefaults> extends Podium {
     /**
      * An object containing parsed HTTP state information (cookies) where each key is the cookie name and value is the matching cookie content after processing using any registered cookie definition.
      */
-    readonly state: Record<string, any>;
+    readonly state: Record<string, unknown>;
 
     /**
      * The parsed request URI.

lib/types/route.d.ts

@@ -358,7 +358,7 @@ export interface RouteOptionsPreObject<Refs extends ReqRef = ReqRefDefaults> {
     /**
      * key name used to assign the response of the method to in request.pre and request.preResponses.
      */
-    assign?: keyof Refs['Pres'] | undefined;
+    assign?: keyof MergeRefs<Refs>['Pres'] | undefined;
     /**
      * A failAction value which determine what to do when a pre-handler method throws an error. If assign is specified and the failAction setting is not 'error', the error will be assigned.
      */
@@ -978,5 +978,5 @@ export interface ServerRoute<Refs extends ReqRef = ReqRefDefaults> {
     /**
      * route custom rules object. The object is passed to each rules processor registered with server.rules(). Cannot be used if route.options.rules is defined.
      */
-    rules?: Refs['Rules'] | undefined;
+    rules?: MergeRefs<Refs>['Rules'] | undefined;
 }

test/types/index.ts

@@ -4,6 +4,7 @@ import * as CatboxMemory from '@hapi/catbox-memory';
 
 import {
     Plugin,
+    ReqRef,
     Request,
     RequestRoute,
     ResponseToolkit,
@@ -68,20 +69,20 @@ interface RequestDecorations {
         prefix: string[];
     },
     AuthUser: {
-        id: string,
-        name: string
-        email: string
+        id: string;
+        name: string;
+        email: string;
     },
     AuthCredentialsExtra: {
-        test: number
-    }
+        test: number;
+    },
     AuthApp: {
-        key: string
-        name: string
+        key: string;
+        name: string;
     },
     AuthArtifactsExtra: {
-        some: string
-        thing: number
+        some: string;
+        thing: number;
     }
 }
 
@@ -287,3 +288,230 @@ server.decorate('server', 'obj3_1', { func5: theFunc });
 // Error when extending on server with objects
 // @ts-expect-error Lab does not support overload errors
 check.error(() => server.decorate('server', 'obj3_1', { func5: theFunc }, { apply: true, extend: true }));
+
+// Issue #4561 - Generic Request<Refs> should resolve augmented ReqRefDefaults auth properties
+
+interface ExtraCred {
+    extra_id: string;
+}
+
+interface UserProfile {
+    id: string;
+}
+
+declare module '../..' {
+    interface ReqRefDefaults {
+        AuthCredentialsExtra: Partial<ExtraCred>;
+    }
+}
+
+// Generic route (no custom refs) should see augmented UserCredentials
+const genericAuthRoute: ServerRoute = {
+    method: 'GET',
+    path: '/auth-check',
+    handler: (request, h) => {
+
+        check.type<string>(request.auth.credentials.user!.someId);
+        check.type<string>(request.auth.credentials.user!.someName);
+
+        const credIsAny: IsAny<typeof request.auth.credentials> = false;
+
+        return 'ok';
+    }
+};
+
+// Generic function should see augmented credentials from ReqRefDefaults
+export function processAuthGeneric<Refs extends ReqRef>(req: Request<Refs>): void {
+
+    if (req.auth.isAuthenticated && req.auth.credentials.extra_id) {
+        check.type<string | undefined>(req.auth.credentials.extra_id);
+    }
+}
+
+// Non-generic Request should also see augmented credentials
+export function processAuthConcrete(req: Request): void {
+
+    if (req.auth.isAuthenticated && req.auth.credentials.extra_id) {
+        check.type<string | undefined>(req.auth.credentials.extra_id);
+    }
+
+    // credentials should NOT resolve to `any`
+    const credIsAny: IsAny<typeof req.auth.credentials> = false;
+    const artifactsIsAny: IsAny<typeof req.auth.artifacts> = false;
+}
+
+// Generic function should accept Request with specific route refs
+interface SpecificRouteRefs {
+    Params: { id: string };
+}
+
+export function callWithSpecificRefs(req: Request<SpecificRouteRefs>): void {
+
+    processAuthGeneric(req);
+}
+
+// =============================================================================
+// ReqRef System Issue Tests
+// Each section demonstrates a specific weakness in the current type system.
+// These tests produce VISIBLE compiler errors to demonstrate each problem.
+// =============================================================================
+
+// -----------------------------------------------------------------------------
+// ISSUE 1: Direct Refs['Key'] access bypasses MergeRefs (route.d.ts:361)
+//
+// RouteOptionsPreObject.assign uses `keyof Refs['Pres']` instead of
+// `keyof MergeRefs<Refs>['Pres']`. When the user doesn't explicitly provide
+// `Pres` in their Refs, `Refs['Pres']` is `unknown` (from ReqRef's
+// Partial<Record<..., unknown>>), so `keyof unknown` is `never`.
+// This means `assign` is impossible unless Pres is explicitly provided.
+// -----------------------------------------------------------------------------
+
+// This should compile — the user only customizes Params, and the default
+// Pres (Record<string, any>) should allow any string for `assign`.
+// ERROR: Type '"user"' is not assignable to type 'never'.
+const issuePreAssign: ServerRoute<{ Params: { id: string } }> = {
+    method: 'GET',
+    path: '/users/{id}',
+    options: {
+        pre: [
+            {
+                method: (request, h) => ({ name: 'test' }),
+                assign: 'user'                                  // TS ERROR — should work
+            }
+        ],
+        handler: (request, h) => 'ok'
+    }
+};
+
+// -----------------------------------------------------------------------------
+// ISSUE 2: Params defaults to Record<string, any> — allows unsafe access
+//
+// URL path params are ALWAYS strings at runtime (before Joi validation), but
+// the default type Record<string, any> means TypeScript allows anything.
+// These assignments should all be errors but none are.
+// -----------------------------------------------------------------------------
+
+const issueParamsAny: ServerRoute = {
+    method: 'GET',
+    path: '/items/{id}',
+    handler: (request, h) => {
+
+        // FIXED: Params now correctly typed as Record<string, string>
+        // @ts-expect-error - params are strings, not numbers
+        const id: number = request.params.id;
+        // @ts-expect-error - params are strings, not boolean[]
+        const wat: boolean[] = request.params.id;
+
+        // FIXED: params is no longer `any`
+        const paramsIsAny: IsAny<typeof request.params.id> = false;
+
+        return 'ok';
+    }
+};
+
+// -----------------------------------------------------------------------------
+// ISSUE 3: Headers defaults to Record<string, any>
+//
+// Node's http.IncomingHttpHeaders types headers as string | string[] | undefined.
+// The Record<string, any> default loses this.
+// -----------------------------------------------------------------------------
+
+const issueHeadersAny: ServerRoute = {
+    method: 'GET',
+    path: '/headers',
+    handler: (request, h) => {
+
+        // FIXED: Headers now correctly typed as Record<string, string | string[] | undefined>
+        // @ts-expect-error - headers are string | string[] | undefined, not number
+        const auth: number = request.headers.authorization;
+
+        // FIXED: headers is no longer `any`
+        const headersIsAny: IsAny<typeof request.headers.authorization> = false;
+
+        return 'ok';
+    }
+};
+
+// -----------------------------------------------------------------------------
+// ISSUE 4: Default RequestQuery has [key: string]: any index signature
+//
+// Without a Query override, any access on request.query is `any`.
+// -----------------------------------------------------------------------------
+
+const issueQueryAny: ServerRoute = {
+    method: 'GET',
+    path: '/search',
+    handler: (request, h) => {
+
+        // FIXED: Query now correctly typed as Record<string, string | string[] | undefined>
+        // @ts-expect-error - query values are string | string[] | undefined, not number
+        const page: number = request.query.page;
+        // @ts-expect-error - query values are string | string[] | undefined, not boolean[]
+        const wat: boolean[] = request.query.anything;
+
+        // FIXED: query is no longer `any`
+        const queryIsAny: IsAny<typeof request.query.page> = false;
+
+        return 'ok';
+    }
+};
+
+// -----------------------------------------------------------------------------
+// ISSUE 5: Request<CustomRefs> not assignable to Request<ReqRefDefaults>
+//
+// A function taking Request (no generic) can't accept Request<{ Params: ... }>
+// even though the custom refs only NARROW a property. Users are forced to
+// choose between generic (accepts all) or concrete (sees defaults).
+// -----------------------------------------------------------------------------
+
+export function concreteHelper(req: Request): string | undefined {
+
+    if (req.auth.credentials.extra_id) {
+        return req.auth.credentials.extra_id;
+    }
+
+    return undefined;
+}
+
+interface MyRouteRefs {
+    Params: { id: string };
+    Query: { expand: string };
+}
+
+// KNOWN LIMITATION: Request<MyRouteRefs> is not assignable to Request<ReqRefDefaults>
+// because TypeScript checks generic interface compatibility invariantly when
+// the generic appears in contravariant positions (e.g. lifecycle method parameters).
+// Workaround: use a generic function like processAuthGeneric<Refs> above instead
+// of concrete Request (no generic) for helper functions that need to accept
+// requests with different Refs.
+export function issueConcreteVsGeneric(req: Request<MyRouteRefs>): void {
+
+    // @ts-expect-error - Known TS limitation: Request<CustomRefs> not assignable to Request<ReqRefDefaults>
+    concreteHelper(req);
+}
+
+// -----------------------------------------------------------------------------
+// ISSUE 6: state and preResponses are not extensible through ReqRef
+//
+// These properties use hardcoded Record<string, any> and are NOT wired
+// through InternalRequestDefaults/ReqRef, so users can't type them.
+// -----------------------------------------------------------------------------
+
+const issueStateAny: ServerRoute = {
+    method: 'GET',
+    path: '/state',
+    handler: (request, h) => {
+
+        // FIXED: state is now Record<string, unknown> — requires type narrowing
+        // @ts-expect-error - state values are unknown, not directly assignable to number
+        const session: number = request.state.session;
+
+        // FIXED: state is no longer `any`
+        const stateIsAny: IsAny<typeof request.state.session> = false;
+
+        // FIXED: preResponses is no longer `any`
+        const preRespIsAny: IsAny<typeof request.preResponses.myPre> = false;
+
+        return 'ok';
+    }
+};