Surge token deletion on logout #1270
Summary
Closes #1270
Currently on the mobile app if you log into one account, then log out and into a different account, you will still receive push notifications for the first account.
This is because the Surge tokens (used for push notifications) that we use on the backend are not deleted on logout.
Steps
To hammer out with @brianhatchet and @msantang78. This is going to likely be tricky to test as we do not digest push notifications on the front-end, just the mobile app. This may require us using a manual build of the app and plugging it into the associated sandbox.
I would recommend testing sessions and account swapping/triggering notifications.
Regression Scope
Changes v Impact
- api/v1/notifications - Simple change, setting the token on notif POST (from mobile).
- Session\Manager - limited to the destroy functionality, so would affect anything that destroys a session (e.g. logging out).
- Entities\User - added functions and an exported value, I don't foresee any issues here.
added scoped labels
- Developer
Set to blocked pending chat with Brian and Martin.
assigned to @brianhatchet, @msantang78, and @benhayward.ben
- Developer
Testing on the app requires two things that we don't have.
- An ability to change the base url in the app for testing purposes.
- The ability to push notifications from the review sites.
So, let's add some logging to show the push notifications going out so we can verify the body of the notification. Once we're sure the logging works, we'll verify it on staging
added scoped label and automatically removed label
146 146 'service' => $service, 147 147 'token' => $passed_token 148 148 ]); 149 150 Core\Session::getLoggedInUser() 151 ->setSurgeToken($token) 152 ->save(); - Developer
We need to dump notification JSON do it in notifications file, but behind developer mode or feature flag.
- Resolved by Ben Hayward
- Resolved by Ben Hayward
- Developer
Added a couple of feedback items, @benhayward.ben
added scoped label and automatically removed label
resolved all threads
146 146 'service' => $service, 147 147 'token' => $passed_token 148 148 ]); 149 150 Core\Session::getLoggedInUser() - Owner
Not sure why we need to make this change but if we are then
Core\Entities\Actions\Save
should be used.
- Last updated by Ben Hayward
106 106 $push['big_picture'] = static::getNotificationBigPicture($notification, $from_user, $entity); 107 107 $push['group'] = static::getNotificaitonGroup($notification, $from_user, $entity); 108 108 109 // temporary logging 110 error_log(var_export($push, true)); - Owner
We shouldn't be putting this on production
- Developer
Definitely not, It's only there to test something on sandboxes, I'll leave this unresolved until its gone so that it doesn't get missed.
246 247 public function destroy($all = false) 247 248 { 248 249 if ($this->session) { 249 $this->repository->delete($this->session, $all); 250 if (!$this->user) { 251 // If no user set grab from session. 252 $this->user = new User($this->session->getLoggedInUser(), false); - Owner
Dangerous building of user entity. EntitiesBuilder->single should be used
246 247 public function destroy($all = false) 247 248 { 248 249 if ($this->session) { 249 $this->repository->delete($this->session, $all); 250 if (!$this->user) { 251 // If no user set grab from session. 252 $this->user = new User($this->session->getLoggedInUser(), false); 253 } 254 255 $this->user->setSurgeToken('') // Delete push notification token. - Owner
Why are you deleting surge tokens when a session is deleted? Mobile apps use OAuth and not sessions. If I log out on desktop, I'm going to stop receiving notifications everywhere.