accounts-js / accounts

Fullstack authentication and accounts-management for Javascript.
https://www.accountsjs.com/
MIT License
1.49k stars 142 forks source link

Support for @nestjs/graphql #506

Open marcus-sa opened 5 years ago

marcus-sa commented 5 years ago

Nest is great and is widely used among the TS community with their GraphQL module.

moltar commented 5 years ago

Here's a working solution. Obviously needs some refactoring, but it's a good start.

This also uses @accounts/typeorm, which is optional, of course.

import { Module } from '@nestjs/common';
import { GraphQLModule } from '@nestjs/graphql';
import { AccountsBoost } from '@accounts/boost';
import { createConnection } from 'typeorm';
import { AccountsTypeorm, entities } from '@accounts/typeorm';
import AccountsPassword from '@accounts/password';
import { print } from 'graphql/language/printer';

@Module({
  imports: [
    GraphQLModule.forRootAsync({
      useFactory: async () => {
        const connection = await createConnection({
          type: 'postgres',
          url: 'postgres://localhost/nestaccounts',
          entities,
          synchronize: true,
        });

        const db = new AccountsTypeorm({
          connection,
        });

        const password = new AccountsPassword();

        const accountBoost = new AccountsBoost(
          {
            db,
            tokenSecret: 'abc',
          },
          {
            password,
          },
        );

        const accounts = await accountBoost.graphql();

        const typeDefs = print(accounts.typeDefs.definitions);

        return {
          typeDefs,
          resolvers: accounts.resolvers,
          schemaDirectives: {
            // In order for the `@auth` directive to work
            ...accounts.schemaDirectives,
          },
          context: ({ req }) => accounts.context({ req }),
        };
      },
    }),
  ],
})
export class AppModule { }
moltar commented 5 years ago

The only thing I am unhappy about is this:

const typeDefs = print(accounts.typeDefs.definitions);

The @nestjs/graphql module expects typeDefs to be a string, but accounts.typeDefs is a DocumentNode type. So print simply stringifies the types back into a schema definition string.

And the reason I am unhappy is that it feels like just an unneeded effort. String -> AST -> string.

NickBolles commented 5 years ago

