Azure / Azure/azure-sdk-for-cpp
PutTokenForAudience raises AuthenticationException for every non-Ok CBS result
- 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
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