Skip to content

[Bug] Dead Code (and possible bug) in CacheKey generation for Account retrieval #577

@merlinio2000

Description

@merlinio2000

Which version of MSAL Go are you using?
Note that to get help, you need to run the latest version.
Microsoft Authentication Library for Go 1.4.2

Where is the issue?

  • Public client
    • Device code flow
    • Username/Password (ROPC grant)
    • Authorization code flow
  • Confidential client
    • Authorization code flow
    • Client credentials:
      • client secret
      • client certificate
  • Token cache serialization
    • In-memory cache
  • Other (please describe)

Is this a new or an existing app?

This is a new app.

What version of Go are you using (go version)?

$ go version
go version go1.24.5 darwin/arm64

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
AR='ar'
CC='cc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='c++'
GCCGO='gccgo'
GO111MODULE=''
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/Users/merlin/Library/Caches/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/Users/merlin/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/46/3j7_x4v13wscs3b7dvs1dm3r0000gn/T/go-build3069806990=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/merlin/Documents/JLS/digitaler-empfang-backend/go.mod'
GOMODCACHE='/Users/merlin/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/merlin/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.24.5/libexec'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/merlin/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.24.5/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.24.5'
GOWORK=''
PKG_CONFIG='pkg-config'

Repro
Not a repro but a description of the possibly buggy code in this library from apps/internal/base/base.go
The fields of the authParams copy get populated with values for the cache key but instead of using the authParams copy to generate the CacheKey the unmodified original is used. One of my golangci-lint linters is also complaining about this being dead code. I don't know what the intended/correct behavior is here otherwise I would've directly opened a PR.

func (b Client) Account(ctx context.Context, homeAccountID string) (shared.Account, error) {
	if b.cacheAccessor != nil {
		b.cacheAccessorMu.RLock()
		defer b.cacheAccessorMu.RUnlock()
		authParams := b.AuthParams // This is a copy, as we don't have a pointer receiver and .AuthParams is not a pointer.
		authParams.AuthorizationType = authority.AccountByID
		authParams.HomeAccountID = homeAccountID
		key := b.AuthParams.CacheKey(false)
		err := b.cacheAccessor.Replace(ctx, b.manager, cache.ReplaceHints{PartitionKey: key})
		if err != nil {
			return shared.Account{}, err
		}
	}
	return b.manager.Account(homeAccountID), nil
}

Expected behavior
CacheKey gets correctly set for each usecase

Actual behavior
I have not experienced any actual errors so far, I was simply digging around to learn more about the not so optimally documented cache partitioning.

Possible solution
Remove the assignments / use the copy to generate the cache key

Additional context / logs / screenshots
Add any other context about the problem here, such as logs and screenshots.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions