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

passportConfigurator userModel not responding to change #95

Open
ianseyer opened this issue Oct 22, 2015 · 34 comments
Open

passportConfigurator userModel not responding to change #95

ianseyer opened this issue Oct 22, 2015 · 34 comments

Comments

@ianseyer
Copy link

ianseyer commented Oct 22, 2015

Hi there!

I am using the facebook passport component, to handle user signup via facebook.

It is fully functional, however:

despite my server.js containing:

passportConfigurator.setupModels({
 userModel: app.models.donor,
 userIdentityModel: app.models.userIdentity,
 userCredentialModel: app.models.userCredential
});

the users are still created in the standard User table, instead of my Donor table.

@superkhau
Copy link
Contributor

Duplicate. See strongloop/loopback#1750

@superkhau superkhau reopened this Oct 29, 2015
@superkhau
Copy link
Contributor

Keeping this issue as the main and closing strongloop/loopback#1750 as a duplicate.

@clockworkgr
Copy link

Following the discussion you had at strongloop/loopback#1750 ,

I can confirm that there is an issue with user models not named User or user due to hardcoded relations in userIdentity and userCredentials.

This falls under supporting extended user models. I have already submitted a PR fixing this here: #93

@jonathan-casarrubias
Copy link

For those -like me- who simply can not wait until this module is merged and released, I took @clockworkgr fix, I did just small tweaks and published under loopback-component-passport-c

So thanks to @clockworkgr fix, now we can assign a specific user model. its working for me now..

$ npm uninstall --save loopback-component-passport
$ npm install --save loopbac-component-passport-c

Following the standard documentation from loopback should work -now it does-, the following method does what everyone would expect it to do.

passportConfigurator.setupModels({
 userModel: app.models.Account, // Now my users are stored in Account and accessToken is now related with this account.
 userIdentityModel: app.models.AccountIdentity,
 userCredentialModel: app.models.AccountCredential
});

Please consider that passing an app.models.user as is recommended in some loopback documentation would partially work, it does not only breaks conventions rules it only works when you run inside node, when you try to access through REST interface, it will try to search for users in "User" collection and not in "user" collection, thus breaking the system... storing in lowercased name collection, and searching in uppercase name collection.. therefore this is a big concern and requires immediate solution.

For that reason I encourage anyone willing to fix this issue in your project ASAP by using loopback-component-passport-c until the moment this is fixed in the core module.

Cheers
@jonathan-casarrubias

@mbouclas
Copy link

Concerning #106 which was closed as duplicate. Anyone has any idea (or example) on how to auth (FB or Google) using a non standard model? By that i mean, a model that has extra required fields to the standard ones. For example, my model has a isActive field, and auth fails every time because of validation error, as the isActive does not exist in the FB response. In my case what i get after every auth attempt is this (or something in this lines)

422 ValidationError: The `Owner` instance is not valid. Details: `firstName` can't be blank (value: undefined); `lastName` can't be blank (value: undefined); `isActive` can't be blank (value: undefined).

@jonathan-casarrubias
Copy link

I think you simply should use an operation hook, Im doing something similar of what you try to achieve, I add a permalink depending the username facebook/twitter sent

    Account.observe('before save', function(ctx, next) {
        if (!ctx.isNewInstance) { next(); }
        // I create a permalink and a relation model on before save hook
    });

    Account.observe('after save', function(ctx, next) {
        if (!ctx.isNewInstance) { next(); }
        // I send a welcome email message here
    });

The documentation states that it should run before validation

https://docs.strongloop.com/display/public/LB/Operation+hooks

Cheers,
@jonathan-casarrubias

@mbouclas
Copy link

@jonathan-casarrubias That brings me closer to what i'm after. Any idea if the ctx object holds the strategy data (FB return data)? I found an "instance" property but it does not hold the FB data. For example, i could use the first and last name returned by FB.

I can see in the user-identity.js the profileToUser receives the profile, can i override it somehow ?

Edit: I realized that by adding my own login method i override the default. Is this considered to be a good practice?

thanks

@jonathan-casarrubias
Copy link

