Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make FieldResolver scoped #675

Closed
xinghul opened this issue Jul 29, 2020 · 9 comments
Closed

Make FieldResolver scoped #675

xinghul opened this issue Jul 29, 2020 · 9 comments
Labels
Question ❔ Not future request, proposal or bug issue Wontfix ❌ This will not be worked on

Comments

@xinghul
Copy link

xinghul commented Jul 29, 2020

Hey there, I was asking earlier about making the resolver methods scoped so that I can have public/private schemas based on resolvers passed in, and after upgrading the package to 1.0.0-rc.2 as you suggested, now the problem is solved, which is great.

But I noticed the FieldResolvers (and @Resolver()s in general) are still global, is there a plan to also make those scoped?

Thank you and it's been amazing using type-graphql!

@MichalLytek
Copy link
Owner

But I noticed the FieldResolvers (and @resolver()s in general) are still global, is there a plan to also make those scoped?

Please describe your issue with more details. What do you do in your code, what you expect the schema to look like, what is the reality.

@MichalLytek MichalLytek added Need More Info 🤷‍♂️ Further information is requested Question ❔ Not future request, proposal or bug issue labels Jul 29, 2020
@xinghul
Copy link
Author

xinghul commented Aug 1, 2020

Say for example I have a public resolver:

@injectable()
@Resolver(() => PublicProfile)
export class PublicProfileResolver {
  ...
}

And here's how I build the public schema:

const schema = await TypeGraphQL.buildSchema({
  resolvers: values([PublicProfileResolver]),
  container: containerGetter,
  emitSchemaFile: {
    path: path.join(__dirname, '../../schema.public.gql'),
    commentDescriptions: true,
  },
});

And I also have a private resolver:

@injectable()
@Resolver(() => PrivateProfile)
export class PrivateProfileResolver {
  ...
}

Here's how I build the private schema:

const schema = await TypeGraphQL.buildSchema({
  resolvers: values([PrivateProfileResolver]),
  container: containerGetter,
  emitSchemaFile: {
    path: path.join(__dirname, '../../schema.private.gql'),
    commentDescriptions: true,
  },
});

Ideally I want schema.public.gql to contain type PublicProfile only and schema.private.gql to contain type PrivateProfile only.
But now they both contain type PublicProfile and type PrivateProfile.
(They used to also contain all public/private resolvers methods, after upgrading to the latest version it fixed the issue, now they only contain public/private resolver methods respectively, which is what I wanted.)

I'm also using inversifyjs to inject dependencies(containerGetter), not sure if that's related.

@xinghul
Copy link
Author

xinghul commented Aug 1, 2020

This also happens to FieldResolver, which is a bigger issue, for example I have some private fields on PrivateProfile which I don't want to expose, but now schema.public.gql exposes the fields:

type PublicProfile {
  ...
  accessToken: Token! # this comes from PrivateProfile @FieldResolver(() => Token) which I don't want to expose
  ...
}

@MichalLytek
Copy link
Owner

MichalLytek commented Aug 1, 2020

type PublicProfile
this comes from PrivateProfile

@xinghul Please describe more what is your public and private profile... is this the same class, just aliased in the import clause?

@MichalLytek
Copy link
Owner

Closing for a housekeeping purposes 🔒

@MichalLytek MichalLytek added Wontfix ❌ This will not be worked on and removed Need More Info 🤷‍♂️ Further information is requested labels Aug 12, 2020
@xinghul
Copy link
Author

xinghul commented Aug 16, 2020

Hey sorry for the late reply, I only have one class Profile:

@ObjectType()
class Profile {
  @Field(() => String)
  public name: string;

  @Field(() => String)
  public email: string;
}

And it's being used by two resolvers, one public and one private, the private one has a FieldResolver for accessToken:

@Resolver(() => Profile)
export class PublicProfileResolver {
  ...
}

@Resolver(() => Profile)
export class PrivateProfileResolver {
  ...

  @FieldResolver(() => Token)
  public async accessToken(@Root() profile: Profile): Promise<Token> {
    return getTokenForProfile(profile);
  }
}

And here's how I build the public and private schema:

// public
const schema = await TypeGraphQL.buildSchema({
  resolvers: values([PublicProfileResolver]),
  container: containerGetter,
  emitSchemaFile: {
    path: path.join(__dirname, '../../schema.public.gql'),
    commentDescriptions: true,
  },
});
// private
const schema = await TypeGraphQL.buildSchema({
  resolvers: values([PrivateProfileResolver]),
  container: containerGetter,
  emitSchemaFile: {
    path: path.join(__dirname, '../../schema.private.gql'),
    commentDescriptions: true,
  },
});

And as a result both schema.private.gql and schema.public.gql has the same Profile definition:

type Profile {
  name: String!
  email: String!
  accessToken: Token!
}

Ideally accessToken should only be in schema.private.gql.

Let me know if this makes sense, thanks :)

@smolinari
Copy link
Contributor

@xinghul - Ideally, JWT tokens should be worked with outside of GraphQL. If a user is authenticated or not should be known before they even get to the GraphQL API, as it is common practice to have the information about the user inside the context object of the resolvers. This tutorial covers the subject very well. https://www.youtube.com/watch?v=25GS0MLT8JU

Also, there is no real way to "privatize" schema output or rather introspection. It's all or nothing. So, either someone should be authorized to see the schema/ the docs, or they aren't (assuming that is what you are also looking for).

What you can also do is put in an Authorization decorator on the field resolver, if you want to hide the data from prying eyes, (but this shouldn't be used for JWTs). https://typegraphql.com/docs/authorization.html

Scott

@MichalLytek
Copy link
Owner

@xinghul Thanks for the clarify.

I can reproduce this issue but unfortunately, it's too hardcorded in the codebase:

const fieldMetadata: FieldMetadata = {
name: def.methodName,
schemaName: def.schemaName,
getType: def.getType!,
target: typeClass,
typeOptions: def.typeOptions!,
deprecationReason: def.deprecationReason,
description: def.description,
complexity: def.complexity,
roles: def.roles!,
middlewares: def.middlewares!,
params: def.params!,
directives: def.directives,
extensions: def.extensions,
};
this.collectClassFieldMetadata(fieldMetadata);
typeMetadata.fields!.push(fieldMetadata);

So without rewriting the pipeline (#183) I can't quickly fix that issue 😞
For now please try to make class PrivateProfile extends Profile and class PublicProfile extends Profile and then attach the @FieldResolver to the PrivateProfile class to fix the leak.

@xinghul
Copy link
Author

xinghul commented Aug 16, 2020

@MichalLytek @smolinari thank you guys, this is helpful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question ❔ Not future request, proposal or bug issue Wontfix ❌ This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants