Skip to content

feat(client): return the account username with the Docker Hub session - #683

Closed
Benehiko wants to merge 2 commits into
mainfrom
feat/dockerhub-session-username
Closed

Benehiko wants to merge 2 commits into
mainfrom
feat/dockerhub-session-username

Conversation

@Benehiko

@Benehiko Benehiko commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

Part of #682 (item 1).

GetDefaultSession already reads the default profile to find the account entry, but it threw the profile away. UserSession.Claims.Username is often empty. That forces callers such as Compose to call GetDefaultProfile a second time to get the username a registry credential needs.

This adds UserSession.Username:

  • GetDefaultSession sets it to the default profile's username, or to the token's username claim if the profile has none.
  • GetSession sets it to the requested username, which is the account entry key (docker/auth/hub/<username>).

The field is tagged json:"-" because the client fills it in; it is not read from the stored payload. Adding a struct field doesn't break callers, and the ClientAuth interface is unchanged. On the Compose side, the profile fallback becomes session.Username.

Details

Open questions

  • Precedence. This PR checks the profile first, then the claim. Compose does the reverse. I chose the profile because the session is found through it and it is already loaded. In practice the two should match.
  • Empty username. If neither the profile nor the claims has a username, GetDefaultSession still succeeds with Username == "", so callers that only need the token keep working. Two other options are left out for now: fall back to the last part of the profile's user_id (the account key), or return an error.

Out of scope: items 2–5 of #682 (expiry helper, ErrSessionExpired, an "unavailable" check, connecting to the Desktop socket). These can be separate PRs.

Testing: go test -race ./... and golangci-lint run in client/ (0 issues). New subtests cover each username source, the precedence, the empty case, and a top-level username in the stored payload being ignored. One test checks that GetDefaultSession does exactly one profile lookup and one session lookup.

AI usage: The code, tests and this description were drafted with alki, a coding agent built on Claude.

GetDefaultSession already loads the default profile to find the account entry, but threw it away. Since the token's username claim is often empty, callers had to call GetDefaultProfile again to get a username for a registry credential.

Add UserSession.Username. GetDefaultSession fills it from the default profile, falling back to the username claim; GetSession fills it from the requested username. The field is not decoded from the stored payload.

Refs #682

Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
@Benehiko
Benehiko marked this pull request as ready for review October 9, 2026 05:28
Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

@Benehiko
Benehiko marked this pull request as draft October 9, 2026 06:05
@Benehiko

Benehiko commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

Closing: UserSession.Claims.Username already carries the username, so a second Username field is not needed. It was empty because Docker Desktop did not always parse the token claims; that is fixed, and a migration is underway to make it consistent. See #682.

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.

2 participants