nexus-plugin-jwt-auth icon indicating copy to clipboard operation
nexus-plugin-jwt-auth copied to clipboard

Type definition of "token" on context not correct

Open kylejreed opened this issue 4 years ago • 12 comments

From image below, the type definition of token is String | null, but when logging the object, it is actually the deserialized object. Is there a way to pass in a type/interface as a part of the setup for type-safety?

image

I'm not a typescript expert, so I don't know if its possible to stringify an interface/type, but it would be cool if we could pass our own type-string into the token definition.

image

I am new to contributing to open source projects, but I could implement something if we can come up with a good solution

kylejreed avatar May 02 '20 18:05 kylejreed

Thanks for bringing this up! I had a conversation with the creator of https://github.com/lvauvillier/nexus-plugin-shield and the way we discussed handling this is by having a token interface that at least contains some common JWT object properties.

export interface Token {
  sub?: string
  iss?: string
  aud?: string
  exp?: number
  iat?: number
  name?: string
  admin?: boolean
}

Camji55 avatar May 06 '20 01:05 Camji55

Interesting thought too about passing an optional parameter to the plugin. Possible can be done by leveraging generics? Looking into it 😄

Camji55 avatar May 07 '20 02:05 Camji55

I like the idea of the generic a lot. At the very least, when I go into the source of the plugin (runtime.js ~ line 30) and change the type string to my desired version:

token: '{ userId: number, username: string}'

The type safety works as I would expect. I think generics would be nicer than that but its definitely an option

kylejreed avatar May 07 '20 13:05 kylejreed

@kylejreed I was able to look into this a bit more this morning and this is what I've come up with:

function genSignature<T extends object>(object: T) {
    const keys = Object.keys(object)
    const values = keys.map(key => { return object[key]})
    let properties: String[] = []

    for (let index = 0; index < keys.length; index++) {
        const key = keys[index]
        const value = values[index]

        if (typeof value == 'object') {
            properties.push(' ' + key + ': ' + genSignature(value) + ' | undefined | null')
        } else {
            properties.push(' ' + key + ': ' + typeof value + ' | undefined | null')
        }
    }

    return '{' + properties + ' }'
}

Usage of this function would look like:

interface Token {
    sub?: string
    iss?: string
    aud?: string
    exp?: number
    iat?: number
    name?: string
    admin?: boolean
}

let token: Token = {
    sub: '123',
    iss: '14r',
    aud: 'wow',
    exp: 0,
    iat: 1,
    name: 'Test',
    admin: false
}

// standard
console.log(genSignature(token))
// returns '{ sub: string | undefined | null, iss: string | undefined | null, aud: string | undefined | null, exp: number | undefined | null, iat: number | undefined | null, name: string | undefined | null, admin: boolean | undefined | null }

interface User {
    name?: string
    id?: number
    token?: Token
}

const user: User = {
    name: 'Cameron',
    id: 12,
    token
}

// recursive
console.log(genSignature(user))
// returns '{ name: string | undefined | null, id: number | undefined | null, token: { sub: string | undefined | null, iss: string | undefined | null, aud: string | undefined | null, exp: number | undefined | null, iat: number | undefined | null, name: string | undefined | null, admin: boolean | undefined | null } | undefined | null }'

This function can take an object and generate it's signature. This signature can then be passed into the (runtime.js ~ line 30) typegen.

Here are the drawbacks I'd like to fix:

  • The function currently takes an instance of an interface rather than the type itself. Ideally I could just pass the interface directly. I was having a hard time coming across any documentation on how this might be possible.
  • The function has no way of determining whether or not a property can be optional, therefore I append the types of null and undefined to every value. Ideally the function could check whether of not the property is optional and then and only then apply the null | undefined to the type.

Thoughts? I'm relatively new to Typescript and would love to hear any advice on how this can be improved.

Camji55 avatar May 23 '20 18:05 Camji55

Hi @Camji55,

Take a look how nexus generates context types from addToContext user function calls. They walk through sources files and use the ts compiler to retrieve types and signatures: https://github.com/graphql-nexus/nexus/blob/master/src/lib/add-to-context-extractor/extractor.ts#L24

This is an example to how nexus properly generates types if the token extraction is done manually and pass through an addToContext call:

