meteor icon indicating copy to clipboard operation
meteor copied to clipboard

Get default options from meteor settings

Open gonzadelarge opened this issue 1 year ago • 4 comments

Add support for custom configuration in AccountsClient initialization

Description Currently, the accounts-base package begins before your own project does it. If you want to use a custom configuration, you must wait for the package to start and then pass the configuration you need. This can cause problems when, for example, you want to use a different DDP connection, such as login attempts to the wrong server on the first login attempt.

At this time, the accounts-base package is ready to receive different configuration parameters, however, you will have to wait for the package to start, and then, when your application starts, you can change the configuration.

To solve this problem, I modified the accounts-base client_main.js file to allow custom configuration when initializing AccountsClient. In my version, the AccountsClient constructor accepts the path to the meteor.settings file. If the Meteor.settings.public.packages.accounts field exists, the accounts will use the settings defined there. Otherwise, it will initialize with the default settings as it currently works.

Proposed Changes Modification of the client_main.js file, line 10, to accept the path to the Meteor settings file and utilize it in the initialization of AccountsClient.

Related Issue This change addresses the problem reported in issue #12870: Link to the issue

Additional Notes This change has minimal impact on the existing accounts-base codebase and resolves a significant issue for users needing custom configurations.

** Example of use** settings_example

gonzadelarge avatar Feb 12 '24 10:02 gonzadelarge

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Feb 12 '24 10:02 CLAassistant

Not bad, but I think we also need the same approach for server (if we don't have it already). Plus documentation addition is needed.

I have added the same option on the server side. Now you can pass parameters to AccountsServer from meteor.settings. I have also updated the documentation.

On the other hand, I noticed that AccountsCommon "accounts_common.js" has a comment (line 211) with the option to initialize "accounts-base" with meteor.settings, but it is just that, a comment. There is no line of code in the file that uses meteor.settings to initialize "accounts-base".

gonzadelarge avatar Feb 12 '24 13:02 gonzadelarge

So far looking good. I just have one question that we need to test: does client setting propagate to server (possibly via commons)? If not then we need to make it happen and in this case server settings should override the client settings if there is conflict ({ ...clientSettings, ...serverSettings }).

StorytellerCZ avatar Feb 14 '24 08:02 StorytellerCZ

So far looking good. I just have one question that we need to test: does client setting propagate to server (possibly via commons)? If not then we need to make it happen and in this case server settings should override the client settings if there is conflict ({ ...clientSettings, ...serverSettings }).

After reviewing the code I found that the server side only expects one param ("loginExpirationInDays") from the list in the "accounts" configuration, but this "loginExpirationInDays" param can only be used by the server side. It does not matter if there are duplicate parameters in the configuration. Also, the server does not use the options directly, it needs to call the config() method implicitly to change the loginExpirationInDays parameter.

gonzadelarge avatar Feb 15 '24 15:02 gonzadelarge

For the server documenting that only loginExpirationInDays is expected I think is enough. We could also make it easier that this setting could be on the client and for server instantiation it would be { ...clientSettings, ...serverSettings }, so that you can write the settings in one place.

StorytellerCZ avatar Mar 03 '24 14:03 StorytellerCZ

I like it, I think this is nice to have. 🚀

nachocodoner avatar Mar 12 '24 19:03 nachocodoner

@gonzadelarge can you please update with the latest changes from 2.16. Thank you!

StorytellerCZ avatar Mar 25 '24 15:03 StorytellerCZ