Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeError: Cannot read property 'service' of undefined #56

Closed
wuyuanyi135 opened this Issue Feb 12, 2016 · 32 comments

Comments

Projects
None yet
6 participants
@wuyuanyi135
Copy link

wuyuanyi135 commented Feb 12, 2016

Hello,
I have made it to create user, but when logging in, I got no luck. Code at gist.

My request was

POST http://localhost:3000/auth/local
Content-Type: application/x-www-form-urlencoded

username=admin%40feathersjs.com&password=admin

It threw the exceptions:

TypeError: Cannot read property 'service' of undefined
    at Object.checkCredentials (/home/wuyuanyi/riddle-web/node_modules/feathers-authentication/lib/services/local/index.js:84:15)
    at Strategy.authenticate (/home/wuyuanyi/riddle-web/node_modules/passport-local/lib/strategy.js:90:12)
    at attempt (/home/wuyuanyi/riddle-web/node_modules/passport/lib/middleware/authenticate.js:348:16)
    at authenticate (/home/wuyuanyi/riddle-web/node_modules/passport/lib/middleware/authenticate.js:349:7)
    at /home/wuyuanyi/riddle-web/node_modules/feathers-authentication/lib/services/local/index.js:141:9
    at new Promise (/home/wuyuanyi/riddle-web/node_modules/core-js/modules/es6.promise.js:197:7)
    at Object.create (/home/wuyuanyi/riddle-web/node_modules/feathers-authentication/lib/services/local/index.js:120:14)
    at Object.wrapper (/home/wuyuanyi/riddle-web/node_modules/feathers/lib/mixins/promise.js:6:28)
    at Object.<anonymous> (/home/wuyuanyi/riddle-web/node_modules/uberproto/lib/proto.js:30:17)
    at Object.wrapped (/home/wuyuanyi/riddle-web/node_modules/rubberduck/lib/rubberduck.js:63:19)
    at /home/wuyuanyi/riddle-web/node_modules/feathers-hooks/lib/before.js:49:20
    at new Promise (/home/wuyuanyi/riddle-web/node_modules/core-js/modules/es6.promise.js:197:7)
    at /home/wuyuanyi/riddle-web/node_modules/feathers-hooks/lib/before.js:36:18
    at run (/home/wuyuanyi/riddle-web/node_modules/core-js/modules/es6.promise.js:104:47)
    at /home/wuyuanyi/riddle-web/node_modules/core-js/modules/es6.promise.js:115:28
    at flush (/home/wuyuanyi/riddle-web/node_modules/core-js/modules/$.microtask.js:19:5)

But once I made some mistake, like Content-Type: application/x-www-form-urle, the rejecting json message was returned

{
"name": "NotAuthenticated"
"message": "Invalid login."
"code": 401
"className": "not-authenticated"
"errors": {}
}

Could anyone reproduce this?

@marshallswain

This comment has been minimized.

Copy link
Member

marshallswain commented Feb 12, 2016

You've mounted your user service at /, but you haven't let the auth plugin know about it.

app.use('/', service({Model: userModel}));
@wuyuanyi135

This comment has been minimized.

Copy link
Author

wuyuanyi135 commented Feb 12, 2016

Should I set userEndpoint: '/' in configuration?
I tried but I still got the same error messages

Thank you

@marshallswain

This comment has been minimized.

Copy link
Member

marshallswain commented Feb 12, 2016

Yeah, the userEndpoint will need to match, but looking at that stack trace, that wasn't the problem that caused this. (but it would have been the next one to come up:)

That top line in the stack trace

at Object.checkCredentials (/home/wuyuanyi/riddle-web/node_modules/feathers-authentication/lib/services/local/index.js:84:15)

Is this:

