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

sdk/azcore: TokenCredential should not be internal #17522

Closed
ItalyPaleAle opened this issue Apr 11, 2022 · 6 comments
Closed

sdk/azcore: TokenCredential should not be internal #17522

ItalyPaleAle opened this issue Apr 11, 2022 · 6 comments
Assignees
Labels
Azure.Core Azure.Identity issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@ItalyPaleAle
Copy link
Member

Bug Report

In sdk/azcore, the TokenCredential interface, as well as related structs such as TokenRequestOptions are in an internal package and cannot be used by third-party code.

We are working with APIs that are not supported by the SDKs yet, including some that have no roadmap to be supported. In order to interact with them, we need to retrieve access tokens. Because the TokenRequestOptions is internal, we cannot invoke GetToken on an azcore.TokenCredential object and retrieve an access token. Thus, we cannot make API calls without having to also re-implement the entire auth flow.

  • import path of package in question, e.g. sdk/azcore
  • SDK version e.g. 0.23.0
  • output of go version: go version go1.18 linux/amd64
@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Apr 11, 2022
@chlowell chlowell added question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Azure.Core Azure.Identity and removed needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels Apr 11, 2022
@chlowell chlowell self-assigned this Apr 11, 2022
@chlowell
Copy link
Member

chlowell commented Apr 11, 2022

TokenCredential and TokenRequestOptions are exported, though from different packages, so it's possible to implement custom credential types and acquire a token from any credential type in azidentity:

package main

import (
    "context"

    "github.com/Azure/azure-sdk-for-go/sdk/azcore"
    "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
    "github.com/Azure/azure-sdk-for-go/sdk/azidentity"
)

// implementing TokenCredential
type CustomCredential struct {}

func (c *CustomCredential) GetToken(ctx context.Context, options policy.TokenRequestOptions) (*azcore.AccessToken, error) {
	return nil, nil
}

var _ azcore.TokenCredential = (*CustomCredential)(nil)

// calling GetToken()
func main() {
	cred, err := azidentity.NewDefaultAzureCredential(nil)
	if err != nil {
		panic(err)
	}
	tk, err := cred.GetToken(
		context.TODO(), policy.TokenRequestOptions{Scopes: []string{"https://management.azure.com/.default"}},
	)
	if err != nil {
		panic(err)
	}
}

@chlowell chlowell added the issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. label Apr 11, 2022
@ghost
Copy link

ghost commented Apr 11, 2022

Hi @ItalyPaleAle. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text “/unresolve” to remove the “issue-addressed” label and continue the conversation.

1 similar comment
@ghost
Copy link

ghost commented Apr 11, 2022

Hi @ItalyPaleAle. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text “/unresolve” to remove the “issue-addressed” label and continue the conversation.

@ItalyPaleAle
Copy link
Member Author

Thanks for the explanation. I'll take a look at that and will let you know if I run into any issue.

@jhendrixMSFT
Copy link
Member

For some additional context, there are two reasons why we have internal types that are exported via type aliases.

  • To break cyclic dependencies.
  • To offer a type in one package (e.g. azcore) but its usage in another (e.g. runtime).

I've refactored the internal content that's actually exported under an internal/exported package to make it clear. In this process, it was discovered that we didn't actually need to do this with TokenCredential and its related types, so those are now defined in the packages from where they're exported.

@ghost
Copy link

ghost commented Apr 26, 2022

Hi @ItalyPaleAle, since you haven’t asked that we “/unresolve” the issue, we’ll close this out. If you believe further discussion is needed, please add a comment “/unresolve” to reopen the issue.

@ghost ghost closed this as completed Apr 26, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2023
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Core Azure.Identity issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Development

No branches or pull requests

3 participants