Fix up out-of-office annotations#259
Open
kroeschl wants to merge 14 commits into
Open
Conversation
kroeschl
force-pushed
the
ooo-annotation
branch
2 times, most recently
from
c91e9ec to
8c4de0c
Compare
Assigning untrusted HTML to innerHTML (even on a detached node) parses it and fires resource-loading events (e.g. <img onerror>). DOMParser gives the same tag-stripping without ever creating live elements.
Each reviewer on a PR calls getOooGraphAccessToken concurrently. The old boolean flag was set before the token resolved, so only the first reviewer triggered auth while the rest got null and showed no annotation until a reload found the cached token. Caching the in-flight promise lets every reviewer resolve from a single attempt, while still limiting it to one attempt per page session (a resolved-null promise does not retry).
Separate the concurrency-coalescing from the once-per-session popup guard: _oooAuthPromise now only coalesces concurrent callers and resets when the acquisition settles (via .finally), while a dedicated _oooSilentAuthAttempted boolean keeps the fallback popup to one attempt per page session. This stops the shared promise from pinning the first resolved token for the whole page session and makes room for alternative acquisition paths.
Request the offline_access scope so AAD returns a refresh token, then mint new access tokens via a background POST (grant_type=refresh_token) instead of a popup. Because the redirect URIs are SPA-type, refresh tokens expire 24h after the original interactive login and that expiry carries over across rotations, so a silent popup is still needed at most once per day rather than ~hourly. - _oooStoreTokens centralizes access/refresh token persistence and stamps the 24h refresh-token expiry only on a fresh login (not on rotation). - _oooRefreshAccessToken does the no-popup refresh and drops the stored refresh token if AAD rejects it, falling back to the silent popup. - The coalesced acquisition now tries the refresh token before the popup. - Reconfiguring the client ID clears the stored refresh token (different app).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Justification
Out-of-office annotations have been broken for a good long while due to IT policy changes after Emerson acquired NI. Additionally, this feature only ever worked for NI users because we fetched OOO user data from an internal source.
Implementation
Testing
I tested this in my browser and now see annotations for OOO data. I also tweaked to script to test token refresh and saw that succeed as expected (without popup).
