Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows Callback URI Scheme other than http://localhost #25

Closed
Mabenan opened this issue Nov 10, 2022 · 23 comments · Fixed by #92
Closed

Windows Callback URI Scheme other than http://localhost #25

Mabenan opened this issue Nov 10, 2022 · 23 comments · Fixed by #92
Assignees
Labels
enhancement New feature or request

Comments

@Mabenan
Copy link

Mabenan commented Nov 10, 2022

Hi like i mentioned here appwrite/sdk-for-flutter#96 (comment) I found a potential solution for the callback-uri-scheme.
With this package https://pub.dev/packages/desktop_webview_window we could open a WebView window and call the Auth Url after the login process a URL change to our callback-uri-scheme will happen this can be catched with a registration here void addOnUrlRequestCallback(OnUrlRequestCallback callback);.

For the transition time i would say that we implement it as an additional feature.

So if a callback-uri-scheme is provided which is not fitting this Constraints

if (callbackUri.scheme != 'http' ||
(callbackUri.host != 'localhost' && callbackUri.host != '127.0.0.1') ||
!callbackUri.hasPort) {
throw ArgumentError(
'Callback url scheme must start with http://localhost:{port}',
);
}

Use the in App WebView

If wanted I will provide an PR for this.

@Mabenan Mabenan added the enhancement New feature or request label Nov 10, 2022
@ThexXTURBOXx
Copy link
Owner

I have seen that comment already, but don't think that it will make a lot of sense to implement another temporary solution on top of a current temporary one.
The biggest drawback of that package is that it will require the user to install additional software, i.e., WebView2 Runtime or webkit2gtk-4.0, respectively.
Right now, the user has the best experience whilst the developer has to make sure his application fits the constraints.
However, as already stated, if there are better (permanent) solutions available to tackle this problem, I will of course apply the solutions asap! :)

@Mabenan
Copy link
Author

Mabenan commented Nov 10, 2022

Ok what i also found out on my Researches are that you can register a scheme in the windows registry. Need to check the native windows api if the programm can do this itself without admin consent.

Then the solutuon would be to register the scheme with a native call and assign it to the exe than implement on the start a reaction on the callback and send the data to the server we already create, or change it to a socket solution.

@Mabenan
Copy link
Author

Mabenan commented Nov 11, 2022

Ok it is possible to add a registry key without user interaction and without admin rights.
So now we only need a concept how we init the callback scheme and how we react on it.

This include like already mention a reaction on the callback at the start of the implimentation.

I think of a static method call where you give in the callback scheme you want to register and the arguments passed to the application.

@ThexXTURBOXx
Copy link
Owner

As far as I can remember from my old regedit times, it suffices to add the scheme as a key in HKCR and specifying the application to launch inside that key, right?
If this is the case, there could be a couple of problems:

  1. I do not know if that is true at all, but maybe the PC must be restarted after changing the registry. There are at least many keys that need a computer restart for their change to take effect. If that is really the case, it is not a good way to add it in runtime and removing it afterwards. It should stay there permanently, which would mean in turn that after uninstalling the application, it should also get removed in order to avoid future hicups.
  2. The application will not be able to retrieve correctly if it is already open. This could be circumvented by the application indicating that it only can be opened once at a time and if it is opened again, it just sends the start arguments via socket to the already open application (like Luyten, for example, although it is non-Flutter)

There might be more problems, but these are the two largest one I can think of right now. Maybe I will test a few things in the near future to see if I can get them to work. However, I cannot promise anything since I am in the middle of writing my Master's Thesis at the moment and have to use my spare time to keep all my projects at least somewhat alive. So, bugs can be fixed; enhancements might take longer :)

@Mabenan
Copy link
Author

Mabenan commented Nov 11, 2022

I also do some researches in the meantime from what I see it is not necessary to restart the computer or the app to activate the registry. At least for the HKEY_CURRENT_USER hive, which is the one I would use because a normal user can add keys here.

@Mabenan
Copy link
Author

Mabenan commented Nov 11, 2022

For instancing we could utilize this one
https://pub.dev/packages/multi_instance_handler

@ThexXTURBOXx
Copy link
Owner