@moltar After fixing the schema option in my PR: https://github.com/nestjs/graphql/pull/317 you should be able to just do

        const {schema, context} = await accountBoost.graphql();

        return {
          schema,
          context
          // or if you want to customize the context:
         context: ({req} => {something: "foo", ...(await accounts.context({req}))}
        };

I'm also building a nestjs module that will make this a lot more nestjs friendly and include nest routes, decorators guards etc. It's going a bit slow since I'm moving right now, but I have a semi-working version right now. Let me know if you want access to it. I'd be curious to get feedback on it.

NickBolles commented 4 years ago

@moltar @marcus-sa I would love some input on my PR for a nestjs module for accounts. See my PR #840

TrejGun commented 4 years ago

@NickBolles

Hi man! I'm new here so may ask a stupid question but still

Recently I started working with Nest and have compiled couple of articles about authentificartion in nest. you can find my blog So I was googling graphql auth for nestjs and found this issue. It looks promising to me!

  1. Public routes inside globally protected applications

Especially I like part when you can queue auth validators

@AuthValidator(IsDarthVader)
class DarthVader {
    @Get()
    @UseGuards(AuthGuard)
    @AuthValidator(TalkingToLuke, AsyncValidator)
    superSecret() {
        return "Luke, I am your father"
    }
}

here goes my first question. so I would like to have a way to make all rotes protected assigning AuthGuard for the whole class or globally but have one/few exceptional public method

@UseGuards(AuthGuard) // for class
@AuthValidator(IsDarthVader)
class DarthVader {
    @Get("/private")
    private() {
        return "I'm private"
    }
    @Get("/piblic")
    piblic() {
        return "I'm public"
    }
}
async function bootstrap(): Promise<void> {
  const app = await NestFactory.create(AppModule);

  const reflector = app.get(Reflector);
  app.useGlobalGuards(new AuthGuard(reflector)); // globally

  await app.listen(process.env.PORT);
}

In my solution there is @Public decorator which adds exception for route, in yours it could be two solutions. first one is same as mine, and second one is a special validator which somehow bypass runValidators function

@UseGuards(AuthGuard)
@AuthValidator(IsDarthVader)
class DarthVader {    
    @Public()
    @Get("/piblic")
    public() {
        return "I'm public"
    }

    @AuthValidator(Public)
    @Get("/piblic")
    public2() {
        return "I'm public too"
    }
}
  1. Determinate wether it is gql resolver or rest endpoing

I'm a bit disapointed nestjs does not provide an easy way to do this. And your approach looks a bit overkill but I don`t have good solution yet.

@Injectable()
export class AuthGuard implements CanActivate {
  constructor(private readonly reflector: Reflector) {}

  async canActivate(context: ExecutionContext): Promise<boolean> {
    const ctx = GqlExecutionContext.create(context);
    const gqlCtx = ctx.getContext();
    if (gqlCtx) {
      try {
        const gqlParams: GQLParam = [ctx.getRoot(), ctx.getArgs(), gqlCtx, ctx.getInfo()];
        // use the authenticated function from accounts-js. All that's really needed is context
        await authenticated(() => null)(...gqlParams);
        return this.runValidators(gqlCtx.user, context, gqlParams);
      } catch (e) {
        return false;
      }
    }
    const req = context.switchToHttp().getRequest();
    if (!req || !req.user) {
      return false;
    }
    return this.runValidators(req.user, context, req);
  }
}

May be it make sense to check your own AccountsJsModule.options for GraphQL/REST first

  1. Server?

In context of account.js

class AppAccountsOptionsFactory implements AccountsOptionsFactory {
  constructor(@Inject(ConfigService) private readonly configService: ConfigService) {}
  createAccountsOptions(): NestAccountsOptionsResult {
    return {
      serverOptions: {
        db: new Mongo(),
        tokenSecret: this.configService.get('secret'),
      },
      services: {
        password: new AccountsPassword(),
      },
      REST: true, // or an Object with any @accounts/rest options
      GraphQL: true, // or an Object with any @accounts/graphql-api options
    };
  }
}

this code says serverOptions? What? Does it create (another) server for me? What kind of server (express) ? Why do I need it at all? Please elaborate on this.

  1. TypeORM

I used to consume nestjs with typeorm, how could I share connection? Lets say I have already existing users table with email/password fields, module, controller, service whatever, I just want to plug in accounts.js to handle auth instead of passport.js

@Module({
  imports: [
    TypeOrmModule.forRoot(ormconfig), // <-- from here
    AccountsJsModule.register({
      serverOptions: {
        db: new Mongo(), // <-- to here
        tokenSecret: 'secret',
      },
      services: {
        password: new AccountsPassword(),
      },
      REST: true,
      GraphQL: true,
    }),
    UserModule
  ],
})
export class AppModule {}

Thanks for attention, hope you like my input.

and thanks for your code!

TrejGun commented 4 years ago

It also looks like you have a mistype in

@Injectable()
export class AuthGuard implements CanActivate {
  constructor(private readonly reflector: Reflector) {}

  async canActivate(context: ExecutionContext): Promise<boolean> {
    const ctx = GqlExecutionContext.create(context);
    const gqlCtx = ctx.getContext();
    if (gqlCtx) {
      try {
        const gqlParams: GQLParam = [ctx.getRoot(), ctx.getArgs(), gqlCtx, ctx.getInfo()];
        // use the authenticated function from accounts-js. All that's really needed is context
        await authenticated(() => null)(...gqlParams);
        return this.runValidators(gqlCtx.user, context, gqlParams);
      } catch (e) {
        return false;
      }
    }
    const req = context.switchToHttp().getRequest();
    if (!req || !req.user) {
      return false;
    }
    return this.runValidators(req.user, context, req);
  }
}

it should be gqlCtx.req.user not gqlCtx.user

NickBolles commented 4 years ago

@TrejGun great comments. Do you mind adding these as review comments on the pull request? it's easier to triage them that way.

A few quick comments, although I'd encourage you to read through the readme first, It seems like several of these are answered in there

so something like this app.module.ts

import { Module } from '@nestjs/common';
import { TypeOrmModule } from '@nestjs/typeorm';
import { AccountsJsModule } from '@nb/nestjs-accountsjs';
import { AppAccountsOptionsFactory } from './AppAccountsOptionsFactory';

@Module({
  imports: [
    TypeOrmModule.forRoot(ormconfig), // <-- from here
    AccountsJsModule.registerAsync({ useClass: AppAccountsOptionsFactory })],
})
export class AppModule {}

AppAccountsOptionsFactory.ts

import { Connection } from 'typeorm'; // add this import
import typeorm from '@accounts/typeorm'; //import the accountsjs database adapter

class AppAccountsOptionsFactory implements AccountsOptionsFactory {
  constructor(
    @Inject(ConfigService) private readonly configService: ConfigService,
    @Inject(Connection) private readonly connection: Connection // inject the typeorm connection
   ) {}

  createAccountsOptions(): NestAccountsOptionsResult {
    return {
      serverOptions: {
        db: new typeorm({connection: this.connection }) // Create a new instance of the accounts typeorm database adapter with the existing connection
        tokenSecret: this.configService.get('secret'),
      },
      services: {
        password: new AccountsPassword(),
      },
      REST: true, // or an Object with any @accounts/rest options
      GraphQL: true, // or an Object with any @accounts/graphql-api options
    };
  }
}
TrejGun commented 4 years ago

Oh niiiice. As I said I started to work with nest about 2 month ago and a lot of stuff is not documented so you comment shed some light on why do I need optionFactory at all)))

