open webbrowser with url for the user

[mike-kfed]

googles python api has that as an option. very convenient as the auth URL doesn't have to be copied from terminal, especially with the installed-app-flow no copy/paste needed, ever.

this is a quickhack showing it works in rust too. I guess you might want to make this optional somehow?
Please let me know how to integrate this for your library.

[ggriffiniii]

This is @dermesser's decision, but I wanted to chime in with my $0.02. I personally don't like it when applications open urls in my browser. It's very common that I have multiple browser profiles open and the url get's opened in a random one that is usually not the one I want. InstalledFlowDelegate is already a trait that users can extend with whatever functionality they want. The default implementation should really just be considered an example of bare minimum functionality in my opinion.

[mike-kfed]

makes sense to make that configureable, googles python api has the display url only and open url in browser thing separately too. I didn't know the InstalledFlowDelegate trait existed (used the InstalledFlowAuthenticator::builder where this is hidden). So a non-quick hack would be adding a new Flow called e.g. InstalledFlowBrowserAuthenticator that does the browser opening. Or I add a project to examples/ how to implement a custom flow? what do you prefer?

[ggriffiniii]

IMO, it absolutely makes sense to have something in examples/ that demonstrates how to do this. The question would be whether that example creates it's own local trait that people could then copy+paste as a starting point when they want this functionality, or if the yup_oauth2 crate should provide a second implementation of InstalledFlowAuthenticator that people could specify without writing their own.

I don't have strong feelings either way. The one technical difference is that the latter option would require bringing webbrowser as a dependency for every user of yup_oauth2 regardless of whether they require it (unless you go through the trouble of making it a cfg option). That would probably be the deciding factor for me and lean towards providing an implementation in an example that people could copy+paste+modify in their own code. Then webbrowser would only be a dev-dependency.

[mike-kfed]

I'll write the example, you're right dragging in the webbrowser dependency and making it more complicated with cfg options is probably less clean of a solution.

[dermesser]

hello @mike-kfed! Thank you for contributing :) As your proposed change already shows, it is very feasible to open the web browser from the delegate. The easiest way to make this convenient in a non-intrusive (for yup-oauth2) way would be:

• Show in an example how to effectively use the installed flow delegate
• make authenticator_delegate::present_user_url() pub so that consumers of our API can quickly implement the InstalledFlowDelegate trait by themselves, with their implementation opening a web browser first and then calling present_user_url() just as we do in the current default implementation.

This is a win-win, I'd say- for users, it's just a few additional lines of code, and for us it is just a pub in the right place.

Am I overlooking anything?

[mike-kfed]

alright here's my first attempt at this. cargo run --example custom_flow works for me.

I assume you want a clean git history, but I figured we can play in my current branch until documentation, filenames and so on have been agreed upon. After that I'll make a new PR with a clean history :)

[ggriffiniii]

I don't think present_user_url() needs to be made public. It's already effectively public via a different name DefaultInstalledFlowDelegate::present_user_url. I think the InstalledFlowBrowserDelegate should just create an instance of DefaultInstalledFlowDelegate and call present_user_url on that.

[mike-kfed]

good point, I've implemented it now like this

[dermesser]

LGTM

[mike-kfed]

made a clean pull request, see #139