nestjs / docs.nestjs.com

The official documentation https://docs.nestjs.com 📕
MIT License
1.18k stars 1.69k forks source link

NestJS Authenticated sessions documentation has major gaps and is seemingly wrong #237

Open Offlein opened 5 years ago

Offlein commented 5 years ago

I'm submitting a...


[ ] Regression 
[ ] Bug report
[ ] Feature request
[x] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

"Session" instructions in "Authentication" section of documentation is erroneous and this functionality is otherwise undocumented. (And non-obvious.)

Expected behavior

It should be clear how to implement AuthGuard with a cookie-based sessions.

Please see my comment from Sept. 14 here.

At this point I DO have my browser at least storing a cookie after adding stuff directly to my main.ts, which I think is bad form:

import { NestFactory } from '@nestjs/core';
import { AppModule } from './app.module';
import * as session from 'express-session';
import * as passport from 'passport';

async function bootstrap() {
  const app = await NestFactory.create(AppModule);
    app.use(session({
        secret: 'a secret',
        name: 'abcd',
        resave: true,
        saveUninitialized: true
    }));

    app.use(passport.initialize());
    app.use(passport.session());
  await app.listen(3000);
}
bootstrap();

This at least gets me a cookie with key "abcd" in my Chrome Developer Tools "Application" > "Cookies" section, with a long value (s%3Ac1xPblaYplJQL2DLmfIUVoLdLIbkjsYQ.e8Q69kfkdn1Mm96Clk6BMg4Ea0k647RnIkT2bP60Lwc).

In my custom Guard, I can actually inspect the request and see that it has a session property with a Session object in it. The Session object has an id RUHHx_IvcDIPFWt-9MHeZHruw1bSO7-O which is different from what's stored in my cookie -- but that may be normal. The request object also has a sessionID property, with the same ID... And a sessionStore property with a MemoryStore object that has as property of sessions in it, that's just an empty object.

Going back to the documentation's recommended code: just calling super.logIn(theRequestObject) sends me directly to my Google Auth strategy's serializeUser function, which is supposed to callback with the user that gets passed to it and/or an error.

Unfortunately, there is never any user that gets passed to it. I'm not sure how the User is intended to be retrieved from the session memory store, or where it's intended to be set to the memory store...

So there's a ton of stuff missing here. :(

Environment

Nest version: 5.4.0

For Tooling issues:

Other: @kamilmysliwiec It seems at least 3 other people are confused about this. It's very frustrating. I think NestJS is so cool, but I took a break from it largely because I was spinning my wheels with this very issue. It would be one thing if I felt like I was just confused, but as it stands the docs are clearly wrong (or at least misleading), so I don't feel like I can move forward.

Dragory commented 5 years ago

Hey, I was tackling with this same issue, and after digging around the code, I found out the docs aren't entirely wrong - just misleading. This seems to work (in your custom AuthGuard):

async canActivate(context: ExecutionContext): Promise<boolean> {
  const request = context.switchToHttp().getRequest();
  const result = (await super.canActivate(context) as boolean);
  await super.logIn(request);
  return result;
}

The reason the User never gets passed to serializeUser in your case is because super.canActivate(context) is actually what runs the strategy's authentication function, which results in the User object. Hence calling super.logIn() (which calls request.logIn(), which sets up sessions etc.) before that (like the docs suggest) doesn't work, since it doesn't have the User object yet.

That being said, I think the docs could be clarified around this:

alex88 commented 5 years ago

@Dragory yours was the only example I've found on how to extend the AuthGuard, however I get

[Nest] 91203   - 2/16/2019, 4:30:02 PM   [ExceptionsHandler] passport.initialize() middleware not in use +5756ms
Error: passport.initialize() middleware not in use

I don't think we need to use passport.initialize() do we? Btw I'm just trying to make sure that that the user has the active property true, so trying to use request.user after the logIn call

Dragory commented 5 years ago

