Fix the presence write commands - #72
Open
aberoham wants to merge 5 commits into
Open
Conversation
The presence write commands all call Graph operations that require the delegated Presence.ReadWrite permission, but the built-in scope string asked only for Presence.Read.All, so every one of them returned 403 for a login that did not override the defaults. Microsoft does not mark Presence.ReadWrite as admin-consent required, so it can join the default set. Closes osodevops#70
Graph identifies a presence session by the application that owns it, and both setPresence and clearPresence expect that application's ID as sessionId. clearPresence was sent an empty body and rejected with a 400, while setPresence sent a fresh UUID on every invocation, so nothing could name a session again once it was opened. Both now read the ID from the access token's azp or appid claim, ignoring blank values, which stays correct if the profile's configured client ID changes or the token arrived through TEAMS_CLI_ACCESS_TOKEN. Closes osodevops#71
`--scopes`, `TEAMS_CLI_SCOPES` and a profile's `scopes` field replace the default delegated scope string rather than extend it, so the instruction to log in again after `Presence.ReadWrite` joined the defaults only works for logins that take the defaults. A login that names its own scopes has to restate the whole list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Graph keys a presence session to the application that owns it, and the CLI read that application's ID out of the access token's `azp` or `appid` claim. Microsoft asks callers to treat access tokens as opaque, and a Graph token is not guaranteed to be a readable JSON Web Token, so a configured client ID — `TEAMS_CLI_CLIENT_ID` or the profile's `client_id` — now names the application directly and wins. The token claim remains the fallback, which is what logins through the built-in application use, since those configure no client ID. `set` and `clear` still agree, because both resolve the value the same way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Graph answers 404 when the application has no presence session to clear, which is the state `clear` exists to reach, so a second clear — or a retry after an ambiguous response — failed against presence that was already automatic. That answer is no longer an error. It is not the same outcome as closing a live session, though: a session opened under a different application ID answers the same way. `clear` now reports which one Graph gave, `presence_cleared` or `no_presence_session`, and both `set` and `clear` report the session ID they used, so a `clear` run under different configuration than the `set` is visible rather than silent. A retry whose earlier attempt succeeded but lost its response also sees 404. The prose and a wiremock test covering 503 then 404 both say so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Default delegated logins omitted the permission all three presence-write
commands need.
presence setandpresence clearalso sent an invalid ormissing
sessionId. Reproductions and the Graph responses are in #70 and #71.Default scopes (#70)
setPresence, setStatusMessage and clearPresence all require the delegated
Presence.ReadWritepermission, whichDEFAULT_DELEGATED_SCOPESdid notrequest. Microsoft does not mark it admin-consent required, so it
can join the default set.
docs/auth.mdanddocs/faq.mdnow list it with therest.
Existing sessions keep the scopes they were granted, so users need to run
teams auth loginagain.auth refreshcannot add a scope that was neverconsented to. That instruction only reaches logins that take the defaults:
--scopes,TEAMS_CLI_SCOPESand a profile'sscopesfield replace thedefault set rather than extend it, so a login that names its own scopes has to
restate the whole list with
Presence.ReadWritein it.README.mdanddocs/auth.mdsay so.Deployment follow-up, outside this PR: verify whether the OSO app
registration's static delegated-permission list also needs updating.
Session ID (#71)
Graph identifies a presence session by the application that owns it, and both
setPresence and clearPresence document that application's ID as the
sessionIdthey expect.clearPresencewas sent an empty body;setPresencesent a new random UUID on every invocation.
Both now use a configured client ID —
TEAMS_CLI_CLIENT_IDor the profile'sclient_id— when there is one, because that names the application directlyand Microsoft asks callers to treat access tokens as opaque. Otherwise the
value comes from the token's own claims, preferring
azpand falling back toappid, matchingredeem_refresh_token's claim order; that is the path alogin through the built-in application takes, since it configures no client ID.
Blank values are skipped, and a token with no usable application-ID claim and
no configured client ID fails with an auth error rather than a guess.
setandclearagree either way, because both resolve the value the same way.Clearing a session that is not there
Graph answers 404 when the application has no presence session to clear, which
is the state
clearexists to reach, so a second clear — or a retry after anambiguous response — failed against presence that was already automatic. That
answer is no longer an error.
It is not the same outcome as closing a live session: a session opened under a
different application ID answers the same way.
clearreports which one Graphgave,
presence_clearedorno_presence_session, and bothsetandclearreport the session ID they used, so a
clearrun under different configurationthan the
setis visible rather than silent. A retry whose earlier attemptsucceeded but lost its response also sees 404, and the prose says so.
Tests
wiremock asserts the serialized
setPresenceandclearPresencepayloads, andcovers
clearPresenceanswering 404 directly and answering it after a retried503. Unit tests cover default-scope membership and session-ID selection: a
configured client ID winning over the token, a blank one falling through, an
opaque token accepted when a client ID is configured, and the
azp/appidorder, blank claims and undecodable tokens on the fallback path.
Not covered
--expirationis still passed through without local validation. Presence writesalso do not reject app-only tokens locally, unlike message writes, which call
require_delegated_token.