Repository navigation
Accept a bearer token for azureblob - #1212
Benmartin92 wants to merge 1 commit into
Conversation
gaul
left a comment
There was a problem hiding this comment.
Thanks for this. The production change is small and leaves the SAS and account-key paths alone, and copies correctly fall through to a user delegation key in signedSourceUrl. A few things before merging, in rough priority:
- The new test uses 2-space indentation, but checkstyle's
Indentationmodule runs on test sources withfailOnViolation=true. I expect this failsmvn verify. Please reformat to the 4-space style of the siblingAzureSasCredentialTest. - The test swaps the JVM-wide
SSLContextdefault for a trust-all one (inline comment below). - The placeholder
Credentialssteers both credentialifchains implicitly (inline comment below).
A design question too. Since #1161, expiring backend credentials flow through Supplier<Credentials> and Credentials.sessionToken(), re-read on each request. GCloudBlobStore already carries a Google OAuth bearer token that way. A separate TokenCredential constructor serves only applications that embed S3Proxy. Someone configuring through a properties file, who could set jclouds.azureblob.auth=azureAd in 3.x, still has no path. On the other hand, TokenCredential is the idiomatic Azure type and carries an expiry that Credentials cannot. I'd like to weigh this before merging: is there a reason not to select bearer-token mode for the supplier's token with a property?
88b309f to
d0e04c1
Compare
|
Hey @gaul, thanks for reviewing!
Regarding the design: fair point, I had only my own embedding use case in mind. I changed it to go through the supplier like GCP does and to work from a properties file. |
|
Thanks, this addresses everything: the enum makes both branches explicit, the injected One correction, and it's partly my fault for the framing in my first review. In jclouds 2.7.0, Smaller things:
|
Before 4.0.0, S3Proxy reached Microsoft Entra through jclouds. Its azureblob provider's jclouds.azureblob.auth=azureAd took a client id and secret by default and fetched and renewed tokens itself, which DefaultAzureCredential now does from AZURE_TENANT_ID, AZURE_CLIENT_ID and AZURE_CLIENT_SECRET. Overriding jclouds.oauth.credential-type to the bearer-token type let the application supply the access token itself instead. Applications that obtain their tokens from a sidecar or a token broker relied on that override. The Azure SDK store that replaced jclouds accepts only a shared access signature, an account key, or DefaultAzureCredential. None of those fits a token the application already holds: it is neither a signature nor a key, and DefaultAzureCredential looks only at the environment, the workload identity token file, managed identity and the developer tools. Those applications cannot upgrade past 3.x. Let s3proxy.azureblob.auth name how the store authenticates: sas, key, default, or the new bearer, which says that the session token is such a token. Left unset, the store infers sas, key or default from the parts of the credential that are present, as it did before; set, the mode is checked against the credential at construction rather than falling through to another on an empty value. Under bearer the store hands the token to the client as a bearer token, read from the credential supplier for every request as a signature already is, and as GCloudBlobStore does with a Google OAuth token: nothing in a Credentials says how long the token lasts, so each is handed over already lapsed rather than kept past its withdrawal. The endpoint must be https, since the client refuses to send a bearer token over http. Server-side copies sign their source URL with a user delegation key, as under DefaultAzureCredential, so the token's principal needs a role that includes generateUserDelegationKey. A copy under sas now signs its source with the signature the client holds, the last one the supplier named, rather than failing on a supplier that is briefly empty, which SasRefreshPolicy already tolerates. The test hands the store an HttpClient that records each request and refuses it, since the client sends bearer tokens only to an https endpoint and only what was presented is in question. It checks that the token arrives as a bearer token, that one the supplier replaces gives way to its successor, that a copy asks for a user delegation key under the same token, that an unknown mode is refused, and that a named mode must find its credential. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
d0e04c1 to
c416f56
Compare
|
Thanks for the review, I added required changes. |
Problem
Before 4.0.0, S3Proxy reached Microsoft Entra through jclouds. By default
jclouds.azureblob.auth=azureAdtook a client id and secret injclouds.identityandjclouds.credentialand fetched and renewed tokens itself. Overridingjclouds.oauth.credential-typeto the bearer-token type let the application supply the access token itself instead. Applications that get their tokens from a sidecar or a token broker relied on that override.The Azure SDK store that replaced jclouds in 4.0.0 accepts only a shared access signature, an account key, or
DefaultAzureCredential. The first setup maps ontoDefaultAzureCredentialwithAZURE_TENANT_ID,AZURE_CLIENT_IDandAZURE_CLIENT_SECRET, which renews tokens as jclouds did. The second has no equivalent: a token the application already holds is neither a signature nor a key, andDefaultAzureCredentiallooks only at the environment, the workload identity token file, managed identity and the developer tools. Those applications cannot upgrade past 3.x.Change
s3proxy.azureblob.authnames how the store authenticates:sas,key,default, or the newbearer, which says thatjclouds.session-token(or the supplier's session token) is a Microsoft Entra access token rather than a shared access signature. Left unset, the store inferssas,keyordefaultfrom the parts of the credential that are present, exactly as today, so no existing configuration changes. Set, the mode is checked against the credential at construction instead of falling through to another mode on an empty value.bearerthe store wraps the supplier in aTokenCredentialthe same wayGCloudBlobStorewraps it for a Google OAuth token: nothing in aCredentialssays how long the token lasts, so each is handed over already lapsed and the supplier is asked again for every request. The endpoint must be https, since azure-core refuses to send a bearer token over http.Authenum field, and both the constructor andsignedSourceUrlbranch on it explicitly.DefaultAzureCredential. The javadoc and README say that the token's principal needs a role that includesgenerateUserDelegationKey, such as Storage Blob Delegator, or copies fail while reads and writes succeed.sasnow signs its source with the signature the client holds, whichSasRefreshPolicykeeps at the last non-empty value the supplier returned, instead of re-reading the supplier and failing if it is briefly empty.HttpClient, for tests.jclouds.azureblob.authis not read any more; the client-id-and-secret setup moves todefaultwith theAZURE_*variables, and the raw-token setup tojclouds.session-tokenpluss3proxy.azureblob.auth=bearer.Tests
AzureBearerTokenTesthands the store anHttpClientthat records each request and answers 403, so there is no socket, no TLS and no global state. It checks that:Bearer <token>;comp=userdelegationkey, carrying the bearer token;authvalue is refused;bearerwithout a session token andkeywithout an account name and key are refused.mvn verifyand the fullmvn testsuite pass.🤖 Generated with Claude Code