Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use decorators to implement credential caching and logging #10316

Closed
coryan opened this issue Nov 28, 2022 · 0 comments · Fixed by #10412
Closed

Use decorators to implement credential caching and logging #10316

coryan opened this issue Nov 28, 2022 · 0 comments · Fixed by #10412
Assignees
Labels
type: cleanup An internal cleanup or hygiene concern.

Comments

@coryan
Copy link
Contributor

coryan commented Nov 28, 2022

As I work in #5915 it is clear that I will need good logging for credentials (see #3429) and I don't want to repeat the code to cache credentials either.

Using a decorator seems like a better solution, but we need to change some signatures (these are Okay, they are in *_internal namespaces).

Overview

The g::c::oauth2_credentials::Credentials class will change from:

class google::cloud::oauth2_internal::Credentials {
 public:
   virtual StatusOr<std::pair<std::string, std::string>> AuthorizationHeader();
};

to:

// Already existing ... just a reminder for this bug
struct google::cloud::internal::AccessToken {
  std::string token; // empty for anonymous credentials
  std::chrono::system_clock::time_point expiration;
};

class google::cloud::oauth2_internal::Credentials {
 public:
   virtual StatusOr<internal::AccessToken> GetToken(std::chrono::system_clock::time_point now) = 0;
};

Returning an access token makes it possible to log the returned token (truncated for security reasons) and its expiration time. Passing in the current time makes it easier to test the implementation.

Work Breakdown

I am planning to

  1. Introduce the GetToken() function and a caching decorator
  2. Change the existing functions returning g::c::oauth2_internal::RefreshingCredentialsWrapper::TemporaryToken to return internal::AccessToken
  3. Change the existing classes, remove AuthorizationHeader() and the custom caching loops
    • This will be a large CL
  4. Remove g::c::oauth2_internal::RefreshingCredentialsWrapper
  5. Add a logging decorator for credentials, enabled only if some tracing component is present (auth?)

Future Work

There may be some opportunities to add a type field to AccessToken. Normally this would be Bearer, but could be None or some other magical string to indicate the credentials are anonymous.

We may consider a different tracing component to enable low-level (HTTP headers and traffic) for auth requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant