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

iOS SDK sends x-parse-client-key for push notifications but server expects X-Parse-Master-Key` #3455

Closed
jaimeagudo opened this issue Jan 30, 2017 · 12 comments

Comments

@jaimeagudo
Copy link

jaimeagudo commented Jan 30, 2017

Issue Description

Describe your issue in as much detail as possible.

Using iOS SDK the headers when sending Push Notifications for the master-key are simply inconsistent following the tutorial. The SDK sends the key in x-parse-client-key but the REST is waiting for X-Parse-Master-Key. Hence my curl tests are fine but my iOS SDK is failing.

See https://github.com/ParsePlatform/parse-server/blob/f864141663533bc6195dcbd4622e735798bfac81/src/middlewares.js#L24

[FILL THIS OUT]

Steps to reproduce

Please include a detailed list of steps that reproduce the issue. Include curl commands when applicable.

  1. Open a sample iOS SDK Project
let push = PFPush()
        pushQuery.whereKey("user", equalTo:  PFUser.current()!)
        push.setQuery(pushQuery) 
        push.setMessage("hello world")
        push.sendInBackground()

Expected Results

The logs on server are ok, returns 200

Actual Outcome

"error": {
        "status": 403,
        "message": "unauthorized: master key is required"
    },

Environment Setup

  • Server

    • parse-server version (Be specific! Don't say 'latest'.) : 2.3.2
    • Operating System: Ubuntu 16.04
    • Hardware: LINODE
    • Localhost or remote server? Linode
  • Database
    IRRELEVANT

Logs/Trace

{
    "method": "POST",
    "url": "/parse/push",
    "headers": {
        "x-forwarded-for": "88.17.YY.XXX",
        "host":"myserver:41337",
        "x-nginx-proxy": "true",
        "connection": "upgrade",
        "content-length": "165",
        "x-parse-client-version": "i1.14.2",
        "accept": "*/*",
        "x-parse-session-token": "r:7df48309d473e1e9e403aa0c2e35dcfb",
        "x-parse-application-id": "MYAPP",                               
        "x-parse-client-key": "myprivatekey_justfine",
        "x-parse-installation-id": "12afc24d-37c0-484f-9e73-1bf6756c3d4c",
        "x-parse-os-version": "10.2 (16B2555)",
        "accept-language": "en-us",
        "accept-encoding": "gzip, deflate",
        "content-type": "application/json; charset=utf-8",
        "user-agent": "app/1 CFNetwork/808.2.16 Darwin/16.1.0",
        "x-parse-app-build-version": "1",
        "x-parse-app-display-version": "0.1"
    },
    "body": {
        "where": {
            "user": {
                "__type": "Pointer",
                "className": "_User",
                "objectId": "p1w7xaoRAL"
            }
        },
        "data": {
            "alert": "Carol J would love to work at Sonora Disco! Call him/her now!"
        }
    },
    "level": "verbose",
    "message": "REQUEST for [POST] /parse/push: {\n  \"where\": {\n    \"user\": {\n      \"__type\": \"Pointer\",\n      \"className\": \"_User\",\n      \"objectId\": \"p1w7xaoRAL\"\n    }\n  },\n  \"data\": {\n    \"alert\": \"Hi Carol !\"\n  }\n}"
}
1 | Paid2Par | {
    "error": {
        "status": 403,
        "message": "unauthorized: master key is required"
    },
    "level": "error",
    "message": "Error generating response. { Error: unauthorized: master key is required\n    at promiseEnforceMasterKeyAccess (/home/jaime/production/source/node_modules/parse-server/lib/middlewares.js:297:17)\n    at /home/jaime/production/source/node_modules/parse-server/lib/PromiseRouter.js:132:22\n    at process._tickDomainCallback (internal/process/next_tick.js:129:7) status: 403, message: 'unauthorized: master key is required' }"
}
1 | myapp | Error: unauthorized: master key is required
1 | myapp | at promiseEnforceMasterKeyAccess(/home/jaime / production / source / node_modules / parse - server / lib / middlewares.js: 297: 17)
1 | myapp | at / home / jaime / production / source / node_modules / parse - server / lib / PromiseRouter.js: 132: 22
1 | myapp | at process._tickDomainCallback(internal / process / next_tick.js: 129: 7)

You can turn on additional logging by configuring VERBOSE=1 in your environment.

Possibly related to #3022

Just in case you doubt about the installation requests with

 "method": "POST",
    "url": "/parse/classes/

    "x-parse-client-key":

Works just fine

@natanrolnik
Copy link
Contributor

That's correct. Client push is not allowed anymore, only with Master Key.

@jaimeagudo
Copy link
Author

Thanks for the quick reply but... what should i do? patch parse-server? @natanrolnik

@jaimeagudo
Copy link
Author

BTW What's the reasoning of not discharging the server load and save a bit on hosting?

@natanrolnik
Copy link
Contributor

One option, if you really want client push, would be to fork parse-server and allow it...

But it's not a bug - it's by design. Client push is not secure, and you wouldn't want random clients with being able to send pushes in your behalf. Create a cloud function, and send the push from there. Not to mention it will be much more flexible than shipping code to your clients that gets more time to be updated than your server code.

What do you mean by "discharging the server load and save a bit on hosting"?

@jaimeagudo
Copy link
Author

From a security standpoint I don't see the difference from my ignorance, AFAIK you are sending HTTP requests to a server if you intercept the master-key you can call cloud functions and run the same functionality. And a difference in favour of the SDK requests is that they need a session key which is temporary. Am I getting something wrong @natanrolnik ?

*Cloud call

curl -X POST \
  -H "X-Parse-Application-Id: ${APPLICATION_ID}" \
  -H "X-Parse-REST-API-Key: ${REST_API_KEY}" \
  -H "Content-Type: application/json" \
  -d '{}' \
  https://api.parse.com/1/functions/mycustompush
  • Push call
"method": "POST",
"url": "/parse/push",
"headers": {
    "x-forwarded-for": "88.17.70.200",
    "host": "p.aidto.party:41337",
    "x-parse-session-token": "r:7df48309d473e1e9e403aa0c2e35dcfb",
    "x-parse-client-key": "111111122223344444555667"
},
"body": {
    "where": {
        "user": {
            "__type": "Pointer",
            "className": "_User",
            "objectId": "p1w7xaoRAL"
        }
    },
    "data": {
        "alert": "Hello world"
    }
},
"level": "verbose",

@jaimeagudo
Copy link
Author

jaimeagudo commented Jan 30, 2017

Why did you you close the issue @natanrolnik ? Even if you are right you should not close it without referring to other issue (if duplicated). You are making an small favour with such contribution, docs are not coherent at minimum and there is no synch with the https://github.com/ParsePlatform/Parse-SDK-iOS-OSX/ repo.

https://parseplatform.github.io/docs/ios/guide/#push-notifications

Can you @flovilmart please briefly explain what's the expected behaviour? Thanks

@natanrolnik
Copy link
Contributor

natanrolnik commented Jan 30, 2017

@jaimeagudo first things first:

1 - By design, Parse Server was not built to support client pushes, and unfortunately, the wiki, docs and the iOS SDK are not in sync. Even with Parse.com, they recommended to disable client push in production (allowing it only for development purposes), and you can read more here.

Believe me, this is a real security breach and I personally have experienced this happening in production a few months ago. Someone hacked the app and sent a push notification to more than a million devices because push client was enabled.

2 - The master key should never be shipped with clients. Using it in the server, or between servers that you control is fine.

When I mentioned a cloud function, I didn't refer to passing the master key; I was referring to creating a cloud function that is triggered by the client, and from there, using the master key is safe to send the push you want.

3 - If we get to a conclusion that this is a real issue, and parse-server should support client push, we can reopen this issue with just one click 🙂.

@natanrolnik
Copy link
Contributor

natanrolnik commented Jan 30, 2017

@jaimeagudo as you requested, here are a few issues that mention push and client push, (and one of them was very similar and closed): #575, #564, #401 and #601.

They might help you; otherwise, I'll be happy to assist you with this. My email is available in my profile page.

As sending push via iOS is not supported anymore, PRs are welcome on both Parse docs and in the iOS SDK to remove the push related code.

@flovilmart
Copy link
Contributor

So, I'll weigh in,

We disabled the ability to send push notifications from clients for security reasons, as it was originally encouraged on parse.com.

Regarding the masterKey interception, if your master key leaks, you should rotate it, your master key should be treated as sensitive as your database passwords, private keys etc...

@natanrolnik provided a proper explanation for the expected behavior.

If you really wish to allow push on client, you can either use your own fork, or open a PR that introduces option like allowClientPush (Which would be off by default).

@jaimeagudo
Copy link
Author

Hi @natanrolnik @flovilmart, sorry for the delayed response and thanks for your time. I'll try to send a PR to update docs once I get the whole picture but it's really a pity this lack of synch for so "long".

Couple of extra questions:
@natanrolnik you said The master key should never be shipped with clients. but that sounds impossible to me in order to authenticate the app I have to include it on the iOS app somehow

Apart of the "immediate" empty body response sent by Jobs (vs Cloud code) is there any other difference regarding their execution priority? I mean, are they run as soon as their requests are received or is there any kind of queue for low-priority tasks? My idea is to use Jobs intensively if it's not discouraged

Thanks again

@flovilmart
Copy link
Contributor

The master key should never be shipped with clients, you SHOULD never ship the master key, think of if as your root password for the parse-servers. IF it came to leak, one would have full access and could drop all the objects in your database.

The main difference between jobs and cloud functions are:

Job Cloud Function
require masterKey x
can be run by client x
returns immediately x user defined
provides result to the client

@jaimeagudo
Copy link
Author

jaimeagudo commented Feb 3, 2017

Thanks for the clarification @flovilmart, will go with jobs then in a Fire & tell fashion (AKKA style). Regarding the master key I have been confused, I assumed the clientKey was synonymous of the masterKey. I issued parse-community/Parse-SDK-iOS-OSX#1102 btw, those links were lost in redirection loops

jaimeagudo added a commit to jaimeagudo/docs that referenced this issue Feb 3, 2017
Avoid newcomers wasting time implementing something useless
See parse-community/parse-server#3455
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

No branches or pull requests

3 participants