As for account server - I'm completely unfamiliar with account but I do understand one can setup it as an express server but is it mandatory to setup second server in parallel with nest?

Thanks for your reply!

NickBolles commented 4 years ago

@TrejGun

As for account server - I'm completely unfamiliar with account but I do understand one can setup it as an express server but is it mandatory to setup second server in parallel with nest?

no. That's the whole point of this module, to avoid having any accounts stuff outside of the nest.js context. You pass in either options for the accounts server, or an accounts server instance and it sets up providers so that you can inject it. at that point you should be able to do anything you could with an express app, but inside of nest.js.

edit: To expand on that, I find that most things in nest.js need a small wrapper to wrap it into a provider, interceptor or some other nest structure. That's essentially what this module does, but it has a lot of extras, decorators, etc. Plus a lot of this module is just overdone, since it was my first reusable nest module, it;s a little overkill.

Oh niiiice. As I said I started to work with nest about 2 month ago and a lot of stuff is not documented so you comment shed some light on why do I need optionFactory at all)))

What kind of stuff isn't documented? The nest documentation and the samples are pretty good. The discord channel is another good resource too.

TrejGun commented 4 years ago

@NickBolles nest has really bad docs... sorry to say but it does not give full picture of what's going on. For example it says nothing about ExecutionContext and what switchToHttp does

marcus-sa commented 4 years ago

@TrejGun NestJS doesn't have bad documentation. I assume you're not reading it, because what you're talking about is basically internal stuff. You could do what most people do, and look through existing code utilizing it and then try to understand what's going on.

The reason why stuff like optionsFactory or whatever you wanna call it exists, is so that people can asynchronously instantiate the options using a separate injectable provider (it's kinda self-explanatory lol) And this is explained in the docs, so you've just not read it through.

TrejGun commented 4 years ago

You could do what most people do, and look through existing code utilizing it and then try to understand what's going on. this is not called documentation

also there is differentce between information and experience. I can read and know about existance but havo no idea how to use it in practice. so besides of reading docs you have to have project where you can use your knowledge in practice

NickBolles commented 4 years ago

This issue is getting way off track.

@TrejGun I agree there's a difference between documentation and understanding how to do something and how to implement that for your solution. But that's what software development is all about, solving problems. Again there are a lot of resources for nest. And IMHO the docs are pretty solid and improving every month.

I haven't had a chance to look at the PR again, but feel free to continue to comment on it and when I get the chance I can look at the comments.

TrejGun commented 4 years ago

And IMHO the docs are pretty solid and improving every month. true. and I'm helping as I can.