@alex88 I call both passport.initialize() and passport.session() in my app. I also have 2 guards: 1 for the login itself (which is what's shown above), and 1 for pages that require you to be logged in, which is basically just this:

canActivate(context: ExecutionContext) {
    const req = context.switchToHttp().getRequest();
    if (!req.user) throw new UnauthorizedException();

    return true;
}
bzsozsy commented 5 years ago

Is there any kick off date to this topic? I implemented my app with jwt authentication, but for some reason I have to change it to cookie based one. However based on the above conversation I leave as is for now and don't mess up my code... :/

ejhayes commented 5 years ago

I was able to get this working -- here's the solution I came up with:

configure sessions and passport to use sessions

In main.ts (or wherever you're settting up nest):

app.use(session({
    secret: 'CHANGEME',
    resave: false,
    saveUninitialized: false,
  }));
  app.use(passport.initialize());
  app.use(passport.session());

session from the express-session package.

guards

I'm using passport-local so my LocalAuthGuard class looks like this:

import { Injectable, ExecutionContext, UnauthorizedException, Logger } from '@nestjs/common';
import { AuthGuard } from '@nestjs/passport';

@Injectable()
export class LocalAuthGuard extends AuthGuard('local') {
    async canActivate(context: ExecutionContext): Promise<boolean> {
        const request = context.switchToHttp().getRequest();
        const result = (await super.canActivate(context) as boolean); // THIS MUST BE CALLED FIRST
        await super.logIn(request);
        return result;
    }

    handleRequest(err, user, info) {
        if (err || !user) {
            throw err || new UnauthorizedException();
        }

        return user;
    }
}

configure the serializer

If you look at the implementation of PassportSerializer (from @nestjs/passport) you'll notice that both serializeUser and deserializeUser are abstract....so they need to be implemented by you. Take a look: https://github.com/nestjs/passport/blob/master/lib/passport/passport.serializer.ts

My barebones serializer looks like this:

import { PassportSerializer } from '@nestjs/passport';
import { Injectable } from '@nestjs/common';

@Injectable()
export class CookieSerializer extends PassportSerializer {
    serializeUser(user: any, done: (err: any, id?: any) => void): void {
        done(null, user);
    }

    deserializeUser(payload: any, done: (err: any, id?: any) => void): void {
        done(null, payload);
    }
}

update auth module

Then in the module (for example I'm using AuthModule), this needs to be included. So my AuthModule class looks like this:

import { Module } from '@nestjs/common';
import { AuthService } from './auth.service';
import { AuthController } from './auth.controller';
import { LocalStrategy } from './strategies/local.strategy';
import { PassportModule } from '@nestjs/passport';
import { CookieSerializer } from './cookie.serializer';

@Module({
    imports: [
        PassportModule.register({session: true}),
    ],
    providers: [AuthService, LocalStrategy, CookieSerializer],
    controllers: [AuthController],
    exports: [PassportModule],
})
export class AuthModule { }

session guard

Create a guard to check to see if the user is in the session.

import { CanActivate, ExecutionContext, UnauthorizedException } from '@nestjs/common';

export class SessionGuard implements CanActivate {
    canActivate(context: ExecutionContext): boolean | Promise<boolean> {
        const httpContext = context.switchToHttp();
        const request = httpContext.getRequest();

        try {
            if (request.session.passport.user) {
                return true;
            }
        } catch (e) {
            throw new UnauthorizedException();
        }

    }
}

putting it all together

Here is how this can be used in a controller:

import { Controller, Get, Post, Param, Body, UseGuards, HttpCode, HttpStatus, Req } from '@nestjs/common';
import { CreateUserDto } from './dto/createUser';
import { AuthGuard } from '@nestjs/passport';
import { LoginCredentialsDto } from './dto/login.dto';
import { LocalAuthGuard } from './guards/local.guard';
import { SessionGuard } from './session.guard';
import { SessionUser } from './sessionuser.decorator';

@Controller('auth')
export class AuthController {
    @Get('me')
    @UseGuards(SessionGuard)
    getMetadataArgsStorage(@SessionUser() user: any) {
        return user;
    }

    @Post('login')
    @HttpCode(HttpStatus.OK)
    @UseGuards(LocalAuthGuard)
    login(@Body() login: LoginCredentialsDto, @Req() req): Promise<any> {
        return;
    }
}

You can test this out with cURL/Postman:

Logging in:

$ curl -X POST http://localhost:3000/auth/login -d "username=jdoe&password=1" -v
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 3000 (#0)
> POST /auth/login HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.54.0
> Accept: */*
> Content-Length: 23
> Content-Type: application/x-www-form-urlencoded
>
* upload completely sent off: 23 out of 23 bytes
< HTTP/1.1 200 OK
< X-Powered-By: Express
< Set-Cookie: connect.sid=s%3ADuKdgMobTzL7pHXa9oszUqYRy2J85Eiv.11yD1Gq3NwgdSASUfBYU%2B7sxBIwLH9bqwv1pGtq2T%2BU; Path=/; HttpOnly
< Date: Thu, 25 Apr 2019 21:44:25 GMT
< Connection: keep-alive
< Content-Length: 0
<
* Connection #0 to host localhost left intact

Then to test it out:

$ curl -X GET http://localhost:3000/auth/me -H 'Cookie: connect.sid=s%3AfBdKLAFybLq6xh13NeET3LDEQjfxv7cD.XyZanjMKgAq9SlBUVcDsCdZxJn0auKCgWZCnczaLczQ' -v
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 3000 (#0)
> GET /auth/me HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.54.0
> Accept: */*
> Cookie: connect.sid=s%3ADuKdgMobTzL7pHXa9oszUqYRy2J85Eiv.11yD1Gq3NwgdSASUfBYU%2B7sxBIwLH9bqwv1pGtq2T%2BU
>
< HTTP/1.1 200 OK
< X-Powered-By: Express
< Content-Type: text/html; charset=utf-8
< Content-Length: 43
< ETag: W/"2b-p23HHgfkAEp1vHx70BYS/ngFWwQ"
< Date: Thu, 25 Apr 2019 21:51:04 GMT
< Connection: keep-alive
<
* Connection #0 to host localhost left intact
{"firstName":"John","lastName":"Doe"}

Thanks @kamilmysliwiec for all your work on NestJS -- seems very slick!

Abrissirba commented 5 years ago

@ejhayes You don't happen to have this example in a public git repo? I don't understand where CookieSerializer is used

Insidexa commented 5 years ago

@Abrissirba add to providers array in module

johnbiundo commented 5 years ago

@Abrissirba This repo implements sessions with a similar pattern: https://github.com/johnbiundo/test-auth-chapter-sample

NormySan commented 5 years ago

The solutions provided in the issue make absolutely zero sense to me as a new user of NestJS.

It would be great if someone could update the documentation with how sessions work together with NestJS, express, express-session, and passport with the local guard.

Insidexa commented 5 years ago

@NormySan The solutions provided in the issue works fine. Its problem not of NestJS its problem people how works sessions.

johnbiundo commented 5 years ago

This article covers sessions in detail. Has been reviewed by the NestJS core team. https://dev.to/nestjs/authentication-and-sessions-for-mvc-apps-with-nestjs-55a4

I'll wait a few days to see if there are any further comments before closing.

Insidexa commented 5 years ago

Example project how to use session in NestJS https://github.com/Insidexa/nestjs-session-example

filipjnc commented 5 years ago

Would be great to split the documentation example into two paths after setting up everthing: 1. Session, 2. JWT. Session is industry-standard for authentication IMO.

johnbiundo commented 5 years ago

@filipjnc Yeah, I do think it's worth adding something on sessions to the MVC chapter perhaps.

TrejGun commented 5 years ago

@johnbiundo thank you so much for you code, it helped me a lot

the only thing i want to ask you to implement user module, especially controller properly. its kinda not obvious why code doesnt work when I move code from app conteroller to user controller. The answer is I have to add LocalStrategy, SessionSerializer to imports either in user controller or app controller but as i said this is not obvious

johnbiundo commented 5 years ago

@TrejGun Sorry, I'm not clear on what the issue is. Can you clarify or provide a sample repo?

TrejGun commented 5 years ago

@johnbiundo sorry for confussion

your example uses only user.service and there is no implementation of user.controller, instead /login endpoin lives in app.controller which is on my oppinion is wrong. It should live in users.controller so i created one and moved endpoint. After that I spent some time figuring out what dependencies it requires and why it does not work. To save some time for next who want to do this, I ask you to implemen user.controller. I can share my code with you if you want but i guess you dont really need.

Tanks

johnbiundo commented 5 years ago

@TrejGun Thanks for the additional input. I'll give it some thought and see if I can find time to improve the example.

TrejGun commented 4 years ago

you can find my version of solution in my blog, there are also other articles about nest.js https://trejgun.github.io/

dngconsulting commented 4 years ago

Hi all,

After digging into whole Nestjs framework, I find it really cool. But, the security aspects are very painful. Implementing a simple local-strategy session should not be as difficult as the code @TrejGun has provided (thanx for that btw). We have to provide :

Look at this snippet from Spring Security for example :

@RestController
@RequestMapping("/api")

public class TestService {
    @Secured("ROLE_USER")
    @RequestMapping("/currentUser")    
    public Principal information(Principal principal) {    
       return principal;
    }

  @PreAuthorize("hasRole('ROLE_USER') and hasRole('ROLE_TRAINER')")    
  @RequestMapping("/courses")    
   public List<Course> courses() {        
      return courseDAO.getCourses(); 
   }}

This code can be used for LDAP, local strategy, Google/Facebook and whatever you want without changing any single line of code in your controllers.

With UserGuard/Auth annotations, we tie the controllers code with custom annotations. There's no notion of Principal, only req.session.passport user which is very low level.

We should be able to say in app.module that we want a Passport local strategy (session/basic or other) and all the boilerplate code that we generally see should be generated or implemented in NestJS for us.

My 2 cents

Sami

hegelstad commented 4 years ago

@TrejGun Can you add example on how to add openid-client strategy? It would make it very easy to support a broad range of IdPs.

TrejGun commented 4 years ago

@hegelstad hey! i can, do you have example?

hegelstad commented 4 years ago

@TrejGun See this: https://github.com/onelogin/onelogin-oidc-node/blob/master/5.%20Auth%20Flow%20-%20PKCE/app.js

This is OpenIDConnect example, using Onelogin as the IdP. How can it be implemented within the boundaries of Nest PassportStrategy? I think I have a solution, but it is currently not working yet, I will share it when it works!

hegelstad commented 4 years ago

@TrejGun I made an example that seems to work here: https://github.com/nestjs/docs.nestjs.com/issues/99#issuecomment-557878531

Do you see this as a viable approach, do you see any way to improve it? The useFactory for instantiating the strategy feels a little bit odd, but it works.

TrejGun commented 4 years ago

@hegelstad I don't really like to cretae user on login, otherwise looks nice and clean. i will add this to my repo when I have time

hegelstad commented 4 years ago

@TrejGun Good point, I think it would be better to create a module that loads all the users and roles from IdP (onelogin in this case) and stores it in the UsersStore. Then updates updates to user accounts in the IdP could be pushed via webhooks to update the UsersStore in nest.

Please do add the example, I think it can be helpful.

TrejGun commented 4 years ago

@hegelstad I have ported your code to my repo but I did not check it because I dont have account on onelogin. can you please do me a favour - put your keys in my example and check if it still works? here is a PR https://github.com/TrejGun/session-based-authorization-for-nestjs/pull/4 you can review and/or push to the same branch

hegelstad commented 4 years ago

@TrejGun please see the PR I made https://github.com/TrejGun/session-based-authorization-for-nestjs/pull/5

rmainwork commented 4 years ago

Just chipping into this just now. I'm using the SAML strategy with our custom in-house SAML authentication source. I need a way to set-up a fake user session in development (so you open the app and you're logged in).

In app.module.js I'm trying to call req.login(fakeUser) which gives me an error message saying: [Nest] 979 - 02/04/2020, 5:21:58 PM [ExceptionsHandler] passport.initialize() middleware not in use +2053ms. and and samlStrategy.success(user) which produces an error saying that samlStraegy.success is not a function.

I know passport does have a req.login method and you have to call passport.initialize() first, but calling passport.initialize() throws saying that initialize is not a function

I have a SAML strategy as follows:

@Injectable()
class SAMLStrategy extends PassportStrategy(Strategy) {
  public constructor(config: ConfigService) {
    super({
      entryPoint: config.get('CAS_URL'),
      issuer: 'passport-saml',
      host: config.get('EXTERNAL_URL'),
    });
  }

  public async validate(profile?: Profile): Promise<User> {
    if (!profile) {
      throw new UnauthorizedException();
    } else {
      return new User({
        eppn: profile.eppn,
        firstName: profile.givenName,
        lastName: profile.sn,
        email: profile.email,
      });
    }
  }
}

export { SAMLStrategy };
TrejGun commented 4 years ago

@rmainseas just follow my examples https://trejgun.github.io/ ad if it was useful I will appreciate RP with SAML

rmainwork commented 4 years ago

@TrejGun is there a specific post I should look at?

TrejGun commented 4 years ago

@rmainseas https://github.com/GemunIon/nestjs-auth has more strategies than others

rmainwork commented 4 years ago

@TrejGun Yeah none of that really seemed applicable here(unless I'm missing something), but thanks

Bishal18 commented 3 years ago

I was able to get this working -- here's the solution I came up with:

configure sessions and passport to use sessions

In main.ts (or wherever you're settting up nest):

app.use(session({
    secret: 'CHANGEME',
    resave: false,
    saveUninitialized: false,
  }));
  app.use(passport.initialize());
  app.use(passport.session());

session from the express-session package.

guards

I'm using passport-local so my LocalAuthGuard class looks like this:

import { Injectable, ExecutionContext, UnauthorizedException, Logger } from '@nestjs/common';
import { AuthGuard } from '@nestjs/passport';

@Injectable()
export class LocalAuthGuard extends AuthGuard('local') {
    async canActivate(context: ExecutionContext): Promise<boolean> {
        const request = context.switchToHttp().getRequest();
        const result = (await super.canActivate(context) as boolean); // THIS MUST BE CALLED FIRST
        await super.logIn(request);
        return result;
    }

    handleRequest(err, user, info) {
        if (err || !user) {
            throw err || new UnauthorizedException();
        }

        return user;
    }
}

configure the serializer

If you look at the implementation of PassportSerializer (from @nestjs/passport) you'll notice that both serializeUser and deserializeUser are abstract....so they need to be implemented by you. Take a look: https://github.com/nestjs/passport/blob/master/lib/passport/passport.serializer.ts

My barebones serializer looks like this:

import { PassportSerializer } from '@nestjs/passport';
import { Injectable } from '@nestjs/common';

@Injectable()
export class CookieSerializer extends PassportSerializer {
    serializeUser(user: any, done: (err: any, id?: any) => void): void {
        done(null, user);
    }

    deserializeUser(payload: any, done: (err: any, id?: any) => void): void {
        done(null, payload);
    }
}

update auth module

Then in the module (for example I'm using AuthModule), this needs to be included. So my AuthModule class looks like this:

import { Module } from '@nestjs/common';
import { AuthService } from './auth.service';
import { AuthController } from './auth.controller';
import { LocalStrategy } from './strategies/local.strategy';
import { PassportModule } from '@nestjs/passport';
import { CookieSerializer } from './cookie.serializer';

@Module({
    imports: [
        PassportModule.register({session: true}),
    ],
    providers: [AuthService, LocalStrategy, CookieSerializer],
    controllers: [AuthController],
    exports: [PassportModule],
})
export class AuthModule { }

session guard

Create a guard to check to see if the user is in the session.

import { CanActivate, ExecutionContext, UnauthorizedException } from '@nestjs/common';

export class SessionGuard implements CanActivate {
    canActivate(context: ExecutionContext): boolean | Promise<boolean> {
        const httpContext = context.switchToHttp();
        const request = httpContext.getRequest();

        try {
            if (request.session.passport.user) {
                return true;
            }
        } catch (e) {
            throw new UnauthorizedException();
        }

    }
}

putting it all together

Here is how this can be used in a controller:

import { Controller, Get, Post, Param, Body, UseGuards, HttpCode, HttpStatus, Req } from '@nestjs/common';
import { CreateUserDto } from './dto/createUser';
import { AuthGuard } from '@nestjs/passport';
import { LoginCredentialsDto } from './dto/login.dto';
import { LocalAuthGuard } from './guards/local.guard';
import { SessionGuard } from './session.guard';
import { SessionUser } from './sessionuser.decorator';

@Controller('auth')
export class AuthController {
    @Get('me')
    @UseGuards(SessionGuard)
    getMetadataArgsStorage(@SessionUser() user: any) {
        return user;
    }

    @Post('login')
    @HttpCode(HttpStatus.OK)
    @UseGuards(LocalAuthGuard)
    login(@Body() login: LoginCredentialsDto, @Req() req): Promise<any> {
        return;
    }
}

You can test this out with cURL/Postman:

Logging in:

$ curl -X POST http://localhost:3000/auth/login -d "username=jdoe&password=1" -v
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 3000 (#0)
> POST /auth/login HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.54.0
> Accept: */*
> Content-Length: 23
> Content-Type: application/x-www-form-urlencoded
>
* upload completely sent off: 23 out of 23 bytes
< HTTP/1.1 200 OK
< X-Powered-By: Express
< Set-Cookie: connect.sid=s%3ADuKdgMobTzL7pHXa9oszUqYRy2J85Eiv.11yD1Gq3NwgdSASUfBYU%2B7sxBIwLH9bqwv1pGtq2T%2BU; Path=/; HttpOnly
< Date: Thu, 25 Apr 2019 21:44:25 GMT
< Connection: keep-alive
< Content-Length: 0
<
* Connection #0 to host localhost left intact

Then to test it out:

$ curl -X GET http://localhost:3000/auth/me -H 'Cookie: connect.sid=s%3AfBdKLAFybLq6xh13NeET3LDEQjfxv7cD.XyZanjMKgAq9SlBUVcDsCdZxJn0auKCgWZCnczaLczQ' -v
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 3000 (#0)
> GET /auth/me HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.54.0
> Accept: */*
> Cookie: connect.sid=s%3ADuKdgMobTzL7pHXa9oszUqYRy2J85Eiv.11yD1Gq3NwgdSASUfBYU%2B7sxBIwLH9bqwv1pGtq2T%2BU
>
< HTTP/1.1 200 OK
< X-Powered-By: Express
< Content-Type: text/html; charset=utf-8
< Content-Length: 43
< ETag: W/"2b-p23HHgfkAEp1vHx70BYS/ngFWwQ"
< Date: Thu, 25 Apr 2019 21:51:04 GMT
< Connection: keep-alive
<
* Connection #0 to host localhost left intact
{"firstName":"John","lastName":"Doe"}

Thanks @kamilmysliwiec for all your work on NestJS -- seems very slick!

Hey, really helpful this! :) But, only thing i am still bothered about is why do i need to bootstrap the initialize and session methods, especially when we can do that in the module initialisation like so:

PassportModule.register({ session: true });

I know, without the initialisation the solution doesn't work but then why is there an option to register a session in the module?

Thanks again!

yoni333 commented 2 years ago

@ejhayes You don't happen to have this example in a public git repo? I don't understand where CookieSerializer is used

i also didnt understood - after we insert the 'CookieSerializer' to the providers array. who inject it ? who call it ? i read all the files nd projects attach here - but it is like a magic....

jmcdo29 commented 2 years ago

To add another resource, there's this article by me that does a deep dive on passport with Nest and shows how to set up a session based authentication with passport, Nest, and Redis

mikehaertl commented 2 years ago

Is it just me or does the documented solution feel very strange?

  1. It uses a guard to perform the actual login. To me this sounds like a mutation. Guards should not do mutations IMO. (From the docs: "Guards have a single responsibility. They determine whether a given request will be handled by the route handler or not")
  2. It still requires manual setup of passport, i.e. a call to passport.initialize() and passport.session()
  3. It also requires manual call of login() even though passport usually does that automatically under the hood. Why?
  4. It breaks validation as pipes are exectued after guards. I.e. if I have a dto with an email field as username I can not validate the email address with class-validator (unless I rebuild the validation logic in yet another guard, as someone suggested on Discord)
  5. Making it work with your own DTO complicates things even more as you have to manually read out the DTO object from the request and set the parsed object on req.body to let passport-local do it's job
  6. The whole setup has so many moving parts and is extremely hard to follow. Which would not be a problem per se if the actual task wasn't so easy without nest.

Given all this I wonder why we can't use a simple controller action or - as I'm using GraphQL - a mutation as usual. Something like this:

import { LoginUserInput } from './dto/login-user.input';
import { AuthService } from './auth.service';

export class AuthResolver {
  constructor(private authService: AuthService) {}

  @Mutation(() => User, { name: 'login' })
  async loginUser(
    @Args('loginUserInput') loginUserInput: LoginUserInput,
    @Context('req') req
  ) {
    const user = await this.authService.validateUser(loginUserInput);
    if (!user) {
      // @todo: report error 
    }
    req.login(user);
    return user;
  }
}

And use AuthGuard only for those actions that I want to protect from unauthenticated access.

I tried this but req.login() is not defined - even though I've initialized passport in the app module.

As a newcomer to nestjs all this makes me raise my eyebrows. It would be great to get some feedback from the developers about why the design decision was made that way.

EDIT: I made it work (forgot to call forRoutes('*') after consumer.apply(...) in the app module's configure()). So the question remains: Why all the fuss with guards and what-not if a simple setup like this just seems to work?

jmcdo29 commented 2 years ago

@mikehaertl honestly, I'm with you. I find that, at least for login and signup with a local username and password, passport is total overkill. I personally opt not to use it in cases that I really don't need to, and instead prefer to implement that logic myself. If you know what you're doing with a hashing library, it's a pretty straightforward integration and you can either set up a session or a JWT to be checked in subsequent requests (either by passport or by your own guard).

It still requires manual setup of passport, i.e. a call to passport.initialize() and passport.session()

This is only if you want passport to manage sessions, (which I realize is what this thread is about but still pointing that fact out). We can't outright bind them because if someone wants to use fastify (even though it doesn't play the best with passport) those middleware won't work, which is really annoying. It also makes adding your own session store easier, though there should probably be better ways around that

It breaks validation as pipes are exectued after guards. I.e. if I have a dto with an email field as username I can not validate the email address with class-validator (unless I rebuild the validation logic in yet another guard, as someone suggested on Discord)

Yeah, this is kind of unfortunate, though if you implement the login logic yourself (i.e. without passport) then this shouldn't be a problem because the AuthGuard() won't be executed. Also, if the email in invalid the guard should return a 401/403 depending on how you have passport set up

The whole setup has so many moving parts and is extremely hard to follow. Which would not be a problem per se if the actual task wasn't so easy without nest.

This is why I wrote the article a few comments above that should help demystify what is happening between Nest and passport

Given all this I wonder why we can't use a simple controller action or - as I'm using GraphQL - a mutation as usual. Something like this:

You can, and in fact I suggest you do. passport-local is incredibly cumbersome for what it does and causes so many headaches for new devs.

As a newcomer to nestjs all this makes me raise my eyebrows. It would be great to get some feedback from the developers about why the design decision was made that way.

Part of the reason this all feels so wonky is because passport is old (it was originally designed for connect, express's predecessor) but it is also tried and true. It's documentation is also iffy at best, which is another pain point for a lot of people.

If I can find the time I'd love to rip it out and make a @nestjs/auth which can handle local, jwt, cookie, and session based authentication, and maybe help with oauth, so that passport isn't necessary anymore and we can just have something that is transport agnostic as that's really what Nest is striving to be. Unfortunately, designing something like that is going to take a lot of time and testing, which in turn means that it's going to really need a lot of work and dedication that I can't always give to it.

mikehaertl commented 2 years ago

@jmcdo29 Thanks for your detailed answer and the confirmation that I'm on the right path.

This is why I wrote the article a few comments above

Without that I would have been completely lost, even if it only helped me to find a simpler solution in the end. The official nestjs docs are really well organized and look great at first glance. But to me it feels like they often only explain about 60% and leave you with many question marks.

OT: It would also be awesome to see more articles about the internal architecture of the framework. Like for example how are pipes, guards or interceptors implemented or how does the DI mechanism work internally. Or in this case here how exactly does the passport module "know" about your LocalStrategy when you never import it but only add it as a provider. I tried to study the source code a little but it would be much easier with some pointers to the important parts, like you did in your article.

kamilmysliwiec commented 2 years ago

I think what would improve the Authentication chapter significantly is:

mikehaertl commented 2 years ago

focus on JWT authentication more?

Call me oldschool but I still think that sessions have a valid place in authentication. So it would be great to also include them in the example. It would also showcase how to connect different parts of the framework.

gastonmorixe commented 1 year ago

I love NestJS but I can't believe how hard it's to have a simple session auth setup correctly in 2022. I agree with @mikehaertl. Sessions should be the most basic auth way to implement. Even if JWT is more popular lately, in Rails for example you can get session auth running in a few minutes.

kamilmysliwiec commented 1 year ago

I'll make sure to update this chapter as soon as I can. It's indeed way too complicated right now

dragonfleas commented 5 months ago

I'll make sure to update this chapter as soon as I can. It's indeed way too complicated right now

Hey it's been 2 years and these docs are absolutely to the point of being unusable. Trying to introduce NestJS to new collaborators on projects is a very painful experience with the current state of the session documentation. Could we get a priority on this?