feat: Adds GoogleCredential.FromJsonParameters #2037
Conversation
There was a problem hiding this comment.
For using with user secrets, you can do something like:
JsonCredentialParameters credentialParams = Configuration.GetSection("GoogleCredential").Get<JsonCredentialParameters>();
GoogleCredential credential = GoogleCredential.FromJsonParameters(credentialParams);The method above that extracts the config will attempt to match the properties by name (or binder options may be provided). The drawback here is that the name of the properties in the credential JSON file (the one from Google) do not match the property names in JsonCredentialParameters. But storing the credential parameters in the configuration is supposed to be a one time thing, so this shouldn' be very bad.
amanda-tarafa
force-pushed
the
credentials-init
branch
from
542dcbe to
b7cfb24
Compare
|
Jon this is now ready for review. |
|
|
||
| private IConfiguration BuildConfigurationForCredential(string json) => | ||
| // We parse the string and call ToString after because within the key | ||
| // there's character that need scaping. Newtonsoft seems to handle them |
There was a problem hiding this comment.
| // there's character that need scaping. Newtonsoft seems to handle them | |
| // there's a character that need escaping. Json.NET seems to handle them |
| Assert.Equal("CLIENT_EMAIL", serviceCred.Id); | ||
| Assert.Equal("PROJECT_ID", serviceCred.ProjectId); | ||
| } | ||
|
|
There was a problem hiding this comment.
Add tests for a) invalid JSON; b) valid JSON that doesn't contain everything we expect; c) valid JSON that doesn't conform with the types in our class? (I'd expect these to be pretty simple, but they'd document the expected behavior.)
| private static GoogleCredential CreateDefaultCredentialFromParameters(JsonCredentialParameters credentialParameters) | ||
| { | ||
| switch (credentialParameters.Type) | ||
| internal GoogleCredential CreateDefaultCredentialFromParameters(JsonCredentialParameters credentialParameters) => |
There was a problem hiding this comment.
Add a nullity check? I suspect we previously "knew" that it would never be null, but we don't any more due to this effectively being exposed publicly.
amanda-tarafa
force-pushed
the
credentials-init
branch
from
b7cfb24 to
fa0c63c
Compare
|
|
||
| private IConfiguration BuildConfigurationForCredential(string json) => | ||
| // We parse the string and call ToString after because within the key | ||
| // there's character that need scaping. Newtonsoft seems to handle them |
| private static GoogleCredential CreateDefaultCredentialFromParameters(JsonCredentialParameters credentialParameters) | ||
| { | ||
| switch (credentialParameters.Type) | ||
| internal GoogleCredential CreateDefaultCredentialFromParameters(JsonCredentialParameters credentialParameters) => |
| Assert.Equal("CLIENT_EMAIL", serviceCred.Id); | ||
| Assert.Equal("PROJECT_ID", serviceCred.ProjectId); | ||
| } | ||
|
|
| }).Build(); | ||
|
|
||
| JsonCredentialParameters credentialParameters = config.GetSection("GoogleCredential").Get<JsonCredentialParameters>(); | ||
| ArgumentNullException ex = Assert.Throws<ArgumentNullException>(() => GoogleCredential.FromJsonParameters(credentialParameters)); |
There was a problem hiding this comment.
Interesting - ArgumentNullException here is unexpected for me, but that's why it's good to have the test :)
There was a problem hiding this comment.
Yep, it was unexpected for me as well. Will rebase and squash soon.
amanda-tarafa
force-pushed
the
credentials-init
branch
from
fa0c63c to
04845ae
Compare
Bug fixes: - googleapis#1991. Disables self signed JWTs for domain-wide delegation. - googleapis#2023. Observes execeptions in refresh token tasks. Features: - googleapis#2037. Adds GoogleCredential.FromJsonParameters. - googleapis#2046. Improves error messages for JWT validation. Docs: - googleapis#2044. Adds links to OAuth2 docs in FAQs.
Closes #2036