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

Large file upload is sending Authorization header, causing email attachment upload to fail. #320

Closed
jasonjoh opened this issue Aug 20, 2024 · 8 comments · Fixed by #321
Closed
Assignees

Comments

@jasonjoh
Copy link
Member

Describe the bug

Attempt to upload a file to an email draft as an attachment fails. Each PUT request to upload a slice of the file gets a 401 error with the following payload.

{"error":{"code":"InvalidAudience","message":"The audience claim value is invalid '00000003-0000-0000-c000-000000000000/@'.","innerError":{"oAuthEventOperationId":"f58e4b30-148c-437e-849f-37ca5b7204a9","oAuthEventcV":"ep+jvnMUEywAMLY2BaKRxw.1.1","errorUrl":"https://aka.ms/autherrors#error-InvalidResource","requestId":"7f325e95-d7f8-4ef1-81d2-db9e1577f602","date":"2024-08-20T20:28:19"}}}

Debug middleware reveals an Authorization header is being sent, which is likely the cause. From looking at the .NET implementation, the auth header is specifically omitted.

https://github.com/microsoftgraph/msgraph-sdk-dotnet-core/blob/650189bf223447571e9ce5925d1d3abe7f25d6ef/src/Microsoft.Graph.Core/Tasks/LargeFileUploadTask.cs#L39

/// <param name="requestAdapter"><see cref="IRequestAdapter"/> to use for making upload requests. The client should not set Auth headers as upload urls do not need them.</param>

Expected behavior

File should upload.

How to reproduce

func UploadAttachmentToMessage(graphClient *graph.GraphServiceClient, largeFile string) {
	// <UploadAttachmentSnippet>
	// Create message
	message := models.NewMessage()
	subject := "Large attachment"
	message.SetSubject(&subject)

	savedDraft, _ := graphClient.Me().Messages().Post(context.Background(), message, nil)

	// Set up the attachment
	byteStream, _ := os.Open(largeFile)
	largeAttachment := models.NewAttachmentItem()
	attachmentType := models.FILE_ATTACHMENTTYPE
	largeAttachment.SetAttachmentType(&attachmentType)
	fileName := filepath.Base(largeFile)
	largeAttachment.SetName(&fileName)
	fileInfo, _ := byteStream.Stat()
	fileSize := fileInfo.Size()
	largeAttachment.SetSize(&fileSize)

	uploadSessionRequestBody := users.NewItemMessagesItemAttachmentsCreateUploadSessionPostRequestBody()
	uploadSessionRequestBody.SetAttachmentItem(largeAttachment)

	uploadSession, _ := graphClient.Me().
		Messages().
		ByMessageId(*savedDraft.GetId()).
		Attachments().
		CreateUploadSession().
		Post(context.Background(), uploadSessionRequestBody, nil)

	// Max slice size must be a multiple of 320 KiB
	maxSliceSize := int64(320 * 1024)
	fileUploadTask := fileuploader.NewLargeFileUploadTask[models.FileAttachmentable](
		graphClient.RequestAdapter,
		uploadSession,
		byteStream,
		maxSliceSize,
		models.CreateFileAttachmentFromDiscriminatorValue,
		nil)

	// Create a callback that is invoked after each slice is uploaded
	progress := func(progress int64, total int64) {
		fmt.Printf("Uploaded %d of %d bytes", progress, total)
	}

	// Upload the file
	uploadResult := fileUploadTask.Upload(progress)

	if uploadResult.GetUploadSucceeded() {
		fmt.Print("Upload complete")
	} else {
		fmt.Print("Upload failed.")
	}
	// </UploadAttachmentSnippet>
}

SDK Version

1.47.0

Latest version known to work for scenario above?

No response

Known Workarounds

No response

Debug output

