feat(auth): allow consuming access tokens directly by alvarowolfx · Pull Request #3642 · googleapis/google-cloud-rust

alvarowolfx · 2025-10-29T16:07:49Z

alvarowolfx · 2025-10-29T16:10:44Z

This is a WIP and is missing all documentation for the new interface.

Pros:

AccessTokenCredentials is compatible with Credentials type (via automatic conversion to CredentialProvider -> Credentials)
Reuse all current Builders and can selective pick which Credentials types would support that (we don't want API Key creds to support this)
Cons:
I don't like the build_access_token_credentials() method name. Too long and not sure if is a good pattern to add another final branch to a Builder.
Introduces new AccessTokenCredentials which is almost the same as Credentials. Perhaps it can be simplified ?
AccessTokenCredentialsProvider looks really similar to TokenProvider, can this be reused ?
- Probably not, Token is private and we need a new AccessToken type to expose that.

codecov · 2025-10-29T16:15:39Z

Codecov Report

❌ Patch coverage is 98.18182% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 95.03%. Comparing base (3388d78) to head (67e9d89).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
src/auth/src/credentials.rs	95.45%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3642   +/-   ##
=======================================
  Coverage   95.02%   95.03%           
=======================================
  Files         161      161           
  Lines        6049     6097   +48     
=======================================
+ Hits         5748     5794   +46     
- Misses        301      303    +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

coryan · 2025-10-29T16:25:41Z

I don't know what is the exact level of test coverage we want, but 46% is too low.

alvarowolfx · 2025-10-29T16:26:23Z

I don't know what is the exact level of test coverage we want, but 46% is too low.

this is also missing tests, is just a draft for the public API.

dbolduc

FWIW, this approach makes a lot of sense to me. It is lamentable that we are already making backwards compatibility sacrifices in version 1.1.0 of a library, but that is where we are.

I think I messed up by making Credentials an opaque object instead of a trait. I think at the time I was afraid of all the generics that entailed... but now I realize that is just what flexible Rust looks like.

src/auth/src/credentials.rs

tests/integration-auth/src/lib.rs

src/auth/src/credentials.rs

…ss_token

coryan · 2025-11-20T22:26:06Z

Marking as draft. If / when it is ready for review please let us know.

alvarowolfx · 2025-11-21T13:40:46Z

Marking as draft. If / when it is ready for review please let us know.

@coryan it was ready to review, but I think I'll split into smaller PRs to make it easier to review. Unless you think this is in a reasonable size.

coryan · 2025-11-21T13:56:23Z

@dbolduc can you take another look?

dbolduc

The code looks really good, and I would symbolically approve the PR. I think my comments are mostly grammar nits.

I need an explicit answer on why we are abandoning Extensions / CacheableResource for the AccessToken before I can approve though.

dbolduc · 2025-11-21T21:01:59Z

src/auth/src/credentials.rs

+        Ok(self.build_access_token_credentials()?.into())
+    }
+
+    /// Returns a [Credentials] instance with the configured settings.


nit: s/Credentials/AccessTokenCredentials/

dbolduc · 2025-11-21T21:03:19Z

src/auth/src/credentials.rs

+    ///
+    /// # Errors
+    ///
+    /// Returns a [CredentialsError] if a unsupported credential type is provided


nit: s/a unsupported/an unsupported/

dbolduc · 2025-11-21T21:04:30Z

src/auth/src/credentials.rs

+    /// Returns a [CredentialsError] if a unsupported credential type is provided
+    /// or if the JSON value is either malformed
+    /// or missing required fields. For more information, on how to generate
+    /// json, consult the relevant section in the [application-default credentials] guide.


nit: s/json/the JSON for a credential/ ?

dbolduc · 2025-11-21T21:05:33Z

src/auth/src/credentials/external_account.rs

+        Ok(self.build_access_token_credentials()?.into())
+    }
+
+    /// Returns a [AccessTokenCredentials] instance with the configured settings.


nit: s/Returns a/Returns an/

dbolduc · 2025-11-21T21:07:33Z

src/auth/src/credentials/external_account.rs

