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

Prevent entryCreateUser from being used when forbidClientAccountCreation is enabled #269

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ameagher
Copy link
Contributor

Even when forbidClientAccountCreation was enabled users could still be added from the client if you manually went to the /sign-up route, or by calling entryCreateUser from the console:

Meteor.call(
    'entryCreateUser',
    {email:'[email protected]',password:'asdfasdf1'},
    function(e){console.log(e);
});

This commit disables the entryCreateUser method when forbidClientAccountCreation is enabled.

else
Router.go AccountsEntry.settings.dashboardRoute
else
console.log err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you logging the error on the client side?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the code? It's not my doing. That's from the master branch: signUp.coffee#L160

I just added a conditional at the top and added an indent to all these lines, that's why they're modified (If there's a way to prevent that I'd like to know!)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Github is doing a poor job of showing the diff. It is matching original line 152 with new line 143, when it should be 153 (I only added one line)

@queso
Copy link
Contributor

queso commented Sep 20, 2014

Thanks for the PR. Can we move the if checks inside the code instead of wrapping up global method setting in a big if statement? e.g. Please make sure the methods still exist, just return out of them and do nothing if forbidClientAccountCreation is set to true.

@ameagher
Copy link
Contributor Author

How's this?

I believe the test is failing because the package itself is failing?

@queso
Copy link
Contributor

queso commented Sep 21, 2014

I'm not concerned about the failing test, just stating my preference in how the code should be structure.

Looking at it again, perhaps this is the best way, I will take a look at it some more.

@ameagher
Copy link
Contributor Author

My most recent changes approached this a different way, and the code does look cleaner.

My original thought was to prevent the functions from being available entirely, but as long as random visitors can't create accounts I'm happy.

@lukeclifton
Copy link

Hey guy's. Any update on this PR? Functionality to prevent sign up's seems pretty vital.

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

Successfully merging this pull request may close these issues.

4 participants