// app.ts
import { extractToken } from "./helpers/token"

schema.addToContext((req) => {
  return {
    token: extractToken(req)
  };
});

// helpers/token.ts

export interface Token {
  sub?: string;
  iss?: string;
  aud?: string;
  exp?: number;
  iat?: number;
  name?: string;
  admin?: boolean;
}

export function extractToken(req: any) {
  const Authorization = req.get("Authorization");
  if (Authorization) {
    const token = Authorization.replace("Bearer ", "");
    return verify(token, process.env.TOKEN_SECRET || "") as Token;
  }
}

In the nexus extractor, the ts compiler extract the signature of my extractToken function and generates types properly

// content of generated typegen-nexus-context/index.d.ts

import app from 'nexus'

// Imports for types referenced by context types.

import { Token } from '/path_to_my_project/src/helpers/token'

// Tap into Nexus' global context interface. Make all local context interfaces merge into it.

declare global {
  export interface NexusContext extends Context {}
}

// The context types extracted from the app.

interface Context { token: Token | undefined; }

I think you can do the same and extend the plugin with a worktime script follows the same trick with a plugin entry point like this:


// app.ts

export interface Token {
  sub?: string;
  iss?: string;
  aud?: string;
  exp?: number;
  iat?: number;
  name?: string;
  admin?: boolean;
}


use(auth<Token>({
  appSecret: "<YOUR SECRET>" // required
}))

The worktime script has to use the ts compiler has to extract the signature and catch the generic type and generates a definition file extending NexusContext with an import of the Token interface.

I'm not sure if it's faisable, but it seems to ben the pattern in nexus framework : use worktime scripts to generate safety for the end user.

Hope this helps.

lvauvillier avatar May 25 '20 20:05 lvauvillier

In the meantime, simpler version to inject custom types to NexusContext:

interface Token {
  sub?: string;
  iss?: string;
  aud?: string;
  exp?: number;
  iat?: number;
  name?: string;
  admin?: boolean;
}

declare global {
  interface NexusContext extends Context {}
}
interface Context {
  token: Token;
}

use(auth({
  appSecret: "<YOUR SECRET>" // required
}))

lvauvillier avatar May 25 '20 20:05 lvauvillier

In the meantime, simpler version to inject custom types to NexusContext:

interface Token {
  sub?: string;
  iss?: string;
  aud?: string;
  exp?: number;
  iat?: number;
  name?: string;
  admin?: boolean;
}

declare global {
  interface NexusContext extends Context {}
}
interface Context {
  token: Token;
}

use(auth({
  appSecret: "<YOUR SECRET>" // required
}))

I have an issue with this saying:

Interface 'NexusContext' cannot simultaneously extend types 'Context' and 'Context'.
  Named property ''token'' of types 'Context' and 'Context' are not identical.ts(2320)

when i try to put quotes around the token they get revoked but when i look into the index.d.ts on the typegen-nexus i see that there are quotes around the token which remain. Don't know what the magic behind this is...

Maetes avatar Jul 30 '20 16:07 Maetes

Any update on this? @lvauvillier I also wasn't able to get your workaround to work. I get the following error (looks like @Maetes got the same one):

Interface 'NexusContext' cannot simultaneously extend types 'Context' and 'Context'.
  Named property ''token'' of types 'Context' and 'Context' are not identical.

zingerj avatar Aug 16 '20 05:08 zingerj

@zingerj Same here

brielov avatar Aug 16 '20 19:08 brielov

There is something like a read-only thing on the typegen-nexus @type... or its a bug... but i also tried to change the typegen js, removing the single quotes but without success, you simply aren't able to ammend the Context interface props... even not with a mapped type.

you can omit it using @ts-ignore or do smoething like this:

validUser = (ctx.token as unknown) as { userId: number }

i prefer the ts-ignore workaround

Maetes avatar Aug 18 '20 05:08 Maetes

Any news on this?

brielov avatar Sep 06 '20 20:09 brielov

Thanks @homerjam for PR #58 which gives a decent workaround for the time being! Release v1.4.0 includes his changes which adds a setting for declaring the token type as a string.

Camji55 avatar Sep 17 '20 17:09 Camji55