search

microsoftgraph / msgraph-sdk-go-core

Microsoft Graph SDK for Go - Core Library
https://learn.microsoft.com/graph/sdks/sdks-overview
MIT License
19 stars 9 forks source link

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

Closed jasonjoh closed 2 months ago

jasonjoh commented 2 months ago

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 ``` ```

Configuration

No response

Other information

No response

andrueastman commented 2 months ago

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.

https://github.com/microsoftgraph/msgraph-sdk-go-core/blob/202886d3f524fe6064d8b60a962327897eae66de/authentication/azure_identity_access_token_provider.go#L34

jasonjoh commented 2 months ago

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/msgraph-sdk-go-core@v1.2.0/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/msgraph-sdk-go-core@v1.2.0/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/msgraph-sdk-go-core@v1.2.0/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
}
jasonjoh commented 2 months ago

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 commented 2 months ago

@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 https://github.com/microsoftgraph/msgraph-sdk-go-core/pull/321 for this.

jasonjoh commented 2 months ago

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
)
andrueastman commented 2 months ago

Just to confirm here, do you get the same error with the latest update after https://github.com/microsoftgraph/msgraph-sdk-go-core/pull/321

jasonjoh commented 2 months ago

That fixed it! Thanks for checking in.

andrueastman commented 2 months ago

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