redwoodjs / redwood

The App Framework for Startups
https://redwoodjs.com
MIT License
17.35k stars 999 forks source link

`requireAuth` does not work with custom functions #3727

Closed olance closed 3 years ago

olance commented 3 years ago

I created a custom function and I need to check that it is called by an authenticated user. I naively added a requireAuth call at the top of my function's handler, but realized it doesn't work.

What I have done

export const handler = async (event, context) => { requireAuth() ... }


- I'm using Firebase auth. I logged into my app, inspected the React components and got my Firebase ID token
- I then used Postman to call my function with two headers:

authorization: Bearer auth-provider: firebase

- I confirmed, using a dummy cell, that the same headers are sent along with GraphQL queries and work as the authentication mean
- I logged `event` from my function to make sure both headers were indeed received:
![image](https://user-images.githubusercontent.com/314079/141663341-91e224eb-dffd-480a-9dd4-c1ea4bed2578.png)

**What's happening**
![image](https://user-images.githubusercontent.com/314079/141663364-dae1d651-3958-49f1-aaa2-fd34afa82033.png)

Note the added `console.error`: the context is an empty object.

**What should happen**
My function should confirm my authentication state and execute properly

**What I have tried/explored**
@dthyresson told me the other day that "any Graphql request that has two headers authorization and auth-provider: <type>  will try to authenticate using the decoder for the type".
Well, first, I forgot about the "Graphql" at the beginning of this sentence and assumed any request with proper headers would authenticate properly 😅 

I looked around and found that `createGraphQLHandler` sets up an Envelop plugin that will indeed read the auth headers and update the global context with the current authenticated user if any.

I tried to reproduce that plugin's behaviour in my function, which I changed to:

```js
import { context as globalContext, setContext } from '@redwoodjs/graphql-server'
import { getAuthenticationContext } from '@redwoodjs/api'

export const handler = async (event, context) => {
  const authContext = await getAuthenticationContext({ event, context })
  if (authContext) {
    setContext({
      ...globalContext,
      currentUser: await getCurrentUser(
        authContext[0],
        authContext[1],
        authContext[2]
      ),
    })
  }

  requireAuth()

A few console.log later I could see that this time, my Firebase token was being read and decoded correctly, and currentUser was set on the global context... however it seems there's something a bit fishy going on with the context, as context does contain a currentUser key with correct values, but context.currentUser returns undefined 😐

image


A few things here:

export const handler = createFunctionHandler(async (event, context) => {
  ...
})

where createFunctionHandler would setup the appropriate context reading/populating, and then call the actual handler once it's done?

dthyresson commented 3 years ago

@olance You are so correct that:

I completely agree with:

As a Redwood user, I don't think I should know about the global context, the authentication context and how to populate the current user data into the global context.

I'd like to see the behavior be much more like the web hook validation / verification:

Perhaps some thing like verifyEvent but checks for the auth-provider header and checks the token/cookie?

See: https://redwoodjs.com/docs/webhooks#how-to-receive-and-verify-an-incoming-webhook

dthyresson commented 3 years ago

@olance Maybe verifyAuth(role: ['admin'])? And raise Auth/Forbidden error if not, and return result of getCurrentUser if verified?

Would that work?

olance commented 3 years ago

@dthyresson sorry I hadn't seen your own issue! Maybe we should merge them?

However, I wonder why you'd go with a different function than requireAuth? I'm pretty sure there should be a way to make it work seamlessly within non-GraphQL serverless functions, it's just a matter of correctly populating the context.

Also I looked at the Webhook docs and I feel there's room for a much easier/developer friendlier approach 🙂 Scratch that, I thought function decorators were a thing in JS/TS :(

I'll try to write up a proposal!

doowb commented 3 years ago

@olance I'm also attempting to use requireAuth() in a custom serverless function and saw how the context was only created in the grapqhql function.

With the code you provided above, I'm able to use the updated global context if I "disable context isolation".

I did that in a wrapper function to ensure that I restore the environment variable, but I don't know if there will be any other effects from that settings.

dthyresson commented 3 years ago

I am coming up with a few ideas -- but, just out of curiosity @olance and @doowb -- if your function needs Redwood user auth, why use a serverless custom function over a GraphQL service? Is there something specific a function can do? Or is the function being used outside the web side?

olance commented 3 years ago

Yeah in my case, I was planning on using this function from outside of the web side. From a Chrome/Firefox extension to be precise :)

I chose to change my approach here and indeed use a regular GraphQL service, mainly for the sake of not having to wait for changes in Redwood to be honest though. I guess in the end it shall work, and I don't think there would be an issue with sending GraphQL requests from an extension.

However, I don't think it invalidates this issue ^^

dthyresson commented 3 years ago

I though that might be one of the use cases.

And I agree that just using GraphQL auth doesn’t make this issue go away.

Going to plan out a few options on Monday to handle a few scenarios. Thanks.