CODEWITHSUNDEEP
javascripttypescriptrxjsobservable
Rob Bailey
Rob Bailey

Reputation: 981

Is there a better RXJs Operator to use in this scenario?

I have a service method which does the following:

  1. Finds the user from the Database by their ID
  2. Checks if a user has been found
  3. Uses bcrypt to compare the password stored in the DB against a password provided as a parameter
  4. Throws an UnauthorizedException if the password was incorrect, returns the user if correct.

I'm just trying to find out if there's a better way of using RxJS operators to do this because I don't like having to pipe from the bcrypt.compare:

public validateUser(email: string, pass: string): Promise<UserDto> {
    return this.userService
      .findOne({ email })
      .pipe(
        map((user: UserDto) => {
          if (!user || !user.password) {
            return throwError(new UnauthorizedException());
          }
          return user;
        }),
        switchMap((user: UserDto) => {
          return from(
            bcrypt.compare(pass, user.password) as Promise<boolean>
          ).pipe(
            map((passwordIsCorrect) => ({
              passwordIsCorrect,
              user
            }))
          );
        }),
        switchMap((res) => {
          if (!res.passwordIsCorrect) {
            return throwError(new UnauthorizedException());
          }
          return of(res.user);
        })
      )
      .toPromise();
  }

Upvotes: 2

Views: 141

Answers (2)

BizzyBob
BizzyBob

Reputation: 14750

I don't think there's a better operator to use, but you could simplify the code to be all within the same switchMap like this:

  public validateUser(email: string, pass: string): Promise<UserDto> {
    return this.userService.findOne({ email }).pipe(
      switchMap(user => {
        if (!user?.password) {
          return throwError(new UnauthorizedException());
        }

        return from(bcrypt.compare(pass, user.password)).pipe(
          switchMap(passwordIsCorrect => passwordIsCorrect ? of(user) : throwError(new UnauthorizedException()))
        )
      })
    ).toPromise();
  }

However, in this case, it seems like you're fighting to use observables, (convert promise to observable, just to convert back to promise).

Even if the userSerivce returns observable, why not just convert it to a promise straight away? It seems like the code would be much simpler:

  public async validateUser(email: string, pass: string): Promise<UserDto> {
    const user = await this.userService.findOne({ email }).toPromise();
    
    if (!user?.password || !await bcrypt.compare(pass, user.password)) {
      throw new UnauthorizedException();
    }
    
    return user;
  }

Upvotes: 2

Steve Holgado
Steve Holgado

Reputation: 12089

If you really don't like the inner pipe, you could use combineLatest to pass along the value of user as well passwordIsCorrect:

public validateUser(email: string, pass: string): Promise<UserDto> {
    return this.userService
      .findOne({ email })
      .pipe(
        map((user: UserDto) => {
          if (!user || !user.password) {
            return throwError(new UnauthorizedException());
          }
          return user;
        }),
        switchMap((user: UserDto) => {
          return combineLatest([
            from(bcrypt.compare(pass, user.password)),
            of(user)
          ])
        }),
        map(([passwordIsCorrect, user]) => ({
          passwordIsCorrect,
          user
        })),
        switchMap((res) => {
          if (!res.passwordIsCorrect) {
            return throwError(new UnauthorizedException());
          }
          return of(res.user);
        })
      )
      .toPromise();
  }

Upvotes: 0

Related Questions