Azure / Azure/azure-sdk-for-cpp

PutTokenForAudience raises AuthenticationException for every non-Ok CBS result

Open
#7,330 0 comments 0 reactions 0 assignees View on GitHub
needs-triage
Dominant language
C++
Stars
205
Forks
172
Avg merge
1d 3h
Merged PRs (30d)
37

Description

## Summary

`PutTokenForAudience` raises `Azure::Core::Credentials::AuthenticationException` for every result of a CBS put-token operation that is not `Ok` (`sdk/core/azure-core-amqp/src/amqp/connection.cpp:160-165`). The claims based security layer maps a service refusal, a transport fault, and a cancelled context to different values of one enum, and all of them reach that single throw. A caller cannot tell the three cases apart. The link rebuild work in #7254 needs that distinction.

## Motivation

`ClaimsBasedSecurityImpl::PutToken` maps `ManagementOperationStatus` to `CbsOperationResult` on the uAMQP transport (`sdk/core/azure-core-amqp/src/impl/uamqp/amqp/claim_based_security.cpp:95-115`). `FailedBadStatus` becomes `Failed`, which is a refusal that the service sent. `Error` and `InstanceClosed` come from transport events instead, for example a failed send of the put-token message or a detach of the `$cbs` link during the operation. `Cancelled` comes from a cancelled context. The test at `connection.cpp:160` compares against `Ok` alone, so each of those raises `AuthenticationException`.

This has two effects.

A retry cannot happen. `AuthenticationException` derives from `std::exception`, and not from `std::runtime_error` (`sdk/core/azure-core/inc/azure/core/credentials/credentials.hpp:139`). `RetryOperation::Execute` catches `EventHubsException`, `OperationCancelledException`, and `std::runtime_error` (`sdk/eventhubs/azure-messaging-eventhubs/src/retry_operation.cpp:61,74,78`). So a transport fault inside a put-token leaves the retry loop on the first attempt, and the caller reads an authentication failure for a network problem.

The Event Hubs producer cannot keep a healthy connection. `ProducerClient::EnsureSenderOrInvalidate` discards the cached session and connection when an attach fails. A first version of that method kept the stack for `AuthenticationException`, because a connection that carried a refusal still works. That exemption came out again, because the same type also covers a `$cbs` link that died, and keeping a sick connection makes each later call report a misleading authentication error. The cost of the removal is one connection rebuild after each credential failure.

The two transports also disagree. The Rust claims based security layer returns `Ok` or throws `std::runtime_error` (`sdk/core/azure-core-amqp/src/impl/rust_amqp/amqp/claim_based_security.cpp:102,105`), so a service refusal on that transport never raises `AuthenticationException`.

The .NET client keeps the distinction that this code loses. It maps `amqp:unauthorized-access` to `UnauthorizedAccessException` and treats that type as retryable (`BasicRetryPolicy.cs:163-166` in azure-sdk-for-net), and it turns a transport fault during the CBS call into a transient `EventHubsException` with the reason `ServiceCommunicationProblem` (`AmqpConnectionScope.cs:1127-1139`). Its connection survives an authorization denial, because CBS runs before the session exists.

## Proposal

Raise a type that matches the result in `PutTokenForAudience`.

- `CbsOperationResult::Failed` keeps `AuthenticationException`. The service answered, and it refused the claim, so the connection works.
- `CbsOperationResult::Error` and `CbsOperationResult::InstanceClosed` raise `std::runtime_error`. The transport failed, so a caller must retry, and the Event Hubs producer must discard the cached stack.
- `CbsOperationResult::Cancelled` raises `Azure::Core::OperationCancelledException`, so a cancelled context reports a cancellation.
- `CbsOperationResult::Invalid` raises `std::runtime_error`.

Make the Rust transport report a refusal the same way, so one caller works on both transports.

Restore the `AuthenticationException` exemption in `ProducerClient::EnsureSenderOrInvalidate` after that. A credential failure then keeps a healthy connection, which matches the .NET behaviour.

This changes an exception contract, so each caller of `AuthenticateAudience` needs a review. The callers are `MessageSenderImpl::Open`, `MessageReceiverImpl::Open`, and `ManagementClientImpl::Open`. Any test that expects `AuthenticationException` from a put-token failure needs a review as well.

Contributor guide

Open the contributing guide

Research direction

Start with PutTokenForAudience in sdk/core/azure-core-amqp/src/amqp/connection.cpp and the CbsOperationResult mappings in the uAMQP and Rust transport files. Review AuthenticateAudience callers in MessageSenderImpl::Open, MessageReceiverImpl::Open, and ManagementClientImpl::Open, along with tests expecting AuthenticationException. Done means each result has the proposed exception behavior, both transports agree, and ProducerClient::EnsureSenderOrInvalidate preserves healthy connections after credential refusal.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, rust
Domain
api, networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.