emansih / FireflyMobile

Mobile Application for Firefly III written in Kotlin
GNU General Public License v3.0
323 stars 48 forks source link

OAuth empty URL doesn't trigger anything #60

Closed mastacheata closed 5 years ago

mastacheata commented 5 years ago

Firefly III Server Version: 4.7.17.3 Firefly III Mobile Version: 1.3.5 (you apparently forgot to release 1.3.6 on F-Droid) Android Version:
9 Device Information:

On an indirectly related note: Why did you choose http for your custom handler URL? I doubt that's allowed anymore. Instead of changing the hostname and catching that, you should rather specify a custom scheme (that's the part where it says http or https on the web) or just switch to the Out-Of-Bounds flow for dumb devices (TV, Stereo, etc.) by using oob as the redirect string.

emansih commented 5 years ago

What browser are you using? Some browsers have issues with redirect flow.

I can't use a custom redirect flow since Firefly III does not support it out of the box. See this issue.

mastacheata commented 5 years ago

Ahh, sorry had the browser listet here in an earlier draft. I tried both Firefox and Chrome. (Firefox is my default browser, though as mobile Chrome doesn't support extensions)

mastacheata commented 5 years ago

I'll look into the firefly server code tonight. PHP and Laravel are more my cup of tea than Java and Swift/ObjC, it should be easy to send a PR to make this work as expected.

emansih commented 5 years ago

For Firefox,an extra step is needed. See the wiki

mastacheata commented 5 years ago

I checked the wiki, but the android button is not there in the current Firefox version. (same is true for Chrome, nothing happens and the firefly app is not available in the share dialog)

emansih commented 5 years ago

btw I do not control builds on F-Droid. As of 23 July 2019, 19:58 hrs(GMT +8), build are failing because the F-Droid team have not verified the hash of Gradle 5.5.

== Installed Android Tools ==

INFO: Creating log directory
INFO: Creating temporary directory
INFO: Creating output directory
INFO: Using git version 2.11.0
INFO: Building version 1.3.6 (44) of xyz.hisname.fireflyiii
INFO: Getting source for revision v1.3.6
INFO: Creating local.properties file at build/xyz.hisname.fireflyiii/local.properties
INFO: Creating local.properties file at build/xyz.hisname.fireflyiii/app/local.properties
INFO: Cleaned build.gradle of keysigning configs at build/xyz.hisname.fireflyiii/app/build.gradle
INFO: Cleaning Gradle project...
ERROR: Could not build app xyz.hisname.fireflyiii: Error cleaning xyz.hisname.fireflyiii:1.3.6
==== detail begin ====
Found 5.5 via distributionUrl
No hash for gradle version 5.5! Exiting...
==== detail end ====

I do have my own self hosted F-Droid repo that you can use.

mastacheata commented 5 years ago

I do have my own self hosted F-Droid repo that you can use. Ahh, I misunderstood that message in the Readme as saying whatever is in F-Droid was coming from your own CI/CD Pipeline.

I'll look into that tonight as well.

mastacheata commented 5 years ago

This is how Firefox looks when opening the empty URL: https://photos.app.goo.gl/pnEGxEd7MkMfJAnZ9

And this is what Google Chrome does with that URL (I even opened the menu, but there is no way to open an external app from here): https://photos.google.com/photo/AF1QipPBX1j0Z_fHtjZz0KwcdPTHPtkq1m1sQ3D9V_Ed

There's also no option to share the link with the Firefly app in Firefox's share dialog: https://photos.google.com/photo/AF1QipPlmmSJoNUd1aygF0zWiscDCCk7xHtg-FljAMf6

emansih commented 5 years ago

Did you press the refresh button in Firefox?

photos.google.com/photo/AF1QipPBX1j0Z_fHtjZz0KwcdPTHPtkq1m1sQ3D9V_Ed photos.google.com/photo/AF1QipPlmmSJoNUd1aygF0zWiscDCCk7xHtg-FljAMf6

Both of them leads me to a 404 page

emansih commented 5 years ago

For Firefox and it's derivatives(Fennec / Firefox focus), there is an extra step. Once authentication is completed, you will be shown a Server not found error message similar to.... Click on refresh and then click on the Android icon in the address bar, you will be redirected back to the app.

mastacheata commented 5 years ago

Ahh, I missed the refresh. Now it works on both chrome and Firefox. It would be a good idea to have that hint ABOVE the image (which is screen filling on my phone) instead of below.

mastacheata commented 5 years ago

The URI validation is part of Laravel's Passport library and all attempts at changing their policy for allowed URI Schemes have failed in the past. Apparently, though: Not only http URIs are allowed, but also a long list of other schemes, inlcuding but not limited to the tag, urn and xri schemes.

For the time being I'd recommend using the xri scheme in the documentation. It's the only of the three schemes that's actually validating the Laravel rules and the set scheme for the URI in the corresponding RFC at the same time. Also the XRI scheme was a failed attempt by OASIS to create a namespace for identifying connection between things that was officially dropped in 2005. Since the namespace was never publicly endorsed or used in any projects, it should be pretty safe to use. Maybe tell the app to listen for the xri://firefly URI instead of the whole scheme in case other app devs might want to use the same scheme to get around similar quirks in web-applications.

I'll also have a look at the league/oauth2-server on the weekend and will try to implement the out-of-bounds/device-flow there. That should drip down to laravel passport and firefly as well eventually, but it will most likely take months before that happens, regardless how fast I can implement it.

emansih commented 5 years ago

Thanks for your input. I will update the wiki to clarify the authentication flow.

You should drop a mail to James or open an issue on the main firefly repo.

mastacheata commented 5 years ago

Except for moving to a different authentication backend, there's nothing he could do in the main app to enable the device flow authentication. I'll ask him to include the list of available URL schemes that work in Laravel Passport into the documentation, though.

Firefly-iii is based on Laravel and uses the Laravel Passport plugin to handle the API-Authentication. The Laravel Passport plugin defines the allowed URL schemes (though, it does it badly because it allows you to use the schemes only when followed by a colon and to slashes, where most schemes don't use or don't allow slashes in the URL. For the OAuth2 process itself, the laravel plugin uses the league/oauth2-server package, but that doesn't have an implentation of the device flow yet. I'm a software developer myself and know both PHP and OAuth (I even built my own OAuth1 library once), so I will try to implement that feature in the league package and hope it will at some point drip down the 3 levels to firefly-iii and the 4th level to your mobile app, which I really like now that I had the chance to try it out for a few days.

spaetz commented 5 years ago

It would be great to add a hint to the oauth page on how to proceed, right now, this has to be manually found out after failing... For Firefox,an extra step is needed. See the wiki This line added to the auth page would have been sufficient to bring me on the right track... :-)

emansih commented 5 years ago

Hi @mastacheata ,I'm closing this issue since there is no activity. Any updates on the URI validation from upstream?

mastacheata commented 5 years ago

They're unwilling to include it at the moment and I don't have the time to build it myself.