Pluggable Storage

[djrodgerspryor]

I'm trying to migrate from v1 to the latest version. I have a custom storage backend for secure MacOS keychain storage written as an implementation of the old yup_oauth2::TokenStorage trait, but as far as I can tell, there's no way to do this in the new version, and none of the existing storage options suit my needs.

Would you be open to replacing the current Storage enum with a trait instead?

[dermesser]

That refactoring work was done by @ggriffiniii, a bit more than a year ago.

I definitely agree that more useful token storage is required and wonder why we moved away from the trait based model (which you mention and I remember).

So I have a few proposals that seem viable:

• Keep the enum and extend it with a third variant External(Box<ExternalStorage>) mimicking the old model, but keeping the enum (where I'm not aware of all the up- and downsides of doing it that way). It would also keep API compatibility.
• Replace the enum with the old TokenStorage trait. Breaks API, but seems most generally useful (and I remember that originally the idea was that mostly every serious consumer of this library would write their own storage).
• Or, keeping more or less with the current approach: You write a somewhat generic Mac OS keychain storage backend, pack it as a crate, and we add "Mac OS keychain support" as an optional feature. The enum will gain feature-guarded variants, and so will the impl. Not super elegant, but nice for a "curated ecosystem" of storage backends, not entirely unreasonable.

Please, you and @ggriffiniii, let me know what you think.

[ggriffiniii]

It's been a while, but I believe the reason for going with the enum rather than the trait was because returning futures via a trait requires a heap allocation and we could avoid that with the enum. I did a quick (non-exhaustive) look at many of the public consumers of yup-oauth2 and none were using a custom trait impl so I proposed removing it until a use case arose.

Sounds like we have such a use case, and my vote is for Option 1 creating another variant of the enum for custom instances of a storage trait. Seems like the best of both worlds by providing the flexibility of a custom implementation while the built-in implementation don't pay the cost of a heap allocation on every call.

[djrodgerspryor]

Thanks for the quick responses!

Option 3 will be difficult to adapt for my use case (the custom storage provider actually combines the MacOS keychain storage with some encryption using a key unlocked by 2-factor authentication, it's pretty specific to our use case and not suitable as a generic storage backend).

I've been playing around with a prototype of option 2, but I'll stash that and have a go at option 1 later today so that we can see how it looks.

[djrodgerspryor]

I've got a working version of option 1 working here: #146

Using the new interface to implement our custom storage type was pretty painless.

Let me know what you think about the approach!

[chrisabruce]

I too need this!

[chrisabruce]

Any movement on this?