Better error handling for required SSL

[PeterKottas]

Report type:
BUG

Summary:
SSL appears to be enforced by AppAuth-Android library therefore you cannot use http for your idp. Makes sense but remember this limitation exists for localhost as well which is quite a letdown for dev and testing.

Expected result:
If you try to connect to NON-SSL idp, a meaningful error message should be presented to the developer.

Actual result:
App crashes with "Unfortunately, XXX has stopped. OK"

[jevakallio]

Thanks for this report.

I believe we could do this by providing a custom ConnectionBuilder as in the certificate pinning example: https://github.com/openid/AppAuth-Android#customing-the-connection-builder-for-http-requests

API-wise, if we end up implementing this, I'd like it to be behind a configuration option such as dangerouslyAllowInsecureHttpRequests or similar flag that makes it obvious that this option might not be a good idea in prod :)

@PeterKottas do you have an example of a publically available IDP endpoint we could test/dev against?

[PeterKottas]

You're welcome guys, this project deserves to get as good as possible. It's by far the best solution to make this happen.
Don't have a publicly available IDP but what you can do is clone the IdentityServer4 demo from https://github.com/IdentityServer/IdentityServer4.Demo, run on localhost and test with that. I think that's as easy as possible.
Totally agreed with dangerouslyAllowInsecureHttpRequests flag but it would be very beneficial to be able to do that.
So far I've spend 10 hours trying to set everything up to have whole localhost on ssl. It's a pain.
(you have to create root CA, then sign certiifate with this ca, it needs to have alt_names to make your chrome and firefox happy but it can't be self signed directly to make android happy ... it's a pain)

[jevakallio]

For local repro there is one additional step: install and setup a Windows VM 😜

But yes, agreed, nobody should ever need to generate a root ca for local testing. We'll see what we can do about it.

Does this affect iOS or only Android?

[PeterKottas]

No idea as for me to test that, I would have to install Mac VM 😜 :D
Dot net core is actually cross platform so I think you should be able to run that demo on Mac or Linux. That being said, I never tried it. Check here https://www.microsoft.com/net/learn/get-started/macos or here for linux https://www.microsoft.com/net/learn/get-started/linuxredhat.

[PeterKottas]

Btw, I assume (and hope) that people use ssl on dev and staging anyways so we're screwed one way or another :D To be fair, biggest complexity with this is how damn hard it is to work with certificates across platforms, emulators, browsers etc. But still good idea to fix it for local. Have Dev-Ops worry about it :D

[jevakallio]

@PeterKottas can you try the feature/allow-insecure-requests branch on Android and tell me if that fixes the issue for you?

You can do this by replacing the dependency in package.json:

    "react-native-app-auth": "git+https://github.com/FormidableLabs/react-native-app-auth.git#feature/allow-insecure-requests"

And passing a dangerouslyAllowInsecureHttpRequests flag in your authorize/refresh config:

const config = {
  issuer: 'http://demo.identityserver.io',
  clientId: 'native.code',
  redirectUrl: 'io.identityserver.demo:/oauthredirect',
  additionalParameters: {},
  scopes: ['openid', 'profile', 'email', 'offline_access'],
+  dangerouslyAllowInsecureHttpRequests: true
};

This will likely break the iOS client for now, just want to know if this works for your use case. If it does, I'll clean it up and write some tests for it.

[PeterKottas]

Damn you're fast! I figured it will take weeks so I ported our whole local env to ssl which hurt a lot :D

Anyways ... I gave this a go, it appears to be 50% done, now the app redirects to http idp (hurray) but when you authenticate and it redirects back, that's when it crashes now. Almost there ;)

[PeterKottas]

I've kept separate branches to test this so feel free to abuse it as you get along with implementing. It quite easy to switch and test for me.

[jevakallio]

Ok, I've no idea why the app would crash after a redirect. Do you see a stack trace in logcat, or does it display an error message generally?

I tried to replicate this using the official IdentityServer 4 demo, but it doesn't appear to support non-TLS connections. I guess we'll have to do a local installation and give it a spin.

[jevakallio]

@PeterKottas all right, I figured this out, I wasn't passing the new connection builder through to the token fetch request. I added that, plus support for using HTTPS with self-signed/invalid certificates.

Could you please test the feature/allow-insecure-requests-v2 branch?

    "react-native-app-auth": "git+https://github.com/FormidableLabs/react-native-app-auth.git#feature/allow-insecure-requests-v2"

I'm pretty sure this will work. If it does, I'll need to do a bit of refactoring before landing this.

[PeterKottas]

Nicely done! All works now as expected. I am curious what the situation is with IOS but unfortunately I am not able to test it. I'll get to it eventually but it will be solid few months before that happens. First we'll do everything on Android and web then spin up Mac VM and pray for pain free app build :D (I'll leave the issue open for now cause I guess you might want to use it to coordinate the release, but if not feel free to close. Everything works as expected. Just that refactoring left to do. GL with the PR)

[jevakallio]

Ok, #50 is open if you want to take a peek.

Tested this on iOS, and it works out of the box as long as App Transport Security is configured to allow plain HTTP traffic via NSExceptionAllowsInsecureHTTPLoads (or any of the other applicable settings).

[PeterKottas]

Cheers I'll take a look. I am kid of curious what changes you made as I never spend much time doing native stuff.