diff --git a/README.md b/README.md index 1c283f44..bfc7010b 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,26 @@ either through custom URI scheme redirects, or App Links. AS's that assume all clients are web-based or require clients to maintain confidentiality of the client secrets may not work well. +From Android API 30 (R) and above, set [queries](https://developer.android.com/preview/privacy/package-visibility) in the manifest, +to enable AppAuth searching for usable installed browsers. +```xml + + + + + + + + + + + + + + ... + +``` + ## Demo app A demo app is contained within this repository. For instructions on how to @@ -261,7 +281,7 @@ this redirect URI. We recommend using a custom scheme based redirect URI (i.e. those of form `my.scheme:/path`), as this is the most widely supported across all versions of -Android. To avoid conflicts with other apps, it is recommended to configure a +Android. To avoid conflicts with other apps, it is recommended to configure a distinct scheme using "reverse domain name notation". This can either match your service web domain (in reverse) e.g. `com.example.service` or your package name `com.example.app` or be something completely new as long as it's distinct @@ -273,12 +293,6 @@ else. When a custom scheme is used, AppAuth can be easily configured to capture all redirects using this custom scheme through a manifest placeholder: -```groovy -android.defaultConfig.manifestPlaceholders = [ - 'appAuthRedirectScheme': 'com.example.app' -] -``` - Alternatively, the redirect URI can be directly configured by adding an intent-filter for AppAuth's RedirectUriReceiverActivity to your AndroidManifest.xml: diff --git a/app/AndroidManifest.xml b/app/AndroidManifest.xml index 36c43f85..30082737 100644 --- a/app/AndroidManifest.xml +++ b/app/AndroidManifest.xml @@ -15,6 +15,19 @@ + + + + + + + + + + + + + - - - - - - - - + diff --git a/library/build.gradle b/library/build.gradle index 0f61f49a..6c68ce2d 100644 --- a/library/build.gradle +++ b/library/build.gradle @@ -3,6 +3,7 @@ apply plugin: 'maven-publish' apply plugin: 'signing' apply from: '../config/android-common.gradle' +apply from: 'gradle-maven-push.gradle' group = GROUP version = rootProject.versionName diff --git a/library/gradle-maven-push.gradle b/library/gradle-maven-push.gradle new file mode 100644 index 00000000..a09f6325 --- /dev/null +++ b/library/gradle-maven-push.gradle @@ -0,0 +1,75 @@ +apply plugin: 'maven-publish' + +def artifactoryURL = "https://artifactory.skyscannertools.net/artifactory/infrastructure-maven" + +task sourceJar(type: Jar) { + from android.sourceSets.main.java.srcDirs + archiveClassifier = 'source' +} + +afterEvaluate { project -> + + project.ext.getJFrogPassword = { + if (project.hasProperty("SKYSCANNER_ARTIFACTORY_MAVEN_PASSWORD")) { + return project.property("SKYSCANNER_ARTIFACTORY_MAVEN_PASSWORD") + } + return "" + } + + project.ext.getJFrogUsername = { + if (project.hasProperty("SKYSCANNER_ARTIFACTORY_MAVEN_USER")) { + return project.property("SKYSCANNER_ARTIFACTORY_MAVEN_USER") + } + return "" + } + + publishing { + publications { + maven(MavenPublication) { + groupId 'net.openid' + artifactId 'appauth' + version versionName + description description + + artifact bundleReleaseAar + artifact sourceJar + + pom.withXml { + + def dependenciesNode = asNode().appendNode('dependencies') + //Iterate over the compile dependencies (we don't want the test ones), adding a node for each + project.configurations.implementation.allDependencies.each { + if (it.group != null && (it.name != null || "unspecified" == it.name) && it.version != null) { + def dependencyNode = dependenciesNode.appendNode('dependency') + dependencyNode.appendNode('groupId', it.group) + dependencyNode.appendNode('artifactId', it.name) + dependencyNode.appendNode('version', it.version) + } + } + } + } + } + + repositories { + maven { + url "$artifactoryURL" + credentials { + username "${project.getJFrogUsername()}" + password "${project.getJFrogPassword()}" + } + } + } + } + + task checkMavenCredentials(type: Exec) { + outputs.upToDateWhen { false } + workingDir './' + commandLine(['curl', + '--silent', + '--fail', + '-I', + '-u', + "${project.getJFrogUsername()}:${project.getJFrogPassword()}", + "$artifactoryURL"]) + } +} diff --git a/library/java/net/openid/appauth/AuthState.java b/library/java/net/openid/appauth/AuthState.java index cfc44748..22ec4eae 100644 --- a/library/java/net/openid/appauth/AuthState.java +++ b/library/java/net/openid/appauth/AuthState.java @@ -430,7 +430,7 @@ public void update( // developer that this is unexpected. Logger.warn( "AuthState.update should not be called in an error state (%s), call update" - + " with the result of the fresh authorization response first", + + "with the result of the fresh authorization response first", mAuthorizationException); mAuthorizationException = null; } diff --git a/library/java/net/openid/appauth/AuthorizationManagementActivity.java b/library/java/net/openid/appauth/AuthorizationManagementActivity.java index 40842874..22ab24ee 100644 --- a/library/java/net/openid/appauth/AuthorizationManagementActivity.java +++ b/library/java/net/openid/appauth/AuthorizationManagementActivity.java @@ -23,7 +23,6 @@ import android.net.Uri; import android.os.Bundle; import androidx.annotation.VisibleForTesting; -import androidx.appcompat.app.AppCompatActivity; import net.openid.appauth.AuthorizationException.AuthorizationRequestErrors; import net.openid.appauth.internal.Logger; @@ -124,7 +123,7 @@ * {@link AuthorizationException} as appropriate. * The AuthorizationManagementActivity finishes, removing itself from the back stack. */ -public class AuthorizationManagementActivity extends AppCompatActivity { +public class AuthorizationManagementActivity extends Activity { @VisibleForTesting static final String KEY_AUTH_INTENT = "authIntent"; @@ -224,6 +223,11 @@ protected void onResume() { */ if (!mAuthorizationStarted) { + if (mAuthIntent == null) { + finish(); + return; + } + try { startActivity(mAuthIntent); mAuthorizationStarted = true; diff --git a/library/java/net/openid/appauth/IdToken.java b/library/java/net/openid/appauth/IdToken.java index 4a4556ef..13bb4306 100644 --- a/library/java/net/openid/appauth/IdToken.java +++ b/library/java/net/openid/appauth/IdToken.java @@ -279,11 +279,9 @@ void validate(@NonNull TokenRequest tokenRequest, // OpenID Connect Core Section 3.1.3.7. rule #10 // Validates that the issued at time is not more than +/- 10 minutes on the current // time. - if (Math.abs(nowInSeconds - this.issuedAt) > TEN_MINUTES_IN_SECONDS) { - throw AuthorizationException.fromTemplate(GeneralErrors.ID_TOKEN_VALIDATION_ERROR, - new IdTokenException("Issued at time is more than 10 minutes " - + "before or after the current time")); - } + // Not enforced. Time-based rules will break if users set the time in their device + // settings manually + // Only relevant for the authorization_code response type if (GrantTypeValues.AUTHORIZATION_CODE.equals(tokenRequest.grantType)) { diff --git a/library/java/net/openid/appauth/RedirectUriReceiverActivity.java b/library/java/net/openid/appauth/RedirectUriReceiverActivity.java index 6580429a..ce2a5257 100644 --- a/library/java/net/openid/appauth/RedirectUriReceiverActivity.java +++ b/library/java/net/openid/appauth/RedirectUriReceiverActivity.java @@ -14,8 +14,8 @@ package net.openid.appauth; +import android.app.Activity; import android.os.Bundle; -import androidx.appcompat.app.AppCompatActivity; /** * Activity that receives the redirect Uri sent by the OpenID endpoint. It forwards the data @@ -24,13 +24,6 @@ * {@link android.app.PendingIntent} * provided to {@link AuthorizationService#performAuthorizationRequest}. * - * App developers using this library must override the `appAuthRedirectScheme` - * property in their `build.gradle` to specify the custom scheme that will be used for - * the OAuth2 redirect. If custom scheme redirect cannot be used with the identity provider - * you are integrating with, then a custom intent filter should be defined in your - * application manifest instead. For example, to handle - * `https://www.example.com/oauth2redirect`: - * * ```xml * * @@ -42,7 +35,7 @@ * * ``` */ -public class RedirectUriReceiverActivity extends AppCompatActivity { +public class RedirectUriReceiverActivity extends Activity { @Override public void onCreate(Bundle savedInstanceBundle) { diff --git a/library/java/net/openid/appauth/browser/CustomTabManager.java b/library/java/net/openid/appauth/browser/CustomTabManager.java index 28573c54..af1728d8 100644 --- a/library/java/net/openid/appauth/browser/CustomTabManager.java +++ b/library/java/net/openid/appauth/browser/CustomTabManager.java @@ -81,8 +81,13 @@ public void onServiceDisconnected(ComponentName componentName) { public void onCustomTabsServiceConnected(ComponentName componentName, CustomTabsClient customTabsClient) { Logger.debug("CustomTabsService is connected"); - customTabsClient.warmup(0); - setClient(customTabsClient); + try { + customTabsClient.warmup(0); + setClient(customTabsClient); + } catch (SecurityException ex) { + Logger.error("CustomTabsService failed to warmup", ex); + setClient(null); + } } private void setClient(@Nullable CustomTabsClient client) { diff --git a/library/javatests/net/openid/appauth/IdTokenTest.java b/library/javatests/net/openid/appauth/IdTokenTest.java index 13944f7c..077ce3da 100644 --- a/library/javatests/net/openid/appauth/IdTokenTest.java +++ b/library/javatests/net/openid/appauth/IdTokenTest.java @@ -448,22 +448,6 @@ public void testValidate_shouldFailOnExpiredToken() throws AuthorizationExceptio idToken.validate(tokenRequest, clock); } - @Test(expected = AuthorizationException.class) - public void testValidate_shouldFailOnIssuedAtOverTenMinutesAgo() throws AuthorizationException { - Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000; - Long tenMinutesInSeconds = (long) (10 * 60); - IdToken idToken = new IdToken( - TEST_ISSUER, - TEST_SUBJECT, - Collections.singletonList(TEST_CLIENT_ID), - nowInSeconds + tenMinutesInSeconds, - nowInSeconds - (tenMinutesInSeconds * 2) - ); - TokenRequest tokenRequest = getAuthCodeExchangeRequestWithNonce(); - Clock clock = SystemClock.INSTANCE; - idToken.validate(tokenRequest, clock); - } - @Test(expected = AuthorizationException.class) public void testValidate_shouldFailOnNonceMismatch() throws AuthorizationException { Long nowInSeconds = SystemClock.INSTANCE.getCurrentTimeMillis() / 1000; diff --git a/library/javatests/net/openid/appauth/browser/DeprecationMigrationTest.java b/library/javatests/net/openid/appauth/browser/DeprecationMigrationTest.java new file mode 100644 index 00000000..9a21e40a --- /dev/null +++ b/library/javatests/net/openid/appauth/browser/DeprecationMigrationTest.java @@ -0,0 +1,88 @@ +package net.openid.appauth.browser; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +import static org.assertj.core.api.Assertions.assertThat; + +@RunWith(RobolectricTestRunner.class) +@Config(sdk=16) +public class DeprecationMigrationTest { + + @Test + public void testMatches_emptyDenyList() { + BrowserDenyList denyList = new BrowserDenyList(); + assertThat(denyList.matches(Browsers.Chrome.customTab("46"))).isTrue(); + assertThat(denyList.matches(Browsers.Firefox.standaloneBrowser("10"))).isTrue(); + assertThat(denyList.matches(Browsers.SBrowser.standaloneBrowser("11"))).isTrue(); + } + + @Test + public void testMatches_singleBrowser() { + BrowserDenyList denyList = new BrowserDenyList(VersionedBrowserMatcher.FIREFOX_BROWSER); + assertThat(denyList.matches(Browsers.Chrome.customTab("46"))).isTrue(); + assertThat(denyList.matches(Browsers.Firefox.standaloneBrowser("10"))).isFalse(); + assertThat(denyList.matches(Browsers.SBrowser.standaloneBrowser("11"))).isTrue(); + } + + @Test + public void testMatches_customTabs() { + BrowserDenyList denyList = new BrowserDenyList( + VersionedBrowserMatcher.CHROME_CUSTOM_TAB, + VersionedBrowserMatcher.SAMSUNG_CUSTOM_TAB); + + assertThat(denyList.matches(Browsers.Chrome.standaloneBrowser("46"))).isTrue(); + assertThat(denyList.matches(Browsers.Chrome.customTab("46"))).isFalse(); + assertThat(denyList.matches(Browsers.SBrowser.standaloneBrowser("11"))).isTrue(); + assertThat(denyList.matches(Browsers.SBrowser.customTab("11"))).isFalse(); + } + + + @Test + public void testMatches_emptyAllowList() { + BrowserAllowList allowList = new BrowserAllowList(); + assertThat(allowList.matches(Browsers.Chrome.customTab("46"))).isFalse(); + assertThat(allowList.matches(Browsers.Firefox.standaloneBrowser("10"))).isFalse(); + assertThat(allowList.matches(Browsers.Firefox.customTab("57"))).isFalse(); + assertThat(allowList.matches(Browsers.SBrowser.standaloneBrowser("11"))).isFalse(); + } + + @Test + public void testMatches_chromeBrowserOnly() { + BrowserAllowList allowList = new BrowserAllowList(VersionedBrowserMatcher.CHROME_BROWSER); + assertThat(allowList.matches(Browsers.Chrome.standaloneBrowser("46"))).isTrue(); + assertThat(allowList.matches(Browsers.Chrome.customTab("46"))).isFalse(); + assertThat(allowList.matches(Browsers.Firefox.standaloneBrowser("10"))).isFalse(); + assertThat(allowList.matches(Browsers.Firefox.customTab("57"))).isFalse(); + } + + @Test + public void testMatches_chromeCustomTabOrBrowser() { + BrowserAllowList allowList = new BrowserAllowList( + VersionedBrowserMatcher.CHROME_BROWSER, + VersionedBrowserMatcher.CHROME_CUSTOM_TAB); + assertThat(allowList.matches(Browsers.Chrome.standaloneBrowser("46"))).isTrue(); + assertThat(allowList.matches(Browsers.Chrome.customTab("46"))).isTrue(); + assertThat(allowList.matches(Browsers.Firefox.standaloneBrowser("10"))).isFalse(); + assertThat(allowList.matches(Browsers.Firefox.customTab("57"))).isFalse(); + } + + @Test + public void testMatches_firefoxOrSamsung() { + BrowserAllowList allowList = new BrowserAllowList( + VersionedBrowserMatcher.FIREFOX_BROWSER, + VersionedBrowserMatcher.FIREFOX_CUSTOM_TAB, + VersionedBrowserMatcher.SAMSUNG_BROWSER, + VersionedBrowserMatcher.SAMSUNG_CUSTOM_TAB); + assertThat(allowList.matches(Browsers.Chrome.standaloneBrowser("46"))).isFalse(); + assertThat(allowList.matches(Browsers.Chrome.customTab("46"))).isFalse(); + assertThat(allowList.matches(Browsers.Firefox.standaloneBrowser("10"))).isTrue(); + assertThat(allowList.matches(Browsers.Firefox.customTab("56"))).isFalse(); + assertThat(allowList.matches(Browsers.Firefox.customTab("57"))).isTrue(); + assertThat(allowList.matches(Browsers.SBrowser.standaloneBrowser("10"))).isTrue(); + assertThat(allowList.matches(Browsers.SBrowser.customTab("4.0"))).isTrue(); + assertThat(allowList.matches(Browsers.SBrowser.customTab("3.9"))).isFalse(); + } +}