Fail revoke() calls for old (deserialized) accounts that don't have a URL for it
#45
Conversation
| pub(crate) revoke_cert: String, | ||
| // The fields below were added later and old `AccountCredentials` may not have it. | ||
| // Newer deserialized account credentials grab a fresh set of `DirectoryUrls` on | ||
| // deserialization, so they should be fine. Newer fields should be optional, too. | ||
| pub(crate) new_authz: Option<String>, | ||
| pub(crate) revoke_cert: Option<String>, | ||
| pub(crate) key_change: Option<String>, |
There was a problem hiding this comment.
You may want to update the deserialize_old_credentials (add these fields?), and maybe deserialize_new_credentials unit tests accordingly.
There was a problem hiding this comment.
I actually don't want to update old_credentials, because that reflects the old usage that I'm trying to still support to the extent possible. From that perspective new_credentials is testing the "new old" behavior, so probably also want to keep it as is. Can't think of another one for which adding it would have much value?
| // The fields below were added later and old `AccountCredentials` may not have it. | ||
| // Newer deserialized account credentials grab a fresh set of `DirectoryUrls` on | ||
| // deserialization, so they should be fine. Newer fields should be optional, too. | ||
| pub(crate) new_authz: Option<String>, | ||
| pub(crate) revoke_cert: Option<String>, | ||
| pub(crate) key_change: Option<String>, |
There was a problem hiding this comment.
I'm also seeing renewalInfo and meta fields at https://acme-staging-v02.api.letsencrypt.org/directory, do those matter?
There was a problem hiding this comment.
I think meta doesn't matter for this library's purpose for now. renewalInfo is still in draft apparently so I'd prefer to avoid it for now.
No description provided.