Skip to content
This repository has been archived by the owner on Oct 14, 2020. It is now read-only.

Type definition of "token" on context not correct #23

Open
kylejreed opened this issue May 2, 2020 · 12 comments
Open

Type definition of "token" on context not correct #23

kylejreed opened this issue May 2, 2020 · 12 comments
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@kylejreed
Copy link

kylejreed commented May 2, 2020

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

@Camji55 Camji55 self-assigned this May 6, 2020
@Camji55 Camji55 added the bug Something isn't working label May 6, 2020
@Camji55
Copy link
Owner

Camji55 commented May 6, 2020

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
Copy link
Owner

Camji55 commented May 7, 2020

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

@kylejreed
Copy link
Author

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

@Camji55 Camji55 added the help wanted Extra attention is needed label May 12, 2020
@Camji55
Copy link
Owner

Camji55 commented May 23, 2020

@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.

@lvauvillier
Copy link

lvauvillier commented May 25, 2020

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
Copy link

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
}))

@Maetes
Copy link

Maetes commented Jul 30, 2020

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...

@zingerj
Copy link

zingerj commented Aug 16, 2020

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.

@brielov
Copy link

brielov commented Aug 16, 2020

@zingerj Same here

@Maetes
Copy link

Maetes commented Aug 18, 2020

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

@brielov
Copy link

brielov commented Sep 6, 2020

Any news on this?

@Camji55
Copy link
Owner

Camji55 commented Sep 17, 2020

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants