signIn callback returning before promise resolves

[wwsalmon]

Your question
I use the signIn callback to access MongoDB before letting the user through. My code looks like this:

signIn: async (user, account, profile) => {
    try {
        mongoose.connect(process.env.MONGODB_URL, {
            useNewUrlParser: true,
            useUnifiedTopology: true,
            useFindAndModify: false,
        });

        userModel.findOne({ email: user.email }, (err, foundItem) => {
            if (err) return Promise.reject(new Error(err));

            // if user object already exists, return
            if (foundItem) return Promise.resolve(true);

            const urlName = user.name.split(" ").join("-") + "-" + short.generate();

            // otherwise, create new user object
            userModel.create({
                email: user.email,
                name: user.name,
                image: user.image,
                urlName: urlName,
                private: false,
            }, (err, newUser) => {
                if (err) return Promise.reject(new Error(err));
                return Promise.resolve(newUser);
            });
        });
    } catch (e) {
        console.log(e);
        return Promise.reject(new Error(e));
    }
}

However, the callback returns true and redirects to the homepage before the promise is even returned. As a minimal reproduction:

signIn: async (user, account, profile) => {
     setTimeOut(() => {
		return Promise.resolve(false);
	}, 2000);
}

does not stop the user, but rather lets them through. Wrapping everything in a promise doesn't work either:

signIn: async (user, account, profile) => {
	return Promise((res, rej) => {
		setTimeOut(() => {
			res(false);
		}
	});
}

This request just returns a pending promise and never resolves.

How do I make the callback return, and the page redirect, only after the function has fun and the promise has resolved?

Here's my whole project repo for further context: https://github.com/wwsalmon/updately

Feedback
Documentation refers to searching through online documentation, code comments and issue history. The example project refers to next-auth-example.

• Found the documentation helpful
• Found documentation but was incomplete
• Could not find relevant documentation
• Found the example project helpful
• Did not find the example project helpful

[balazsorban44]

Hi there! Since you are using Promises inside userModel.findOne, I assume it is also an async method. I think you should actually return userModel.findOne in your try block, or if I may suggest, you could try using async/await for better readability like so:

signIn: async (user, account, profile) => {
    await mongoose.connect(process.env.MONGODB_URL, {
        useNewUrlParser: true,
        useUnifiedTopology: true,
        useFindAndModify: false,
    })
    const userExists = Boolean(await userModel.findOne({ email: user.email }).exec())
    if (!userExists) {
      const urlName = user.name.split(" ").join("-") + "-" + short.generate();
      await userModel.create({
        email: user.email,
        name: user.name,
        image: user.image,
        urlName: urlName,
        private: false,
      })
    }
    return true
}

Some explanation. I am not entirely sure about the mongoose API, so I made the assumptions from the docs here and here that those operations return a promise, and you can use that instead of callbacks.
This fairly simplifies the code, as you can see. Also, you don't have to try to catch errors, in case you just want the sign-in to fail in case of an error. When async/await code fails, it will throw an error, just like a rejected Promise. next-auth handles it by not showing the user signed in. If you do wish to add some extra functionality, like redirect the user to a special URL if the sign-in process failed, you can throw a string, which should be a valid URL, like so:

try {
  //do some async/await work here
  return true // don't forget to return true at the end
} catch(error) {
  if(errorIWantToHandle(error)) {
    throw "/signin-error"
  }
  throw error
}

(@iaincollins I think the API here could be improved:

next-auth/src/server/routes/signin.js

Line 62 in 5126f4e

return redirect(error)

returning a string would probably enough, instead of throwing.)

I advocate for using async/await over Promises and callbacks because you will quickly end up with a lot of nested calls, which makes it much harder to read the code and accidentally forget to add a missing return.

Also with async/await, you don't need the if (err) return Promise.reject(new Error(err)); check in every callback, because your catch block will catch all the errors collectively.

@wwsalmon, please also note that I THINK (@iaincollins have to back me up here) return Promise.resolve(newUser); will not actually return your new user to the jwt callback or any other later callbacks. signIn only expects only true as a valid return value.
Although the docs says you can:

Return true (or a modified JWT) to allow sign in
Return false to deny access
https://next-auth.js.org/configuration/callbacks#sign-in-callback

The source code suggests that signinCallbackResponse (the thing returned from signIn) will be lost, cause the sign-in process will always end with a redirect.:
https://github.com/nextauthjs/next-auth/blob/main/src/server/routes/signin.js

The user marked the docs as incomplete, which I have to agree with because of the above mentioned mismatch.

[wwsalmon]

@balazsorban44 changing everything to async/await instead of callbacks and returning booleans instead of promises fixed my problem, thanks so much!

re: documentation, I'm a fairly new frontend dev who hasn't worked all that much with async/await and promise patterns before, so maybe a more experienced backend dev wouldn't have run into this problem. For me, though, seeing return true in the documentation definitely would have been clearer!