Click to expand log ```
</details>


### Configuration

_No response_

### Other information

_No response_
@jasonjoh jasonjoh added status:waiting-for-triage An issue that is yet to be reviewed or assigned type:bug A broken experience labels Aug 20, 2024
@andrueastman
Copy link
Member

Thanks for raising this @jasonjoh

I believe the AzureIdentityAccessTokenProvider should already prevent setting the Auth header for non-graph urls.

https://github.com/microsoftgraph/msgraph-sdk-go/blob/cf048153191e603f83c8ae4f2bc60eb54daae03a/graph_service_client.go#L35

Any chance you can share how you are initializing the GraphServiceClient instance?

On a different note, this check should probably be checking for the allowed hosts collection and not the scopes.

@andrueastman andrueastman added status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close and removed status:waiting-for-triage An issue that is yet to be reviewed or assigned labels Aug 22, 2024
@jasonjoh
Copy link
Member Author

Interesting. I was using the AzureIdentityAuthenticationProvider from github.com/microsoft/kiota-authentication-azure-go, which I see doesn't set any valid hosts for the validator, resulting in all URLs being "valid".

I changed to use the AzureIdentityAuthenticationProvider from msgraph-sdk-go-core, and now I get this panic error:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1c9f2c3]

goroutine 1 [running]:
github.com/microsoftgraph/msgraph-sdk-go-core/fileuploader.(*uploadSlice[...]).Upload(0x20d6d20, 0x20821b0)
        /home/jasonjoh/go/pkg/mod/github.com/microsoftgraph/[email protected]/fileuploader/upload_slice.go:81 +0x403
github.com/microsoftgraph/msgraph-sdk-go-core/fileuploader.(*largeFileUploadTask[...]).uploadWithRetry(0x20ef560, {{0x0, 0x0}, {0xc000403c00, 0x63d}, 0x0, 0x4ffff, 0x3160b6, 0x50000, {0x20d8e68, ...}, ...}, ...)
        /home/jasonjoh/go/pkg/mod/github.com/microsoftgraph/[email protected]/fileuploader/large_file_upload_task.go:136 +0xd1
github.com/microsoftgraph/msgraph-sdk-go-core/fileuploader.(*largeFileUploadTask[...]).Upload(0x20ef560, 0x20821d0)
        /home/jasonjoh/go/pkg/mod/github.com/microsoftgraph/[email protected]/fileuploader/large_file_upload_task.go:60 +0x218
sdksnippets/snippets.UploadAttachmentToMessage(0xc000194570, {0xc0001a4190, 0x1b})
        /home/jasonjoh/repos/msgraph-snippets-go/src/snippets/large_file_upload.go:127 +0x64a
sdksnippets/snippets.RunUploadSamples(0xc000194570, {0xc0001a4190, 0x1b})
        /home/jasonjoh/repos/msgraph-snippets-go/src/snippets/large_file_upload.go:23 +0x25
main.main()
        /home/jasonjoh/repos/msgraph-snippets-go/src/main.go:66 +0xba8
package graphhelper

import (
	"context"
	"fmt"
	"log"
	"os"
	"strconv"
	"strings"

	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
	graphdebug "github.com/jasonjoh/msgraph-sdk-go-debug-logger"
	khttp "github.com/microsoft/kiota-http-go"
	graph "github.com/microsoftgraph/msgraph-sdk-go"
	graphcore "github.com/microsoftgraph/msgraph-sdk-go-core"
	auth "github.com/microsoftgraph/msgraph-sdk-go-core/authentication"
)

func NewUserGraphServiceClient(logger *log.Logger) (*graph.GraphServiceClient, error) {
	clientId := os.Getenv("CLIENT_ID")
	tenantId := os.Getenv("TENANT_ID")
	scopes := strings.Split(os.Getenv("GRAPH_USER_SCOPES"), ",")
	debug, err := strconv.ParseBool(os.Getenv("ENABLE_GRAPH_LOG"))
	if err != nil {
		debug = false
	}

	credential, err := azidentity.NewDeviceCodeCredential(&azidentity.DeviceCodeCredentialOptions{
		ClientID: clientId,
		TenantID: tenantId,
		UserPrompt: func(ctx context.Context, message azidentity.DeviceCodeMessage) error {
			fmt.Println(message.Message)
			return nil
		},
	})
	if err != nil {
		return nil, err
	}

	authProvider, err := auth.NewAzureIdentityAuthenticationProviderWithScopes(credential, scopes)
	if err != nil {
		return nil, err
	}

	if debug {
		return NewDebugGraphServiceClient(authProvider, logger)
	} else {
		adapter, err := graph.NewGraphRequestAdapter(authProvider)
		if err != nil {
			return nil, err
		}

		client := graph.NewGraphServiceClient(adapter)
		return client, nil
	}
}

func NewDebugGraphServiceClient(authProvider *auth.AzureIdentityAuthenticationProvider, logger *log.Logger) (*graph.GraphServiceClient, error) {
	showTokens, err := strconv.ParseBool(os.Getenv("GRAPH_LOG_TOKENS"))
	if err != nil {
		showTokens = false
	}
	showPayloads, err := strconv.ParseBool(os.Getenv("GRAPH_LOG_PAYLOADS"))
	if err != nil {
		showPayloads = false
	}

	clientOptions := graph.GetDefaultClientOptions()
	middleware := graphcore.GetDefaultMiddlewaresWithOptions(&clientOptions)
	debugMiddleware := graphdebug.NewGraphDebugLogMiddleware(logger, showTokens, showPayloads)
	allMiddleware := append(middleware, debugMiddleware)
	httpClient := khttp.GetDefaultClient(allMiddleware...)

	adapter, err := graph.NewGraphRequestAdapterWithParseNodeFactoryAndSerializationWriterFactoryAndHttpClient(
		authProvider, nil, nil, httpClient)
	if err != nil {
		return nil, err
	}

	client := graph.NewGraphServiceClient(adapter)
	return client, nil
}

@microsoft-github-policy-service microsoft-github-policy-service bot added needs attention 👋 and removed status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close labels Aug 22, 2024
@jasonjoh
Copy link
Member Author

FYI I get the same panic if I switch back to using the auth provider from Kiota abstractions and pass in an array of valid hosts, like so:

authProvider, err := auth.NewAzureIdentityAuthenticationProviderWithScopesAndValidHosts(credential, scopes, []string{"graph.microsoft.com"})

@andrueastman
Copy link
Member

andrueastman commented Aug 26, 2024

@jasonjoh Any chance you can share a sample of the go.mod file as well?

Are you able to get the same error if the authentication lib and http package are the latest? I'm seeing a different issue on my end. Authored #321 for this.

@andrueastman andrueastman added status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close and removed needs attention 👋 labels Aug 26, 2024
@andrueastman andrueastman self-assigned this Aug 26, 2024
@jasonjoh
Copy link
Member Author

As far as I can tell I have the latest versions.

module sdksnippets

go 1.21

toolchain go1.21.5

require (
	github.com/Azure/azure-sdk-for-go/sdk/azcore v1.14.0
	github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0
	github.com/jasonjoh/msgraph-sdk-go-debug-logger v0.0.2
	github.com/joho/godotenv v1.5.1
	github.com/microsoft/kiota-abstractions-go v1.6.1
	github.com/microsoft/kiota-authentication-azure-go v1.1.0
	github.com/microsoft/kiota-http-go v1.4.4
	github.com/microsoftgraph/msgraph-sdk-go v1.47.0
	github.com/microsoftgraph/msgraph-sdk-go-core v1.2.1
	github.com/thlib/go-timezone-local v0.0.3
)

require (
	github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 // indirect
	github.com/AzureAD/microsoft-authentication-library-for-go v1.2.2 // indirect
	github.com/cjlapao/common-go v0.0.40 // indirect
	github.com/davecgh/go-spew v1.1.1 // indirect
	github.com/go-logr/logr v1.4.2 // indirect
	github.com/go-logr/stdr v1.2.2 // indirect
	github.com/golang-jwt/jwt/v5 v5.2.1 // indirect
	github.com/google/uuid v1.6.0 // indirect
	github.com/kylelemons/godebug v1.1.0 // indirect
	github.com/microsoft/kiota-serialization-form-go v1.0.0 // indirect
	github.com/microsoft/kiota-serialization-json-go v1.0.8 // indirect
	github.com/microsoft/kiota-serialization-multipart-go v1.0.0 // indirect
	github.com/microsoft/kiota-serialization-text-go v1.0.0 // indirect
	github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect
	github.com/pmezard/go-difflib v1.0.0 // indirect
	github.com/std-uritemplate/std-uritemplate/go v1.0.5 // indirect
	github.com/stretchr/testify v1.9.0 // indirect
	go.opentelemetry.io/otel v1.29.0 // indirect
	go.opentelemetry.io/otel/metric v1.29.0 // indirect
	go.opentelemetry.io/otel/trace v1.29.0 // indirect
	golang.org/x/crypto v0.26.0 // indirect
	golang.org/x/net v0.28.0 // indirect
	golang.org/x/sys v0.24.0 // indirect
	golang.org/x/text v0.17.0 // indirect
	gopkg.in/yaml.v3 v3.0.1 // indirect
)

@microsoft-github-policy-service microsoft-github-policy-service bot added needs attention 👋 and removed status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close labels Aug 26, 2024
@andrueastman
Copy link
Member

Just to confirm here, do you get the same error with the latest update after #321

@andrueastman andrueastman added status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close and removed needs attention 👋 labels Aug 26, 2024
@jasonjoh
Copy link
Member Author

That fixed it! Thanks for checking in.

@microsoft-github-policy-service microsoft-github-policy-service bot added needs attention 👋 and removed status:waiting-for-author-feedback Issue that we've responded but needs author feedback to close labels Aug 26, 2024
@andrueastman
Copy link
Member

Thanks for confirming here. Closing this one for now..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants