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.

Sign up for GitHub

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

Jump to bottom

Android: App crashes if Intent data is null #52

Closed
blazery opened this issue Aug 20, 2019 · 13 comments · Fixed by #85
Closed

Android: App crashes if Intent data is null #52

blazery opened this issue Aug 20, 2019 · 13 comments · Fixed by #85
Labels
android bug Something isn't working
Milestone
2.0.0

Comments

@blazery
Copy link

blazery commented Aug 20, 2019 •
edited
Loading

OAuth2ClientPlugin.java line 222 < response = AuthorizationResponse.fromIntent(data);>
Will crash the application when data is null.

When chrome custom tabs(used by AppAuth) redirect back to the application using the uri scheme, the data provided to handleOnActivityResult can be null.
Passing this to the AuthorizationReponse.fromIntent causes the application to crash.

Edit:
The same sort of issue can be found here
@maggix
Copy link

maggix commented Aug 28, 2019

I had the same issue when the intent URL was the same as the appAuthRedirectScheme.
Below is how I made the error go away, but maybe the documentation should be updated?

build.gradle:

manifestPlaceholders = [
                'appAuthRedirectScheme': 'com.testapp.ionic'
        ]

Strings.xml:

    <string name="custom_url_scheme">com.testapp.ionic.intent</string>

(I added .intent suffix to the app, but could be any text, as long as it differs from the other)

@blazery
Copy link
Author

blazery commented Aug 29, 2019 •
edited
Loading

Thanks for the response,
Unfortunatly i haven't got this to work yet.

In android/app/build.gradle I made sure to set the following:
manifestPlaceholders = [ 'appAuthRedirectScheme': 'com.company.mobiletest' ]

This matches the file structure in android/app/...../java

And in the strings.xml slightly different:
<string name="custom_url_scheme">com.company.mobiletestapp</string>
I need the last one to be com.company.mobiletestapp, because the uri registered with the Oauth server is com.company.mobiletestapp://oauth, and if i understand correctly this is what fill in in

 android: {
      customScheme: 'com.company.mobiletestapp://oauth'
 },

what am I doing wrong, im using the responseType: 'token', expecting the token to be returned in something like 'com.company.mobiletestapp://oauth#access_token=XXX'

@moberwasserlechner moberwasserlechner added android bug Something isn't working labels Oct 19, 2019
@moberwasserlechner moberwasserlechner added this to the 2.0.0 milestone Apr 15, 2020
@steve-allam
Copy link

steve-allam commented Apr 15, 2020 •
edited
Loading

Also suffering with this one - trying to get it to work, and I can see the following in the phone log:

5 16:52:35.731 27554 27587 D GeckoViewNavigation[C]: loadURI: uri=xxx.yyyy.myapp://oauth#access_token=EwBwA8J%3d%3d&token_type=bearer&expires_in=3600&scope=https://graph.microsoft.com/.default%20https://graph.microsoft.com/Mail.Read%20https://graph.microsoft.com/User.Read&state=mfvymhWXGXLLgtdvZiQz where=1 flags=0x800000 tp=null

So I can see that the phone is attempting to open a URI, and the handleOnActivityResult routine is being called in OAuth2ClientPlugin.java

I put a logging line in there, and it logs that requestCode=2000, resultCode=0 and data=null.

I tried various ways at getting the URI, but haven't succeeded yet - hopefully you might be able to give me some pointers.

Interestingly, the next few capacitor log lines are:
04-15 16:52:35.899 26698 26698 D Capacitor: Sending plugin error: {"save":false,"callbackId":"103362721","pluginId":"OAuth2Client","methodName":"authenticate","success":false,"error":{"message":"ERR_GENERAL; NULL INTENT DATA"}}

my error

04-15 16:52:35.910 26698 26698 V Capacitor/Plugin/App: Notifying listeners for event appUrlOpen
04-15 16:52:35.910 26698 26698 D Capacitor/Plugin/App: No listeners found for event appUrlOpen
...
04-15 16:52:35.994 26698 26698 D Capacitor: App restarted
04-15 16:52:36.012 26698 26698 D Capacitor: App started
...
04-15 16:52:36.015 26698 26698 D Capacitor/Plugin/App: Firing change: true
04-15 16:52:36.015 26698 26698 V Capacitor/Plugin/App: Notifying listeners for event appStateChange
04-15 16:52:36.016 26698 26698 D Capacitor/Plugin/App: No listeners found for event appStateChange
04-15 16:52:36.033 26698 26698 D Capacitor: App resumed

@moberwasserlechner
Copy link
Collaborator

moberwasserlechner commented Apr 15, 2020 •
edited
Loading

I'm about to look into this as I'm currently working on the 2.0.0 release but I need your version information and config of the plugin.

### Capacitor version:
<!-- Provide the version of Capacitor and related installed dependencies. 
You can use `npx cap doctor` for the output from the root directory of your project. -->

Run `npx cap doctor`:

Replace this with the commands output

### Library version:
<!-- Please remove all items that are not relevant. -->

- 2.0.0
- 1.1.0
- 1.0.1
- 1.0.0
- other: (Please fill in the version you are using.)

