feat: key not from file & other improvements #29
Conversation
chris13524
force-pushed
the
feat/key-not-from-file
branch
from
d3ed7c9
to
c0a45bc
Compare
chris13524
changed the title
There was a problem hiding this comment.
Hey @chris13524. Thanks for the PR. I managed to spot a few minor things, hope this is not too much of a hustle to do the changes. Also would be great to bump up the version in Cargo to 0.9, since there is a breaking change to access_token return type.
| HttpRequest(reqwest::Error), | ||
|
|
||
| #[error("failed to send request")] | ||
| HttpRequestUnsuccessful(StatusCode, std::result::Result<String, reqwest::Error>), |
There was a problem hiding this comment.
std::result::Result could be shorted to StdResult, since there is already an import on line 3
src/serv_account/errors.rs
Outdated
| HttpJson(reqwest::Error), | ||
|
|
||
| #[error("response returned non-Bearer auth access token: {0}")] | ||
| AccessTokenNotBeaarer(String), |
There was a problem hiding this comment.
Looks like there is a typo in Beaarer
| } | ||
|
|
||
| // Account for clock skew or time to receive or process the response | ||
| const LEEWAY: Duration = Duration::seconds(30); |
There was a problem hiding this comment.
This piece seems not to compile for me. Here is the error message that I am get
error[E0015]: cannot call non-const fn `chrono::Duration::seconds` in constants
--> src/serv_account/mod.rs:108:34
|
108 | const LEEWAY: Duration = Duration::seconds(30);
| ^^^^^^^^^^^^^^^^^^^^^
|
= note: calls in constants are limited to constant functions, tuple structs and tuple variants
```
|
|
||
| Ok(token) | ||
| let expires_at = Utc::now() + Duration::seconds(json.expires_in) - LEEWAY; |
There was a problem hiding this comment.
One of the options to resolve the issue with Duration above would be to keep it simple and use ints:
Utc::now().timestamp() as u64 + json.expires_in - 30
|
Thanks for the review! I'm working on some fixes for the above as well as some other tweaks |
This adds support for constructing a
ServiceAccountdirectly from aServiceAccountKey, rather than from a file. It also:RwLockto enable cloningServiceAccountwhile re-using the current access token across threads/instances. Also allowsServiceAccountto be used without needing to bemut.ServiceAccountKeyand its fields public to allow parsing or constructing the struct from any source (e.g. string, bytes, etc.) and also allows the fields inside to be re-used by other modules (e.g. readingproject_idnecessary for FCM)ServiceAccountBuilderto enable the use of a variety of constructors, including the ability to re-usereqwest::Clientacross instantiations which enables sharing connection pools. Existingfrom_file()anduser_email()methods continue to work for backwards-compatibility, but the builder pattern is generally a better alternativeaccess_token()now returnsAccessTokeninstead of e.g."Bearer token"which allows the use of reqwest's.bearer_token()method without needing to parse-out the token. Also enables the invoker to see when the token will expire.from_file()retained as a helper method and now returns aResulttype since file reading and parsing is performed in this function, rather than lazily inaccess_token(). Also now accepts any path-compatible value, rather than just&str.RsaKeyPairinJwtTokenrather than inputs to improve efficiencyResolves #28