I don't think you should override anything, actually you should be able to get the data you are looking by calling for it inside the hook, should be something like..

 Account.observe('before save', function(ctx, next) {
        if (!ctx.isNewInstance) { next(); }
        var identities = ctx.instance.identities();
    });

I did not test the snippet but should be good enough to give you the idea.

Cheers,
@jonathan-casarrubias

@mbouclas
Copy link

ctx.instance is an object holding the following

{ username: 'facebook-login.10153578469710033',
  password: '$2a$10$vBh8ns3ShTwmPbW870qA0On6V5/u0pr943zTrCOC/NxgjODB48zOq',
  email: 'test@test.com',
  updated_at: Tue Nov 17 2015 23:13:57 GMT+0200 (GTB Standard Time),
  created_at: Tue Nov 17 2015 23:13:57 GMT+0200 (GTB Standard Time) }

which is consistent with what the profileToUser function returns. With that said, looking into user-identity.js i noticed that you can have your own profileToUser method by passing it in the strategy options. This will do just fine and i think that code wise it is somewhat "cleaner" to having a "before save" hook.

thoughts?

P.S. I believe that this including loginCallback and customCallback deserve to be mentioned in the docs cause they give you a great deal of options as far as customization goes.

@jonathan-casarrubias
Copy link

The way I configured, I extended, User, UserIdentities and UserCredentials into Account, AccountIdentities and AccountCredentials, I explicitly added the relation as stated in the documentation, and I do have access to Account.identities() or ctx.instance.identitites() that includes the identities for that account so you may manipulate that data as your convenience.

In order for this to work, I needed to use the module I published that includes @clockworkgr's fix, which is the original topic of this thread, otherwise won't work, see above for more details.

Cheers,
@jonathan-casarrubias

@superkhau superkhau removed the triage label Nov 29, 2015
@superkhau superkhau assigned loay and unassigned superkhau Nov 30, 2015
@superkhau
Copy link
Contributor

@loay Reassigning to you as you are the triage expert.

@loay
Copy link
Contributor

loay commented Dec 1, 2015

Thanks. Having a look.

@ebarault
Copy link

ebarault commented Mar 6, 2016

@jonathan-casarrubias
when you write :

" I explicitly added the relation as stated in the documentation, and I do have access to Account.identities() or ctx.instance.identitites() that includes the identities for that account so you may manipulate that data as your convenience."

could you specify what relation exactly you added?
rigth now i cannot get identities in the operation hook before the custom user is created. empy array is always returned when invoking the relation.

thanks,

@btassone
Copy link

Hey guys any progress on this? I need to extend the user model in one of my projects and I wanted to use loopback 3.x. Sadly the fix up above is only slated for 2.x (the temporary fork is what I am talking about). Is there any timetable for when this will be fixed?

@jonathan-casarrubias
Copy link

I'm starting a project with loopback 3 and will need to use the component passport, I'll see how it goes and update here

@btassone
Copy link

btassone commented Sep 1, 2016

@jonathan-casarrubias Any luck?

@jonathan-casarrubias
Copy link

@btassone we did start the project in LoopBack 3 and were able to use the fork version I published without problems.

Anyway, I had to take the decision of rolling back to version 2, not because of issues with this module, but because the version 3 alpha present many issues that the version 2 does not present, many of these were issues with HasMany relationships.

So, again we were able to use it in version 3, but we rolled back because other issues.

Hope this info is helpful for you

@mamartins
Copy link

1 year after and we're still here :p

@wprater
Copy link

wprater commented Feb 16, 2017

any update on this one? or is there a work around to allow a custom User model?

@nickvanvynckt
Copy link

@wprater, use the "loopback-component-passport-c" package someone mentioned here before. This allows you to use custom User models!

@lowell-list
Copy link

lowell-list commented Jun 13, 2018

