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

Fix for unauthorized access and push notification #1051

Merged
merged 2 commits into from Aug 16, 2017

Conversation

Projects
None yet
4 participants
@kunall17
Contributor

kunall17 commented Aug 16, 2017

ddec1c2 aims to fix the Unauthorised errors, its not a good thing for a server
device-2017-08-16-230805

This prevents to running of the event loop if the apiKey is '', which can only happen if the user logouts!

255448c: Due to some refactoring the savePushToken wasn't working fine, now it will

@smarx

This comment has been minimized.

smarx commented Aug 16, 2017

Automated message from Dropbox CLA bot

@kunall17, it looks like you've already signed the Dropbox CLA. Thanks!

@zulipbot zulipbot added reviewed and removed needs review labels Aug 16, 2017

@borisyankov

Looks good. Not sure about the error handling.
A console.log is no good apart from local debugging?

try {
await unregisterPush(auth, pushToken);
} catch (e) {
console.log('failed to unregister Push token', e); // eslint-disable-line

This comment has been minimized.

@borisyankov

borisyankov Aug 16, 2017

Contributor

Is this enough? Are you using it to silence the exception or?
When does that happen?

This comment has been minimized.

@kunall17

kunall17 Aug 16, 2017

Contributor

Ideally we should hook up the sentry to log these error events, I'm not sure how to do it know, hence a log statement!

It shouldn't happen though, the only param is the pushToken and this function is called the token is not ''

This comment has been minimized.

@borisyankov

borisyankov Aug 16, 2017

Contributor

OK, good that you have considered it. Indeed we can hook up Sentry later.

@@ -77,11 +77,15 @@ export const deleteTokenPush = (): Action => ({
type: DELETE_TOKEN_PUSH,
});
export const saveTokenPush = (pushToken: string): Action => ({
const saveTokenPush = (pushToken: string) => ({

This comment has been minimized.

@borisyankov

borisyankov Aug 16, 2017

Contributor

Did you intentionally remove the export?

This comment has been minimized.

@kunall17

kunall17 Aug 16, 2017

Contributor

Yup, the saveTokenPush is not being used any where else

@borisyankov borisyankov merged commit 82f3ab0 into zulip:master Aug 16, 2017

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@zulipbot zulipbot removed the reviewed label Aug 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment