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

PM-10600: Re-register device with push token every 7 days. #4303

Closed
wants to merge 2 commits into from

Conversation

mzieniukbw
Copy link

@mzieniukbw mzieniukbw commented Nov 14, 2024

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-10600

📔 Objective

The Azure Notification Hub tags can change frequently, like here for Notification Center work we need to add new tags to existing devices registrations. Instead of painful migration plan, this PR adds a frequent 7 day interval re-registration of push notification for the device against Bitwarden Server.

Technical change:

  • A new coroutine is added that runs every 1 minute (scheduled job) and detects if the last time the currently stored push was sent to Bitwarden Server is older than 7 days.
    • If true, the current device is re-registered against Bitwarden Server. In normal circumstances this would have no effect whatsoever from server point of view, but in case a new tag needs to be added for Azure Notification Hub, that is being now done with this functionality.
    • If false, nothing happens.
  • On user id change (login, switch user, first app open with locked vault):
    • we run the coroutine immediately, for an edge case, that app was opened, vault unlocked and app killed immediately after (does not reach 1 minute delay).
    • the above coroutine is started with initial delay of 1 minute, for an edge case, that app is never killed and runs constantly.
  • Change the meaning of the LastPushTokenRegistrationDate, to only serve the above coroutine case.

📸 Screenshots

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation or informed the documentation team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed
    issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

The Azure Notification Hub tags can change frequently, like here for Notification Center work we need to add new tags to existing devices registrations. Instead of painful migration plan, this PR adds a frequent re-registration of push notification for the device against Bitwarden Server.
Copy link
Contributor

github-actions bot commented Nov 14, 2024

Logo
Checkmarx One – Scan Summary & Details6fb13240-44b0-490f-b382-9c583e5d2456

No New Or Fixed Issues Found

@mzieniukbw mzieniukbw closed this Nov 26, 2024
@mzieniukbw mzieniukbw deleted the km/pm-10600-re-register-push-token-intervals branch November 26, 2024 14:54
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.

1 participant