Can't redirect/render view if findOrCreateUser fails #206

Closed
jcayzac opened this Issue Mar 8, 2012 · 4 comments

Projects

None yet

2 participants

@jcayzac
jcayzac commented Mar 8, 2012

I authenticate with Twitter and I use a custom findOrCreateUser to check if the twitter account is already associated with an account of another type. I don't have access to res so I can't redirect. Also, findOrCreateUser can't break to anything. Returning a failure just crashes the app.

@bnoguchi
Owner
bnoguchi commented Mar 8, 2012

You don't need to throw an error at that step. If you just return a Promise promising your user, then eventually it will get passed to the final sendResponse step, which has a default implementation per below that redirects the user to your redirectPath configured param:

everyauth.twitter
  .findOrCreateUser( function (session, accessToken, accessTokenSecret, oauthUser) {
    var promise = this.Promise();
    db.findOrCreateUserByOauthUserParas( oauthUser, accessToken, accessTokenSecret, function (err, user) {
      if (err) return promise.fail(err);
      promise.fulfill(user);
    });
    return promise;
  })
  .sendResponse( function (res) { /* default implementation */
    var redirectTo = this.redirectPath();
    if (!redirectTo)
      throw new Error('You must configure a redirectPath');
    this.redirect(res, redirectTo);    
  });

If you want, you can overwrite the default implementation of sendResponse to do a conditional redirect based on if the user was found or not. Every step function has a hidden final argument that encapsulates all promised data to date. So, for example:

everyauth.twitter
  .findOrCreateUser( function (session, accessToken, accessTokenSecret, oauthUser) {
    var promise = this.Promise();
    db.findOrCreateUserByOauthUserParas( oauthUser, accessToken, accessTokenSecret, function (err, user) {
      if (err) return promise.fail(err);
      promise.fulfill(user);
    });
    return promise;
  })
  .sendResponse( function (res, data) {
    var user = data.user; /* called "user" because that's what the step `findOrCreateUser` promises in modules/oauth.js */
    if (! user.justCreated) {
      return this.redirect(res, 'some/custom/path');
    }

    // Otherwise, run default implementation
    var redirectTo = this.redirectPath();
    if (!redirectTo)
      throw new Error('You must configure a redirectPath');
    this.redirect(res, redirectTo);    
  });
@bnoguchi bnoguchi closed this Mar 8, 2012
@jcayzac
jcayzac commented Mar 9, 2012

In my case I don't have any user to return: he doesn't exist yet, and I have insufficient data to create it at this stage.
What I ended up doing:

getSession: function(req) {
    // Hack to access "res" from findOrCreateUser!
    req.session.req = req
    return req.session
},
findOrCreateUser: function (session, accessToken, accessTokenSecret, oauthUser) {
    // non-important stuff removed
    User.findById(session.auth.userId, function (err, user) {
        // If user is already authenticated, just update the details
        if (user) return update(user)

        User.where('tiedTwitterAccount.id', oauthUser.id).findOne(function(err, user) {
            // Twitter login
            if (user) return update(user)

            // Not found, redirect to register page BUT
            // keep the twitter auth in the session meanwhile
            session.pendingTwitter = {
                id: oauthUser.id,
                accessToken: accessToken,
                accessTokenSecret: accessTokenSecret
            }

            // Grab the response object from the session
            // (see hack note in getSession())
            var res = session.req.res
            delete session.req

            // This piece of crap cancels the promise
            try { promise.breakTo() }
            catch(e) { }

            res.redirect('/register')
        })
    })
    return promise
}
@bnoguchi
Owner
bnoguchi commented Mar 9, 2012

You can access res from the hidden final argument I mentioned.

findOrCreateUser: function (session, accessToken, accessTokenSecret, oauthUser, data) {
  var res = data.res;
  // ...
});

That said, my strong recommendation is to overwrite sendResponse and to fulfill the promise as a "null user" if you want to hold off on creating a user until after a separate registration. The code would become:

findOrCreateUser: function (session, accessToken, accessTokenSecret, oauthUser) {
    // non-important stuff removed
    User.findById(session.auth.userId, function (err, user) {
        // If user is already authenticated, just update the details
        if (user) {
          promise.fulfill(user); // bnoguchi: Make sure to also fulfill the promise
          return update(user);
        }

        User.where('tiedTwitterAccount.id', oauthUser.id).findOne(function(err, user) {
            // Twitter login
            if (user) {
              promise.fulfill(user); // bnoguchi: Make sure to also fulfill the promise
              return update(user);
            }

            // Not found, redirect to register page BUT
            // keep the twitter auth in the session meanwhile
            session.pendingTwitter = {
                id: oauthUser.id,
                accessToken: accessToken,
                accessTokenSecret: accessTokenSecret
            };
            promise.fulfill(null); // bnoguchi: a `null` user
        })
    })
    return promise;
},

sendResponse: function (res, data) {
    if (data.session.pendingTwitter) {
        return res.redirect('/register');
    }
    return res.redirect(this.redirectPath());
}
@jcayzac
jcayzac commented Mar 9, 2012

Thanks, I changed it to your version.
Returning a null user didn't work, tho, as I use everyauth through mongoose-auth and the latter tries to dereference properties from the user object. I simply swapped null for {}.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment