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

[Bug] The suggested partitioning scheme breaks silent authentication #424

Open
chlowell opened this issue May 25, 2023 · 0 comments
Open
Labels
bug Something isn't working p2 public-client

Comments

@chlowell
Copy link
Collaborator

...because clients suggest partition keys inconsistently. For example, here's a repro using interactive auth:

package main

import (
	"context"
	"fmt"
	"path"
	"runtime"

	"github.com/AzureAD/microsoft-authentication-library-for-go/apps/cache"
	"github.com/AzureAD/microsoft-authentication-library-for-go/apps/public"
)

type logger struct{}

func (*logger) Replace(ctx context.Context, cache cache.Unmarshaler, cacheHints cache.ReplaceHints) error {
	pc, _, _, _ := runtime.Caller(2)
	details := runtime.FuncForPC(pc)
	fmt.Printf("%s called Replace with suggested partition key %q\n", path.Base(details.Name()), cacheHints.PartitionKey)
	return nil
}

func (*logger) Export(ctx context.Context, cache cache.Marshaler, cacheHints cache.ExportHints) error {
	pc, _, _, _ := runtime.Caller(2)
	details := runtime.FuncForPC(pc)
	fmt.Printf("%s called Export with suggested partition key %q\n", path.Base(details.Name()), cacheHints.PartitionKey)
	return nil
}

func main() {
	pca, err := public.New("04b07795-8ddb-461a-bbee-02f9e1bf7b46", public.WithCache(&logger{}))
	if err != nil {
		panic(err)
	}
	ctx := context.Background()
	scopes := []string{"https://management.azure.com//.default"}
	ar, err := pca.AcquireTokenInteractive(ctx, scopes)
	if err != nil {
		panic(err)
	}
	_, err = pca.AcquireTokenSilent(ctx, scopes, public.WithSilentAccount(ar.Account))
	if err != nil {
		panic(err)
	}
}

Output:

public.Client.AcquireTokenInteractive called Replace with suggested partition key ""
public.Client.AcquireTokenInteractive called Export with suggested partition key ""
base.Client.AcquireTokenSilent called Replace with suggested partition key "oid.tid"

Only AcquireTokenSilent suggests a key. It therefore always fails for an application using the key to look up a partition because the partition in question will always be empty or nonexistent, because AcquireTokenInteractive didn't suggest that key after acquiring tokens.

Part, if not all, of the problem is that AuthParams.CacheKey() doesn't consider all values of AuthorizationType (the value is ATInteractive in interactive auth). Maybe it should have a default case? I see there's also TokenResponse.CacheKey(). That may also need an update, or perhaps there's a way to consolidate these methods.

Another part of the problem is test gaps--neither CacheKey() is tested, nor is the value of AuthParams.AuthorizationType set by AcquireToken methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p2 public-client
Projects
None yet
Development

No branches or pull requests

2 participants