I am not sure if I would like to add too many more dependencies to this library. Especially multi_instance_handler adds shared_preferences for instance which is very unnecessary for these use-cases :)

@ThexXTURBOXx
Copy link
Owner

Oh, and also there are bug reports like this one: lsegal/flutter_multi_instance_handler#3
which make the entire library very unusable

@Mabenan
Copy link
Author

Mabenan commented Nov 11, 2022

Good point, the lib isn't big the sharedpref is not needed for windows so i would say we just take what we need.

@Mabenan
Copy link
Author

Mabenan commented Nov 11, 2022

Oh didn't seen that, ok than i will take the code an make own researches maybe it is a simple bug but it should be possible to implement.

@ThexXTURBOXx
Copy link
Owner

The library in itself would be very nice, but it should not have too many (or even better no) dependencies. Then, it might be worth a try

@sumersm7
Copy link

i was looking around and found this
https://pub.dev/packages/protocol_handler
(android, ios , windows , macos)

this use win32 package to edit registry for custom url.
but after this i was not able to compile for web as web try to compile win32 and its not for web
and idk how i can prevent win32 from compiling for web

@ThexXTURBOXx
Copy link
Owner

@sumersm7 It looks promising, however, yes, we need to see that it won't break other platforms.
Maybe someone wants to make a PR if he knows how to circumvent this.
Otherwise, I will see at some point if I can edit the registry in flutter_web_auth_2 in a similiar way without incorporating the package.

@sumersm7
Copy link

flutter plugin and platform interface is new for me. make my brain hurt trying to understand.
protocol_handler hasn't used plugin_platform_interface like this plugin
i think that's why "win32_registry" is not ignored while compiling for web
i would have liked to help if i am not a noob

@Mabenan
Copy link
Author

Mabenan commented Jan 30, 2023

We could use win32_registry package the important thing is to only call it in windows cosing so in the correct dart file. The package only uses ffi and win32 like we already does to bring the window to the front. I also tried a little bit around and got the registration working.

The thing that is much more complicated is that we get the app to handle the callback correctly because windows will start a new instance of the programm that is customized in the registration

@sumersm7
Copy link

Using app_links and that specific code from protocol_handler to register scheme , and a conditional import and a empty class with same name so it doesn't get imported in other than window. And nodejs and passport js. I was able to use Oauth login. Let me know if u want more detail about it . It's a jank method but it solves the problem 😅

@ThexXTURBOXx
Copy link
Owner

@sumersm7 I am a bit reluctant to replace a jank method (the current one) with another and, thus, add breaking changes.
If I want to replace the current method with breaking changes, at least they should be done properly :)

@Mabenan Sounds good for now. If we are able to circumvent opening another instance at some point, I will probably add this or a similar approach.
Many applications open a server socket when launching and if the port is already bound, they just send the current startup parameters to that port such that the already opened application can react to them. Not the best approach, maybe there is a better one that we could implement...

@sumersm7
Copy link

  1. Iam not suggesting any changes until ur satisfied.
  2. My jank method won't be replacing urs it will be adding on top of it (it's a joke)
  3. Is there another place to discuss. (Other than this issue thread just asking)

@macwilko
Copy link

Hey guys, thanks for maintaining this library.

I'm currently implementing a large complex Flutter app with an OAuth2 login flow.

I plan to support:

  • iOS
  • Android
  • Windows
  • macOS

will I be able to support all of these platforms with flutter_web_auth_2 ? I'm a bit confused about Windows support specifically due to this callback URL limitation.

@ThexXTURBOXx
Copy link
Owner

@mattsrobot Look at the example implementation, it works on all these platforms simultaneously :)

@aawssm
Copy link

aawssm commented Apr 17, 2023

Do u have any backend server like nodejs, or dart? @mattsrobot

If u have a node server u can checkout my flutter code that i tested on Android windows and web.

@macwilko
Copy link

@aawssm thank-you kindly! I'm familiar with Node.js I'de love to check it out, much appreciated.

@ThexXTURBOXx
Copy link
Owner

This is no longer a limitation in 4.0.0-alpha.0, thanks to #92!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants