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

Adds ability to disable anonymous users #440

Merged
merged 1 commit into from
Feb 16, 2016
Merged

Adds ability to disable anonymous users #440

merged 1 commit into from
Feb 16, 2016

Conversation

flovilmart
Copy link
Contributor

Proposed fix for #327

  • Adds enableAnonymousUsers to the config.
  • Marks the service unsupported when enableAnonymousUsers !== true.
  • Adds according tests.

@nitrag
Copy link

nitrag commented Feb 16, 2016

This is great! I didn't have the exact knowledge where to put the checks. My only comment is that this could break a lot of current installations and it might be best if it was "disableAnonymousUsers" so existing server installations wouldn't all of a sudden stop working for people.

@flovilmart
Copy link
Contributor Author

@nitrag it defaults to true to prevent any breaking. As for why defaulting to true, it's because undefined, null and false work well with  || true; operator. and != is not the same as !== so it's safer that way.

@nitrag
Copy link

nitrag commented Feb 16, 2016

Ah, me dumb. Thanks for this!

On Mon, Feb 15, 2016 at 11:15 PM, Florent Vilmart notifications@github.com
wrote:

@nitrag https://github.com/nitrag it defaults to true to prevent any
breaking.


Reply to this email directly or view it on GitHub
#440 (comment)
.

@drew-gross
Copy link
Contributor

Thanks! Looks awesome.

drew-gross added a commit that referenced this pull request Feb 16, 2016
@drew-gross drew-gross merged commit 6808304 into parse-community:master Feb 16, 2016
@nitrag
Copy link

nitrag commented Mar 2, 2016

@flovilmart

I just experienced an issue where the session token was bad (pre-revocable) and my app was not responding to a cloud function. Turns out request.user was null. Does your PR request include every single API hit to the database? Or just creating/update? Shouldn't we be throwing an error (Invalid login) or something. I need to have my users be forced back to the login screen.

I'm too amateur to tell if Parse is validating the user's session or not each time he accesses the app. Sigh...

@flovilmart
Copy link
Contributor Author

That seems unrelated to that PR as this only affects the ability to disable anonymous users. Can you open an issue with the proper description of your bug?

@nitrag
Copy link

nitrag commented Mar 3, 2016

Okay so there is an issue related to anonymous access the request.user=null was a separate issue.

I don't believe every request that comes into the server is being session validated. For instance. I am able to make a curl request with a purposely made up session token and it happily returns the data.

I'll email you the curl request due to privacy concerns.

@flovilmart
Copy link
Contributor Author

Can you please open an issue then as it's unrelated with the purpose of that PR.

thanks!

@rafapetter
Copy link

@flovilmart I'm setting enableAnonymousUsers to true but ParseUser.getCurrentUser() is still null. Is that normal? My idea is to begin working with the current user immediately when the application starts, not requiring the user to login/signup at first.

@flovilmart
Copy link
Contributor Author

You have to enable it on the client, it's enabled by default on the server

@rafapetter
Copy link

Ok, got it. Thanks

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.

5 participants