+    /// Returns a [BuilderError] if the `external_account_config`
+    /// provided to [`Builder::new`] cannot be successfully deserialized into the
+    /// expected format for an external account configuration. This typically happens if the
+    /// JSON value is malformed or missing required fields. For more information,
+    /// on the expected format, consult the relevant section in the
+    /// [external_account_credentials] guide.


nit: reflow?

we do not have any sort of tool. I just manully cut lines off at or before 80 characters. 🤷

Also consider a paragraph break before "For more information..."

dbolduc · 2025-11-21T21:07:51Z

src/auth/src/credentials.rs

+    ///
+    /// Returns a [CredentialsError] if a unsupported credential type is provided
+    /// or if the JSON value is either malformed
+    /// or missing required fields. For more information, on how to generate


optional nit: paragraph break before "For more information..."

src/auth/src/credentials/external_account.rs

src/auth/src/credentials.rs

alvarowolfx · 2025-11-25T17:07:48Z

The code looks really good, and I would symbolically approve the PR. I think my comments are mostly grammar nits.

I need an explicit answer on why we are abandoning Extensions / CacheableResource for the AccessToken before I can approve though.

pushed some updates addressing the nits in the docs and also made some improvements so customers can mock the new trait

feat(auth): allow consuming access tokens directly

2d73fcb

dbolduc reviewed Oct 30, 2025

View reviewed changes

src/auth/src/credentials.rs Show resolved Hide resolved

tests/integration-auth/src/lib.rs Show resolved Hide resolved

src/auth/src/credentials.rs Show resolved Hide resolved

src/auth/src/credentials.rs Show resolved Hide resolved

alvarowolfx added 5 commits November 11, 2025 01:51

docs: add docs and test to access token credentials

d2c1def

Merge branch 'main' into feat-auth-acess-token-direct

f717475

docs: add comments about compatibility with Credentials

2bbfe31

fix: lint issues

44fdb14

fix: fmt all the things

6f3dd99

alvarowolfx marked this pull request as ready for review November 11, 2025 19:49

alvarowolfx requested review from a team November 11, 2025 19:49

alvarowolfx marked this pull request as draft November 11, 2025 20:10

alvarowolfx added 3 commits November 17, 2025 16:39

refactor: change token method name and remove unused fields from acce…

180ddf4

…ss_token

Merge branch 'main' into feat-auth-acess-token-direct

907514d

fix: remove todo from integration tests

065040a

alvarowolfx marked this pull request as ready for review November 17, 2025 16:43

alvarowolfx added 3 commits November 17, 2025 16:58

fix: hide token string from debug

cca14b1

test: censored access token

d1473cb

test: add tests using access token creds

8a98c06

coryan marked this pull request as draft November 20, 2025 22:26

coryan marked this pull request as ready for review November 21, 2025 13:56

dbolduc reviewed Nov 21, 2025

View reviewed changes

fix: address review comments

ebe2023

alvarowolfx added 2 commits November 25, 2025 17:07

Merge branch 'main' into feat-auth-acess-token-direct

c0a2ecf

fix: add import for tokio Instant

67e9d89

dbolduc approved these changes Nov 25, 2025

View reviewed changes

alvarowolfx merged commit 37aef82 into googleapis:main Nov 25, 2025
29 checks passed

alvarowolfx mentioned this pull request Dec 10, 2025

Add simpler credentials header accessor #3362

Closed

Conversation

alvarowolfx commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alvarowolfx commented Oct 29, 2025

Uh oh!

codecov bot commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coryan commented Oct 29, 2025

Uh oh!

alvarowolfx commented Oct 29, 2025

Uh oh!

dbolduc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coryan commented Nov 20, 2025

Uh oh!

alvarowolfx commented Nov 21, 2025

Uh oh!

coryan commented Nov 21, 2025

Uh oh!

dbolduc left a comment

Choose a reason for hiding this comment

Uh oh!

dbolduc Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

dbolduc Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

dbolduc Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

dbolduc Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

dbolduc Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

dbolduc Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

alvarowolfx commented Nov 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alvarowolfx commented Oct 29, 2025 •

edited

Loading

codecov bot commented Oct 29, 2025 •

edited

Loading