pusher / push-notifications-android

Android SDK for Pusher Beams
https://www.pusher.com/beams
MIT License
21 stars 22 forks source link

Security concern #83

Closed suragch closed 4 years ago

suragch commented 5 years ago

In the Android API for .setUserId(), one needs to create a BeamsTokenProvider before calling .setUserId().

val tokenProvider = BeamsTokenProvider(
    "<YOUR_BEAMS_AUTH_URL_HERE>",
    object: AuthDataGetter {
      override fun getAuthData(): AuthData {
        return AuthData(
            // Headers and URL query params your auth endpoint needs to
            // request a Beams Token for a given user
            headers = hashMapOf(
                // for example:
                // "Authorization" to sessionToken
            ),
            queryParams = hashMapOf()
        )
      }
    }
)

In doing that I am giving Pusher access to sensitive user authentication data (login credentials or tokens) for my app. It isn't clear what Pusher is doing with this data. Is it getting sent to Pusher servers? I don't think so, but since giving Pusher the user auth data isn't strictly necessary, I recommend an API change.

Let the client get the beams token from the server themself without Pusher API help. Then they can use the Pusher API to set the user id. Something like this would be preferable:


val beamsToken = myOwnServerRequestToGetTheBeamsToken();

PushNotifications.setUserId(
    "<USER_ID_GOES_HERE>",
    beamsToken,
    object : BeamsCallback<Void, PusherCallbackError> { ... }
)

In this way it shields sensitive user data and it keeps Pusher from being liable for it.

jonathanlloyd commented 5 years ago

Hi, thanks for reaching out!

I'm afraid that simply passing in the token won't work because there are some conditions where the SDK is required to reauthenticate with Beams (so it needs to be able to request Beams Tokens on demand).

However, com.pusher.pushnotifications.auth.TokenProvider is actually an interface - the BeamsTokenProvider class is just our default implementation (which does not share any sensitive data with Pusher). However, you can also provide your own implementation if desired.

Let me know if that helps!

suragch commented 5 years ago

Thanks for your reply. That helps. I don't think I would implement my own now but it was useful to view the source code for TokenProvider and BeamsTokenProvider. I am glad to hear that the Pusher implementation doesn't share any sensitive data with the Pusher servers.

Could I know the conditions where the SDK is required to reauthenticate with Beams? (I suppose one would be when the token expires.) That would be helpful to understand since I am in the process of making a Pusher SDK for Flutter.

luismfonseca commented 4 years ago

Could I know the conditions where the SDK is required to reauthenticate with Beams? (I suppose one would be when the token expires.) That would be helpful to understand since I am in the process of making a Pusher SDK for Flutter.

Sorry for the late reply.

Once a device is logged in as a user, the token doesn't expire. What can happen is that the device can log out (by calling stop or clearAllState), or the device becomes invalid for some reason (gateway token now invalid? user changed the push notifications permissions), or it can be deleted from the server remotely. In these cases, (except the stop) the device will go through a new device creation flow again, and a new token will be requested.

Because the security concern isn't really pertinent to these questions, I'll close the issue, but feel free to follow up here or with support@pusher.com. Thanks!