meteor / meteor-feature-requests

A tracker for Meteor issues that are requests for new functionality, not bugs.
Other
89 stars 3 forks source link

[Accounts] set custom collection #20

Open dr-dimitru opened 7 years ago

dr-dimitru commented 7 years ago

Migrated from: meteor/meteor#8757

Add custom option to Accounts package:

//cc @ashburnham @zimme

zimme commented 7 years ago

This solution sounds good to me

hwillson commented 7 years ago

This is related to https://github.com/meteor/meteor/issues/4995 as well.

zimme commented 7 years ago

I feel we have a good enough game plan for this to encourage a PR.

klaussner commented 7 years ago

Should the collection for the ServiceConfiguration, meteor_accounts_loginServiceConfiguration by default, be configurable, too? Otherwise, all apps that use the database would have to make their users log in with the same Facebook app, for example.

zimme commented 7 years ago

@klaussner, nice catch. This would be good, but can we think of a use case and is this related or should it be considered a separate FR?

klaussner commented 7 years ago

If two Meteor apps are using the same external Facebook/GitHub/… app for logins, the external app should have a name that fits both apps so that users don't see an unexpected name when they are asked to approve access. But that may not always be possible.

Although it's related to this FR (each app/set of users should be able to have its own login methods), I think it's more of an edge case and can be addressed in a separate FR.

zimme commented 7 years ago

If two Meteor apps are using the same external Facebook/GitHub/… app for logins, the external app should have a name that fits both apps so that users don't see an unexpected name when they are asked to approve access. But that may not always be possible.

:+1:

Although it's related to this FR (each app/set of users should be able to have its own login methods), I think it's more of an edge case and can be addressed in a separate FR.

I would suggest we make this a separate FR as it could be created as a separate PR, but I would encourage the person taking this on to take care of both FRs.

sakulstra commented 6 years ago

Is this pr-encouraged? If yes, i'd try to implement this after: https://github.com/meteor/meteor/pull/9558 is merged

hwillson commented 6 years ago

Sure thing @sakulstra, it is now. Looking forward to reviewing a PR - thanks!

sakulstra commented 6 years ago

thanks hwillson,

I initially thought this could be as easy as adding a collection option to config(options) in AccountsCommon:

if (options.collection instanceof Mongo.Collection) {
      this.users = options.collection;
    } else {
      this.users = new Mongo.Collection(options.collection || "users", {
        _preventAutopublish: true,
        connection: this.connection
   });
}

But a thing I didn't have in mind is the default constructor. The problem I see and don't really know how to work around is that in server_main https://github.com/meteor/meteor/blob/devel/packages/accounts-base/server_main.js we create an instance of AccountsServer - so even if changing the collection name via config we'll still have a default users collection in our mongodb which is created on startup. Not creating the AccountsServer seems like a breaking change for almost all meteor applications, so I don't know how to continue on that front.

Any hints appreciated :heart:

juho commented 6 years ago

NB: here's some (old) code from our lib/users.js on how to switch the collection per-app:

import CompanyUsers from '/imports/general-imports/users/company-users/collection.js';
Accounts.users = CompanyUsers;
Meteor.users = Accounts.users;

// Deny all client-side updates to user documents
Meteor.users.deny({
  update: function() { return true; },
  remove: function() { return true; }
});

Accounts.config({
  forbidClientAccountCreation : true
});
cpury commented 6 years ago

Is there any update on this? We're building a secondary app for our ecosystem, so it needs to work on the global MongoDB, but define its own kinds of users. The Mongo collection users is already taken for a completely different kind of user from our main app. I recommended Meteor to my team and now we're blocked on something simple as this. I'm a little embarrassed... 🙁

@sakulstra did you make any progress on your PR? Maybe it would be easiest to read the user collection name from a config variable (e.g. from settings.json)?

dr-dimitru commented 6 years ago

Hello @cpury ,

To quickly solve your issue you can grab accounts-base package and place it under meteor-project/packages directory, then update line 23 here to set desired name for collection.

cpury commented 6 years ago

@dr-dimitru awesome, thanks! That worked as a temporary fix. Still, I would love to see this implemented. It's quite a strong assumption of Meteor that the users collection is available.

dr-dimitru commented 6 years ago

@cpury agree, there is huge update coming for accounts packages and should be delivered within weeks. Not sure if it will include this specific ticket, if not I'll send a PR for it, just need to wait for this update