### OAuth Provider:
<!-- Please remove all items that are not relevant. -->

- Google
- Facebook
- Azure AD
- Azure App Registration
- Github
- Other: (Please fill in the provider you are using.)

### Your Plugin Configuration
<!-- Without secret stuff (of course). -->

{
    // Replace this with your plugin configuration 
}

Please create a new comment with your filled in data.

@moberwasserlechner
Copy link
Collaborator

moberwasserlechner commented Apr 15, 2020 •
edited
Loading

A note at "Your Plugin Configuration": Plz mask your secret stuff and do not remove the parameter because I need to see which config parameters you're using.

@steve-allam
Copy link

steve-allam commented Apr 15, 2020 via email •
edited
Loading

@steve-allam
Copy link

Tried something...added this to OAuth2ClientPlugin.java

@Override
protected void handleOnNewIntent(Intent intent) {
        Log.e(getLogTag(), "intent data: "+ intent.getDataString());
}

04-15 21:16:43.899 19959 19959 E Capacitor/Plugin/OAuth2ClientPlugin: intent data: 2000 0
04-15 21:16:43.912 19959 19959 D Capacitor: Sending plugin error: {"save":false,"callbackId":"60725044","pluginId":"OAuth2Client","methodName":"authenticate","success":false,"error":{"message":"ERR_GENERAL; NULL INTENT DATA"}}
-- this is from a log line I put into handleOnActivityResult
04-15 21:16:43.915 19959 19959 E Capacitor/Plugin/OAuth2ClientPlugin: intent data: com.domain.myapp://oauth#access_token=&token_type=bearer&expires_in=3600&scope=https://graph.microsoft.com/.default%20https://graph.microsoft.com/Mail.Read%20https://graph.microsoft.com/User.Read&state=zlANnwD7pBD9komlE3Uw
--- this is from a log line in handleOnNewIntent
04-15 21:16:43.916 19959 19959 V Capacitor/Plugin/App: Notifying listeners for event appUrlOpen
04-15 21:16:43.916 19959 19959 D Capacitor/Plugin/App: No listeners found for event appUrlOpen

Note that the NewIntentHandler does fire when the app starts, but of course the scheme won't match unless the intent matches that from the OAuth2 redirect.

@moberwasserlechner
Copy link
Collaborator

moberwasserlechner commented Apr 15, 2020 •
edited
Loading

Hi Steve,

As your using Capacitor 2.x you will have to wait for 2.0.0 of this plugin because on iOS you will most probably run into a problem with XCode 11.4. See #74 for details.

I'm working on better docs in a feature branch. Plz check if your Android configuration is valid.

Regardless of that v1.1.0 is not intended to work with Capactior 2.x only v2.0.0 will.

BR, Michael

@moberwasserlechner
Copy link
Collaborator

@steve-allam Thx for the Plugin#handleOnNewIntent(Intent intent) hint. I'll look into it.

@moberwasserlechner
Copy link
Collaborator

moberwasserlechner commented Apr 16, 2020 •
edited
Loading

@steve-allam Could you describe the steps on your phone until the error occurs. eg.

  1. Launch Capacitor App
  2. Tab login button
  3. Chrome (tab) launches but Capacitor App lives on in the background
  4. Enter credentials
  5. Chrome closes
  6. Capacitor App is switched to the foreground
  7. Intent is always null. Crash!

Which Android versions and devices do you test on?

I can only think of a situation when the Android system or the user closes the Capacitor App after Chrome is launched.

I also look into handleOnNewIntent but afaik it is only called from the BridgeActivity#load

According to the Capacitor Docs the way I handle the result of a activity should be fine. Android itself proposes the use of new AndroidX features to handle the result but that's not supported by Capacitor right now. (Plugin, Bridge)

I'll try changing the plugin's request code to sth more unique. Maybe another plugin uses the same but that should not be a problem because both plugins would get the intent.

@steve-allam
Copy link

steve-allam commented Apr 16, 2020 via email

@moberwasserlechner
Copy link
Collaborator

Hey Steve,

thx for the detailed response.

I will most likely support both cases and introduce a Android option to switch it on/off.

The default settings will depend on the outcome of my tests. If the OnNewIntent approach works the same way for my use case it the default.

BR, Michael

@moberwasserlechner moberwasserlechner mentioned this issue Apr 17, 2020
Merged
@moberwasserlechner
Copy link
Collaborator

I tested with API Level 29, 28, 27 in Simulator and 26 on a real device and handleAuthorizationRequestActivity was always called. Only on application startup the onNewIntent method was called but naturally without any OAuth information.

The app will not crash any more and I introduced a new Android config parameter handleResultOnNewIntent so you can switch on the alternative handling but it will be off on default.

@moberwasserlechner moberwasserlechner mentioned this issue Apr 25, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
android bug Something isn't working
Projects
None yet
Milestone
2.0.0
Development

Successfully merging a pull request may close this issue.

Android: Add alternative activity result handling hook capacitor-community/generic-oauth2
4 participants
@maggix @moberwasserlechner @blazery @steve-allam