this.app.service(this.options.userEndpoint).find(params).then(function (users) {

So for some reason this.app isn't set correctly in that service.

@marshallswain

This comment has been minimized.

Copy link
Member

marshallswain commented Feb 12, 2016

I'd recommend trying to use the default userEndpoint, or setting it to something other than the root /.

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Feb 12, 2016

Hmm. I'll look at that issue but ya you shouldn't be using the root '/' for a service. Services are meant to be descriptive anyway.

@wuyuanyi135

This comment has been minimized.

Copy link
Author

wuyuanyi135 commented Feb 12, 2016

Thanks. I tried to use the default /user, still got the same problem.
I use feathers.use('api/user', [this module]) to route it so I use the root here. But it seems not the problem

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Feb 12, 2016

if you are using /api/user for your user endpoint the config value you pass for userEndpoint needs to match. So it would also need to be /api/user.

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Feb 12, 2016

I'll try running your gist. See if there is something else going on.

@wuyuanyi135

This comment has been minimized.

Copy link
Author

wuyuanyi135 commented Feb 12, 2016

@ekryski Thanks a lot 👍
Yeah I tried not to use feathers.use to route (directly use app.listen) then it works. I assumed that the userEndpoint is relative to this app. I am trying if changing the endpoint will work

I think that gist should work after modifying the userEndpoint.

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Feb 12, 2016

no problem. I think that may be your issue. Report back. We recently added some docs on routing. http://docs.feathersjs.com/middleware/routing.html

@wuyuanyi135

This comment has been minimized.

Copy link
Author

wuyuanyi135 commented Feb 12, 2016

if I will post login request to http://localhost:8080/riddle/api/user/auth/local
my user end point is http://localhost:8080/riddle/api/user/

index.js : app.use('/riddle/api',api); -> api: app.use('/user',user);
the user is the app in the gist, without listen(3000) but module.exports

What should the userEndPoint be?

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Feb 12, 2016

@wuyuanyi135 ah ok you are mounting a sub-app. I haven't tried that with auth yet. I think userEndpoint should just be /user then.

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Feb 12, 2016

Another thing I noticed in your gist is that all the hooks now take an options object. So you would change this:

userService.before(
    {
        create: [authHooks.hashPassword('password')]
    }
)

to this:

userService.before(
    {
        create: [authHooks.hashPassword({ passwordField: 'password'})]
    }
)

or you could just pass nothing since password is the default expected field.

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Feb 12, 2016

@wuyuanyi135 I just pushed a working sub-app example. Hopefully that helps.

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Feb 12, 2016

I think the reason this.app was undefined is because the sub app wasn't getting mounted properly. I'm going to close this now @wuyuanyi135 but if you aren't able to get the your stuff working after looking at the example we can create a new issue.

@ekryski ekryski closed this Feb 12, 2016

@wuyuanyi135

This comment has been minimized.

Copy link
Author

wuyuanyi135 commented Feb 13, 2016

@ekryski Thanks for your example. I have figured out the reason of this issue. It was improper configuration of the sub-apps. I left out these lines in my api sub-app.

var app = feathers()
.configure(socketio())
.configure(rest())
.use(bodyParser.json())
.use(bodyParser.urlencoded({extended: true}));

Even though there is no explicit use of feathers services, the intermediate router app:

var feathers = require('feathers');
var app = feathers();

var user = require('./user');
app.use('/user',user);

won't work.

Basically, all the sub-apps have to be configured like this to relay the request to deeper level.
Could catch and throw some descriptive message for possible missing configuration steps

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Feb 14, 2016

@wuyuanyi135 ya you are right. We should probably try and toss up some better error messaging or warnings. Thanks for that feedback!

The issue is actually related this one: feathersjs/feathers#216. When we close that you shouldn't ever run into this problem.

@startupthekid

This comment has been minimized.

Copy link

startupthekid commented Mar 12, 2016

I've been having this issue for a few weeks too and it's super frustrating. My setup is relatively simple too:

const app = feathers();

app
  .configure(rest())
  .configure(hooks())
  .use(bodyParser.json())
  .use(bodyParser.urlencoded({ extended: true }))
  .configure(authentication({
    token: {
      secret: config.JWT_SECRET
    },
    local: {
      usernameField: 'email'
    },
    facebook: {
      clientID: config.FACEBOOK_APP_ID,
      clientSecret: config.FACEBOOK_APP_SECRET,
      strategy: FacebookStrategy,
      tokenStrategy: FacebookTokenStrategy,
      permissions: {
        scope: ['email', 'public_profile']
      }
    }
  }))
  .use('/users', UserService)
  .use('/posts', PostService)
  .use('/groups', GroupService);

What's weird is that if I make a call to /api/auth/local (my sub-app is mounted at /api) using x-www-form-urlencoded, I get the TypeError: Cannot read property &#39;service&#39; of undefined error but if I change that to form-data (using Postman), then the error changes to NotAuthenticated: Invalid login..

I did some more digging and in both cases, on the local setup, setup never gets called so that this.app is never set, which is why the call fails.

@wuyuanyi135

This comment has been minimized.

Copy link
Author

wuyuanyi135 commented Mar 12, 2016

@startupthekid how did you configure the parent app?

@startupthekid

This comment has been minimized.

Copy link

startupthekid commented Mar 12, 2016

Very simply:

app
    .use('/api', apiRoutes)
    .use(feathers.static(__dirname));

I believe I just found the error inside the local service but I'll need a few minutes to verify.

@startupthekid

This comment has been minimized.

Copy link

startupthekid commented Mar 12, 2016

Ok yeah I fixed it. @ekryski in the default export of the local service (and maybe others, I'm not sure):

export default function(options){
  options = Object.assign({}, defaults, options);
  debug('configuring local authentication service with options', options);

  return function() {
    const app = this;

    // Initialize our service with any options it requires
    app.use(options.localEndpoint, exposeConnectMiddleware, new Service(options), successfulLogin(options));

    // Get our initialize service to that we can bind hooks
    const localService = app.service(options.localEndpoint);

    // Register our local auth strategy and get it to use the passport callback function
    passport.use(new Strategy(options, localService.checkCredentials.bind(localService)));
  };
}

you have to add localService.setup(app). I remember reading somewhere, one of the others feathers repos, that any service created from an ES6 class (using new Service), has to have app.setup called manually. When I added that line into the default export of the local service, I was able to login.

It had nothing to do with it being a sub-app as far as I can tell, everything to do with the service being an es6 based class.

@daffl

This comment has been minimized.

Copy link
Member

daffl commented Mar 12, 2016

@startupthekid if you are using sub-apps, are you calling subapp.setup(server) as described in the documentation for sub-apps?

@daffl

This comment has been minimized.

Copy link
Member

daffl commented Mar 12, 2016

ES6 classes work the same way as normal services so that should not be an issue. But service.setup is called by app.setup which in your case probably never got called.

@startupthekid

This comment has been minimized.

Copy link

startupthekid commented Mar 12, 2016

Ah I see, the call to app.setup wasn't in the example sub app that @ekryski posted, that's where my confusion was coming from.

@daffl

This comment has been minimized.

Copy link
Member

daffl commented Mar 12, 2016

It does call setup automatically if you use Socket.io but not without. There is no real reason for this except that I was being stupid ;)

feathersjs/feathers#232 and feathersjs/feathers#216 are probably related to this.

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Mar 12, 2016

ok then we should probably call setup when rest is configured as well. I've created an issue for this: feathersjs/feathers#259

@thosakwe

This comment has been minimized.

Copy link

thosakwe commented Mar 17, 2016

Believe it or not, I'm having this problem too. And I don't have my API mounted as a subapp. I have no idea how to fix this, haha.

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Mar 17, 2016

@thosakwe would you be able to post a link to your project somewhere? I'm wondering if it is related to feathersjs/feathers#259. Are you creating your own service? Are you missing app.listen()?

@thosakwe

This comment has been minimized.

Copy link

thosakwe commented Mar 18, 2016

@ekryski I believe it is related to #259.

Here's the app.js:
https://gist.github.com/thosakwe/280e808bcc2175b273f1

I'm at a loss, though. I tried using app.listen in my bin/www (this was originally an Express app) file, but now the error is "data and hash arguments required." So something I did breaks bcrypt somewhere.

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Mar 18, 2016

@thosakwe I need more than that to go on. The initial app setup looks just fine but I need to see how you set up your user service. If you don't have a user service set up, auth will not work. Additionally you need to make sure you are populating the user. Take a look at https://github.com/feathersjs/feathers-chat to see how those hooks are set up on the user service.

@daffl

This comment has been minimized.

Copy link
Member

daffl commented Mar 18, 2016

@thosakwe Would you mind opening a new issue with the code for your app? That way it'll be much easier for us to keep track of instead of looking through the comments of a closed issue :) Thank you!

@thosakwe

This comment has been minimized.

Copy link

thosakwe commented Mar 18, 2016

Okay, cool. New issue: #108

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.