RevereCRE / relay-nextjs

⚡️ Relay integration for Next.js apps
https://reverecre.github.io/relay-nextjs/
MIT License
251 stars 30 forks source link

How to use `variablesFromContext`? #37

Closed p-null closed 2 years ago

p-null commented 2 years ago

Hi Ryan, thank you so much for the work on this package!

After seraching in the issues, digging into the code and many trials and errors, I couldn't get the following working.

Use case: I'd like to supply user_id in varaibles of a graphql query after user is logged in, possibly available on every page. The user_id is stored in a jwt token. I am using next-auth to create the token, store it in cookie etc.

Things I tried:

  1. Since I have the following serverSideProps defined, I found when debugging, I was able to access user_id from pageProps: ctx.components[ctx.route].props.pageProps?.user_id at some point. Then I tried the following:
  serverSideProps: async (ctx) => {
          const { getToken } = await import("next-auth/jwt");
          const token = await getToken({
              req: ctx.req,
              secret: process.env.SECRET,
          });

          return {
              ...,
              userId: token?.user_id,
          };
      },

  variablesFromContext: async (ctx) => {
      return {
          user_id:  ctx.components[ctx.route].props.pageProps?.user_id,
      };
  },

The page is able to load but the console will print out errors everytime I refresh the page:

error - src/pages/index.tsx (94:36) @ variablesFromContext
TypeError: Cannot read properties of undefined (reading 'undefined')
  92 | 
  93 |         return {
> 94 |             user_id: ctx.components[ctx.route].props.pageProps?.user_id,
     |                                    ^
  95 |         };
  96 |     },
  97 |     createClientEnvironment: () => getClientEnvironment()!,

This runtime error shows every time I visit this page from another page.

  1. Another thing I tried is to directly call getToken from next-auth
    variablesFromContext: async (ctx) => {
        const { getToken } = await import("next-auth/jwt");
        const token = await getToken({
            req: ctx.req,
            secret: process.env.SECRET,
        });

        return {
            user_id: token.user_id
        };
    },

This will have runtime error:

Unhandled Runtime Error
Error: Must pass `req` to JWT getToken()

  69 | }
  70 | const { getToken } = await import("next-auth/jwt");
> 71 | const token = await getToken({
     |                    ^
  72 |     req: ctx.req,
  73 |     secret:  process.env.SECRET,
  74 | });

Not sure if it's relevant but my IDE typescript plugin tells me ctx passed in variablesFromContext has type NextPageContext | NextRouter, which I suppose NextRouter won't work here.

Appreciate any help!

rrdelaney commented 2 years ago

Great question! variablesFromContext is meant to be run on both the client-side and server-side so it doesn't have access to props passed from serverSideProps. It does receive the request context from Next.js, but it's not guaranteed where it's running. Basically trying to extract a token from the request context and passing it along in the query is not supported, and it was made that way intentionally. The user ID shouldn't be a query variable, but should be passed along in a signed cookie or authentication header. Otherwise it's very easy to send a malicious request where the user ID is forged.

p-null commented 2 years ago

That is a very good point and I shouldn't pass user ID in query variables.

Thanks so much for the quick help and pointing out a security risk.

p-null commented 2 years ago

Hi @rrdelaney , I wonder how do we handle a situation where variablesFromContext is an async funciton?

The variable I am trying to get in variablesFromContext has to be called using a async function. Therefore variablesFromContext is a sync funciton too. However, if that is the case, the variables in createServerNetwork will be a Promise<pending>

// /src/lib/server/relay_server_environment.ts
export function createServerNetwork() {
  return Network.create(async (params, variables) => {
    const response = await fetch(
      'https://swapi-graphql.netlify.app/.netlify/functions/index',
      {
        method: 'POST',
        headers: {
          'Content-Type': 'application/json',
        },
        body: JSON.stringify({
          query: params.text,
          variables,
        }),
      }
    );

    return await response.json();
  });
}

export function createServerEnvironment() {
  return new Environment({
    network: createServerNetwork(),
    store: new Store(new RecordSource()),
    isServer: true,
  });
}

I realize that in relay-nextjs, opts.variablesFromContext is called in useEffect() which we can't call async function.

rrdelaney commented 2 years ago

Unfortunately this one isn't really possible due to how the internals of the library + Suspense work. It is possible to pass the variables to the query and call the async function on the server side?

p-null commented 2 years ago

Thanks for clarifying! Now I can't think of other ways to pass the variables to the query.

It is possible to pass the variables to the query and call the async function on the server side?

Could you elaborate a bit more on this solution? What would the function in variablesFromContext be like?

The user ID shouldn't be a query variable, but should be passed along in a signed cookie or authentication header.

After a second thought, I think there are use cases we need to pass user ID as a query variable. Plus the graphql backend I use doesn't support read user ID from a session token for "read" type graphql queries.

I understand the user ID could be forged but the graphql query in this case is a read type request and I've set up column level access control rules so that only certain fields can be seen, and is allowed to be read by everyone.

p-null commented 2 years ago

To illustrate this is not my very specific use case, please let me give a example here.

I am using hasura as graphql backend and it currently doesn't support read variables from session. See this issue.

Given that, we have to pass variable like user ID in query variables.

user ID is stored in session after a user is signed in. I am using next-auth - We can get the session from the client side by useSession which uses React context under the hood so it won't work if it's called in variablesFromContext.

Another way is to use getSession which won't work neither since it's async.

rrdelaney commented 2 years ago

It's a bit hacky but you can inject the user ID into each request's variables at the network level and implicitly depend upon it.

Example query:

query MyPageQuery($userId: ID!) {
  user(id: $userId) {
    ...somePage_UserInfo
  }
}

Example network implementation:

export function createClientNetwork() {
  return Network.create(async (params, variables) => {
    const session = await getSession();
    const response = await fetch('/api/graphql', {
      method: 'POST',
      credentials: 'include',
      headers: {
        'Content-Type': 'application/json',
      },
      body: JSON.stringify({
        query: params.text,
        variables: { ...variables, userId: session?.user?.id }
      }),
    });

    const json = await response.text();
    return JSON.parse(json, withHydrateDatetime);
  });
}
p-null commented 2 years ago

I also just figured out the same solution last night and it works! :smiley: I know it's bit hacky and the query will ask for extra data but I am very glad it's not a block for me now.

It's very kind of you to think about it and give examples here. Thanks very much!

Close it since we've found the solution.

rrdelaney commented 2 years ago

Glad I could help! Definitely want to address the real-world use-cases of this library 🙂