[Bug?]: getAuthProviderHeader does not support getting headers from a plain Request object

[cdubz]

What's not working?

The current definition of getAuthProviderHeader says it supports getting the auth provider header from a APIGatewayProxyEvent or Request:

export const getAuthProviderHeader = (
  event: APIGatewayProxyEvent | Request,
) => {

This works as expected for APIGatewayProxyEvent, but not for Request.

How do we reproduce the bug?

import { getAuthProviderHeader } from '@redwoodjs/api'

const testRequest = new Request('https://redwoodjs.com/')
testRequest.headers.append('auth-provider', 'test')
const authProvider = getAuthProviderHeader(request))

In the above example, authProvider should be test, but it is undefined.

What's your environment? (If it applies)

System:
    OS: Linux 6.10 Debian GNU/Linux 11 (bullseye) 11 (bullseye)
    Shell: 5.1.4 - /bin/bash
  Binaries:
    Node: 20.14.0 - /tmp/xfs-54a3a8c7/node
    Yarn: 4.4.0 - /tmp/xfs-54a3a8c7/yarn
  npmPackages:
    @redwoodjs/auth-dbauth-setup: 8.3.0 => 8.3.0 
    @redwoodjs/cli-data-migrate: 8.3.0 => 8.3.0 
    @redwoodjs/cli-storybook-vite: 8.3.0 => 8.3.0 
    @redwoodjs/core: 8.3.0 => 8.3.0 
    @redwoodjs/realtime: 8.3.0 => 8.3.0 
    @redwoodjs/studio: 12.0.0 => 12.0.0 
  redwood.toml:
    [api]
    port = 8911
    [browser]
    open = false
    [experimental.opentelemetry]
    enabled = false
    [notifications]
    versionUpdates = ["latest"]
    [studio.graphiql.authImpersonation]
    authProvider = "dbAuth"
    email = "${AUTH_IMPERSONATION_USER_EMAIL}"
    userId = "${AUTH_IMPERSONATION_USER_ID}"
    [web]
    title = "Local Public"
    port = 8910
    apiUrl = "${REDWOOD_API_URL}"
    sourceMap = true
    includeEnvironmentVariables = [
      "APP_ENV",
      "CLOUDFLARE_IMAGES_URL",
      "PBS_OAUTH_BASE_URL",
      "PBS_OAUTH_CLIENT_ID",
      "PUBLIC_API_URL",
      "SENTRY_DSN",
    ]

Are you interested in working on this?

• I'm interested in working on this

[dthyresson]

Hi @cdubz I dug into the code and it might be that underneath we're looking for a WHATWG Request vs http or Node Request. See:

redwood/packages/api/src/transforms.ts

Line 1 in 4a7b887

import { Headers, Request as PonyfillRequest } from '@whatwg-node/fetch'

import { Headers, Request as PonyfillRequest } from '@whatwg-node/fetch'

It could be that when getting the event header here

redwood/packages/api/src/transforms.ts

Line 46 in 4a7b887

