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

Fixed authentication restart when the app is minimized [SDK-2199] #350

Merged
merged 12 commits into from
Jun 18, 2021

Conversation

Widcket
Copy link
Contributor

@Widcket Widcket commented Dec 28, 2020

Changes

The authentication with the SDK breaks when the screen is minimized and the application is resumed from the application icon. This PR fixes this bug by decoupling the authentication into two activities:

  • AuthenticationActivity: encapsulates the logic to launch the Custom Tabs intent, and pass the result back.
  • RedirectActivity: receives the callback URL (via an intent filter), clears the Custom Tabs activity from the stack and passes the callback URL to the AuthenticationActivity.

Screen Recording 2020-12-27 at 22 44 45

Both Activity definitions are provided by the SDK. Since the RedirectActivity uses intent placeholders, the app must provide the values in the app's build.gradle file:

android {
    defaultConfig {
        // Add the next line
        manifestPlaceholders = [auth0Domain: "YOUR_AUTH0_DOMAIN", auth0Scheme: "${applicationId}"]
    }
    ...
}

THIS IS A BREAKING CHANGE.

Testing

This change was tested manually on an emulator running Android 7.1.1.

  • This change adds unit test coverage
  • This change has been tested on the latest version of the platform/language or why not

Checklist

Copy link
Contributor

@lbalmaceda lbalmaceda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good, a few questions to resolve. Also, let's add a dedicated section on the top of the README with guidance on how to set up the SDK with these changes. If I'm using the previous version, what changes do I need to make to support this? Is it optional or am I forced to use and declare these new activities in the android manifest?

android/src/main/java/com/auth0/react/A0Auth0Module.java Outdated Show resolved Hide resolved
android/src/main/java/com/auth0/react/A0Auth0Module.java Outdated Show resolved Hide resolved
android/src/main/java/com/auth0/react/A0Auth0Module.java Outdated Show resolved Hide resolved
@Widcket Widcket modified the milestones: v2.7.0, v2-Next Jan 5, 2021
@lbalmaceda
Copy link
Contributor

@Widcket skipping this PR from 2.8.0 release's contents as this one contains a breaking change. I'll move it to vNext

@Widcket Widcket modified the milestones: v2.8.2, vNext Apr 29, 2021
@nlindroos
Copy link

Hi! Any chance to expedite getting this to an official version of react-native-auth0? It is a required fix to a critical issue, at least in the way we implement authentication on Android. At the moment we're thus forced to lock react-native-auth0 to a certain commit instead of the official package version to have this fix included.

@Widcket
Copy link
Contributor Author

Widcket commented May 7, 2021

@nlindroos I expect to pick this up next week, because we'll be changing the implementation a bit.

@Widcket
Copy link
Contributor Author

Widcket commented Jun 16, 2021

@lbalmaceda the PR has been updated to use intent placeholders. The README has been updated as well, please check the messaging.

Screen.Recording.2021-06-16.at.19.23.48.mov

Copy link
Contributor

@lbalmaceda lbalmaceda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I left a few comments. Should we add the before/after snippets for the build.gradle and the manifest files, maybe using diff syntax, to highlight the changes required after this breaking change?

@Widcket Widcket requested a review from lbalmaceda June 18, 2021 09:46
@Widcket
Copy link
Contributor Author

Widcket commented Jun 18, 2021

Should we add the before/after snippets for the build.gradle and the manifest files, maybe using diff syntax, to highlight the changes required after this breaking change?

I can point out to the actual commit with the changes on the RN app, but that would need to be after the SDK is released. So:
release SDK -> update app -> update SDK README pointing to commit with the changes.

@Widcket Widcket merged commit b991985 into master Jun 18, 2021
@Widcket Widcket deleted the fix/decouple-auth branch June 18, 2021 12:57
@Widcket Widcket mentioned this pull request Jun 22, 2021
@Bardiamist
Copy link
Contributor

What if I want to continue to use singleTask because other dependencies may require it?

@Bardiamist
Copy link
Contributor

Bardiamist commented Jun 24, 2021

Can auth0Domain store not in android/app/build.gradle? Because different environments (dev/stage/prod) should use different auth0Domain and to replace full android/app/build.gradle isn't good. Need a separate config-file.

@Widcket
Copy link
Contributor Author

Widcket commented Jun 25, 2021

Hi @Bardiamist, you can add a string resource file and reference the domain value like this: auth0Domain: "@string/com_auth0_domain".

If you keep the singleTask, the auth wil restart when the app is minimized:

Screen.Recording.2021-06-24.at.11.00.03.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android: putting the app on background and tapping app's launch icon closes authentication webview
4 participants