My custom User model (named PortalUser) is now being auto-created by loopback-component-passport with these steps:

  1. Create custom user, user identity, and user credential models that extend from the provided User, UserIdentity, and UserCredential models. In my case I named them PortalUser, PortalUserIdentity, and PortalUserCredential.

  2. Set the "belongsTo" relations of PortalUserIdentity and PortalUserCredential to point to PortalUser instead of User:

  ...
  // code in portal-user-credential.json and portal-user-identity.json
  "relations": {
    "user": {
      "type": "belongsTo",
      "model": "PortalUser",
      "foreignKey": "userId"
    }
  }
  ...
  1. pass all three custom models through to passportConfigurator.setupModels():
  // code in server.js
  passportConfigurator.setupModels({
    userModel: app.models.PortalUser,
    userIdentityModel: app.models.PortalUserIdentity,
    userCredentialModel: app.models.PortalUserCredential
  });

@jonathan-casarrubias
Copy link

lol almost 3 years and same problems? that is why I decided to stop using loopback.. good luck guys

@SergeNarhi
Copy link

SergeNarhi commented Aug 31, 2018

@jonathan-casarrubias what are you using instead of it?

It seems the error was caused by a lack of documentation. @lowell-list solution works fine with source package.

@rajkaran
Copy link

rajkaran commented Sep 7, 2018

@lowell-list
This doesn't work for me
"foreignKey": "portalUserId"

have to change it to "foreignKey": "userId" because of below code snippet in user-identity.js

userIdentityModel.findOrCreate(
	{where: {externalId: profile.id}},
	{
	  provider: provider,
	  externalId: profile.id,
	  authScheme: authScheme,
	  profile: profile,
	  credentials: credentials,
	  userId: user.id,
	  created: date,
	  modified: date,
	},
	function(err, identity) {
	  if (!err && user && autoLogin) {
		return (options.createAccessToken || createAccessToken)(
		  user,
		  function(err, token) {
			cb(err, user, identity, token);
		  }
		);
	  }
	  cb(err, user, identity);
	}
);

@lowell-list
Copy link

@rajkaran - you are correct; I reviewed my code and discovered that I later changed "foreignKey" to "userId" in both portal-user-credential.json and portal-user-identity.json because I encountered the same issue you did. I updated my previous post with "userId" to help the next person.

@dragonxsx
Copy link

dragonxsx commented Feb 5, 2020

My custom User model (named PortalUser) is now being auto-created by loopback-component-passport with these steps:

1. Create custom user, user identity, and user credential models that extend from the provided User, UserIdentity, and UserCredential models. In my case I named them PortalUser, PortalUserIdentity, and PortalUserCredential.

2. Set the "belongsTo" relations of PortalUserIdentity and PortalUserCredential to point to PortalUser instead of User:
  ...
  // code in portal-user-credential.json and portal-user-identity.json
  "relations": {
    "user": {
      "type": "belongsTo",
      "model": "PortalUser",
      "foreignKey": "userId"
    }
  }
  ...
1. pass all three custom models through to `passportConfigurator.setupModels()`:
  // code in server.js
  passportConfigurator.setupModels({
    userModel: app.models.PortalUser,
    userIdentityModel: app.models.PortalUserIdentity,
    userCredentialModel: app.models.PortalUserCredential
  });

@lowell-list:
By refer to your solution, I overrided the 'user' relations of UserIdentity and UserCredential to make my CustomUser works.

  const MyCustomUser = app.models.MyCustomUser;
  const UserIdentity = app.models.UserIdentity;
  const UserCredential = app.models.UserCredential;

  // Fix problem related to custom userModel
  UserIdentity.belongsTo(MyCustomUser, {as: 'user', foreignKey: 'userId'});
  UserCredential.belongsTo(MyCustomUser, {as: 'user', foreignKey: 'userId'});

  const passportConfigurator = new PassportConfigurator(app);
  passportConfigurator.init();
  passportConfigurator.setupModels({
    userModel: MyCustomUser
  });

Another solution is removing the built-in User model setting in model-config.json.
It will make these line work well with custom User model:

https://github.com/strongloop/loopback-component-passport/blob/a837974d4b505d7ee5e3232dda07d2794d1b5e70/lib/passport-configurator.js#L61

  if (!this.userIdentityModel.relations.user) {
    this.userIdentityModel.belongsTo(this.userModel, {as: 'user'});
  }

  if (!this.userCredentialModel.relations.user) {
    this.userCredentialModel.belongsTo(this.userModel, {as: 'user'});
  }

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

No branches or pull requests