export const isFetchApiRequest = (

perhaps the Request is of a different type?

Worth trying?

If so, we should fix that type to be clearer.

[cdubz]

Ah, yeah I didn't notice that.

We're integrating the useResponseCache plugin for the GraphQL handler and that is giving us a Node Request at the point where we want to set up a session ID: https://github.com/dotansimha/graphql-yoga/blob/9efdc3831a517890dbb00c783d6da86c6a6d9b4e/packages/plugins/response-cache/src/index.ts#L28

We're having to get the auth provider here because we are using a custom provider alongside dbAuth.

It does work for us with getEventHeader but only if we manually put in Auth-Provider because the AUTH_PROVIDER_HEADER const is all lowercase and getEventHeader only supports either of the provided casing or all lowercase

import { getEventHeader } from '@redwoodjs/api'
const authProvider = getEventHeader(request, 'Auth-Provider')

Re: isFetchApiRequest -- we don't get to it because getAuthProviderHeader doesn't find the header.

I was wondering if it would make sense to adjust getEventHeader to support any casing for headers (e.g., make Auth-Provider, auth-provider, AUTH-PROVIDER all match) and then just use that in getAuthProviderHeader instead of the Object.keys handling? Maybe I'm missing some other issue there.

[dthyresson]

We're integrating the useResponseCache plugin for the GraphQL handler

Interesting -- the reason we use the WHATWG request is in fact because that's what the Guild uses in all GraphQL Yoga.

This https://github.com/ardatan/whatwg-node is managed by Arda from The Guild -- so I'd have though them to be compatible.

I'll ask.

I do also see: https://github.com/dotansimha/graphql-yoga/blob/9efdc3831a517890dbb00c783d6da86c6a6d9b4e/packages/plugins/response-cache/src/index.ts#L134

        const operationId = request.headers.get('If-None-Match');

Maybe it this case since it is custom auth, it makes sense just to get the raw request headers and authorize?

That said when I look at:

export const getAuthProviderHeader = (
  event: APIGatewayProxyEvent | Request,
) => {
  const authProviderKey = Object.keys(event?.headers ?? {}).find(
    (key) => key.toLowerCase() === AUTH_PROVIDER_HEADER,
  )
  if (authProviderKey) {
    return getEventHeader(event, authProviderKey)
  }
  return undefined
}

and your

const authProvider = getEventHeader(request, 'Auth-Provider')

Seems like should work.

Have you tried:

const testRequest = new Request('https://redwoodjs.com/', {
  headers: {
    'auth-provider': 'test',
  },
});

or

testRequest.headers.set('auth-provider', 'test');

But append should mutate the headers in place so kind of scratching my head why.

I still think just getting headers on own for this custom auth may make sense -- at least add a workaround? until we can identify the cause and fix? Maybe?

[cdubz]

To be clear -- getAuthProviderHeader should indeed work. The issue is just that it doesn't work with a plain NodeJS request object.

The alternative approach uses getEventHeader which looks like this:

export const getEventHeader = (
  event: APIGatewayProxyEvent | Request,
  headerName: string,
) => {
  if (isFetchApiRequest(event)) {
    return event.headers.get(headerName)
  }

  return event.headers[headerName] || event.headers[headerName.toLowerCase()]
}

It is only checking for headerName exactly as provided or all lower case. So

const authProvider = getEventHeader(request, 'Auth-Provider')

works fine, but

const authProvider = getEventHeader(request, AUTH_PROVIDER_HEADER)

does not work (if the header is anything other than exactly auth-provider).

It's a minor thing really, but it might be nice to adjust getEventHeader to do something more like getAuthProviderHeader:

export const getEventHeader = (
  event: APIGatewayProxyEvent | Request,
  headerName: string,
) => {
  if (isFetchApiRequest(event)) {
    return event.headers.get(headerName)
  }

  const headerKey = Object.keys(event?.headers ?? {}).find(
    (key) => key.toLowerCase() === headerName.toLowerCase(),
  )

  return headerKey ? event.headers[headerKey] : undefined
}

And from there getAuthProviderHeader could even be adjusted to just do this:

export const getAuthProviderHeader = (
  event: APIGatewayProxyEvent | Request,
) => getEventHeader(event, AUTH_PROVIDER_HEADER)

This would make things work regardless of the casing of the passed in headerName (I think, didn't actually test 😁).

I still think just getting headers on own for this custom auth may make sense -- at least add a workaround? until we can identify the cause and fix? Maybe?

Yes, absolutely. That is what I'm doing and it's really fine. Just trying to think if we can improve these helpful utilities.

[dthyresson]

Just trying to think if we can improve these helpful utilities.

Agreed! Any many thanks for the feedback and suggestions.

I like this change and makes sense --- and tons cleaner.

I'll share with team!

[cdubz]

Sounds good! Note I just made a small edit to fix my example code. I was returning the key name not it's value 🤦