iframe-proxy

hnljp · 2018-07-20T20:46:23Z

a possible default browser of the user is respected.

fixes: #372

split from #369

Signed-off-by: Henning Nielsen Lund henning.n.lund@jp.dk

a possible default browser of the user is respected. fixes: openid#372 split from openid#369 Signed-off-by: Henning Nielsen Lund <henning.n.lund@jp.dk>

codecov-io · 2018-07-20T20:52:44Z

hnljp · 2018-07-20T21:02:09Z

As stated in #369

I have tried to look more into it.
The problem is that the PackageManager.MATCH_ALL flag is making the result unsorted.
https://developer.android.com/reference/android/content/pm/PackageManager#MATCH_ALL
But the problem is that removing MATCH_ALL, will make queryIntentActivities only return the default browser on Android >= 7.0.

Looking at the code generating the list returned by queryIntentActivities:
https://android.googlesource.com/platform/frameworks/base.git/+/master/services/core/java/com/android/server/pm/PackageManagerService.java#7801
That is using the list generated as "an unsorted list":
https://android.googlesource.com/platform/frameworks/base.git/+/master/services/core/java/com/android/server/pm/PackageManagerService.java#7731
That is why there is a need to get the package name of the default browser and the list using MATCH_ALL is needed, in case the default browser is not compatible with Custom Tabs.

hnljp · 2018-07-20T23:02:32Z

Fixes #368

Add support for custom tabs in Firefox. Support for custom tabs was added to stable Firefox in version 57.0 Fixes: openid#368 Signed-off-by: Henning Nielsen Lund <henning.n.lund@jp.dk>

iainmcgin

Thanks for this! If you can address the review comments, in particular the need for tests, I can merge this change.

iainmcgin · 2018-07-27T17:34:44Z

        if (VERSION.SDK_INT >= VERSION_CODES.M) {
            queryFlag |= PackageManager.MATCH_ALL;
        }
+        ResolveInfo resolvedDefaultActivity =


Could you add a brief comment here describing the issue, e.g.

When requesting all matching activities for an intent from the package manager, the user's preferred browser is not guaranteed to be at the head of this list. Therefore, the preferred browser must be separately determined and the resultant list of browsers reordered to restored this desired property.

iainmcgin · 2018-07-27T17:37:56Z


                if (hasWarmupService(pm, info.activityInfo.packageName)) {
-                    browsers.add(new BrowserDescriptor(packageInfo, true));
+                    BrowserDescriptor customTabBrowserDescriptor =


Could you add tests to BrowserSelectorTest that demonstrate:

If the user has no default browser, the order of the returned browsers matches the order of matching intents returned by queryIntentActivities. This case is likely already covered by the existing tests, but it will help to make that more explicit.

If the user has a default browser, with custom tab support, the two entries for that browser are at the start of the list, with the custom tab descriptor first.

If the user has a default browser, without custom tab support, that one entry for that browser is at the start of the list.

The code looks correct to me, but I just want to make sure that the changes are appropriately covered by tests so that future changes don't result in regressions.

The tests have now been added.

a possible default browser of the user is respected. fixes: openid#372 split from openid#369 Signed-off-by: Henning Nielsen Lund <henning.n.lund@jp.dk>

…droid into default-browser

openid#379 (comment) Signed-off-by: Henning Nielsen Lund <henning.n.lund@jp.dk>

Add tests for when a default browser is set. Signed-off-by: Henning Nielsen Lund <henning.n.lund@jp.dk>

The test is now also checking if the BrowserDesriptors are added in the correct order, by checking useCustomTab. Signed-off-by: Henning Nielsen Lund <henning.n.lund@jp.dk>

Now also check if there is no browser supporting custom tabs, but a browser is set as the default browser, so it should be returned. Signed-off-by: Henning Nielsen Lund <henning.n.lund@jp.dk>

hnljp · 2018-08-04T07:33:05Z

iainmcgin

Thank you for your patience, and for the addition of the tests.

* Minor README fixes (openid#351) Thanks to @celphy for spotting these mistakes. * Add WeWork to AUTHORS WeWork, as my employer, is now the copyright owner for my contributions to AppAuth. * Dependency version bump * Handle CustomTabsSession.newSession failures Fixes openid#315. * Enable Java 8 language features * Do not automatically pass scope on token exchange request Fixes openid#297. * Do not override tab title setting Fixes openid#358. * Firefox customtabs (openid#378) Add support for custom tabs in Firefox. Support for custom tabs was added to stable Firefox in version 57.0 Fixes: openid#368 Signed-off-by: Henning Nielsen Lund <henning.n.lund@jp.dk> * Respect default browser of the user (openid#379) Correctly orders the user's preferred browser to the start of the returned list. * Custom Tabs was first added in Samsung Internet 4.0 (openid#383) At the moment all versions of the Samsung Internet Browser is set to support Custom Tabs. Support for Custom Tabs was first added in version 4.0. fixes: openid#382 * add getBrowserDescriptor() (openid#388) Make it possible to get the BrowserDescriptor used by AuthorizationService. fixes: openid#387 Signed-off-by: Henning Nielsen Lund <henning.n.lund@jp.dk> * Android gradle plugin version bump * Implement ID Token validation according to OIDC spec (openid#385) Validate ID Tokens (if any) on TokenResponses according to http://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation * Add Øyvind to the contributors list * Update CONTRIBUTORS with email (openid#397) * Use CustomTabsService.ACTION_CUSTOM_TABS_CONNECTION in BrowserSelector. (openid#411) * This helps with the jetification. I produced an artifact locally, and wrote a sample app using: ```gradle implementation 'androidx.appcompat:appcompat:1.0.0-rc02' implementation 'androidx.browser:browser:1.0.0-rc02' ``` to make sure that the app works. Fixes: openid#395 * Issue openid#402: fixes NPE in case input stream is null (openid#414) * Fix spelling mistake * Update link to custom tabs documentation * Fix typo in README.md (openid#455) * Fix `Using access tokens` snippet in the readme (openid#510) The snippet code in the `Using access tokens` section didn't follow java code formatting. * Use capitalized form for "Bearer" token type While this is case-insensitive, some provider implementations do not correctly handle the lower case form. * Migrate to AndroidX & Update dependencies (openid#508) * gradle update to 5.4.1 * gradle plugin update to 3.5.3 * minor bintray update * bump compile sdk to 29 - this requires bumping build tools to 29 - also updating the gradlewrapper (again, fully) to 5.4.1 * Migrate to AndroidX This required changes to checkstyle & javadocs gradle files (perhaps because of jetifier? not sure tbh) Also mockito now can mock all final classes * Fix compiler warning * update travis build tools to 29.0.2 to align with build.gradle * update travis android sdk version to 29 to align with build.gradle * Android11 support (openid#558) - Migrated from the deprecated Android Support libraries to the newest versions of the AndroidX libraries. - The Android gradle plugin is updated to the current 4.0.0. - Glide is updated to the current 4.11.0, and 4.1x.x is needed for AndroidX support. - limit the allowed queries to only fullbrowsers - openid#554 . - Add support for CATEGORY_APP_BROWSER. - Update dependencies * Deprecate whitelist and blacklist (openid#555) * Replace BrowserBlacklist with BrowserDenyList and BrowserWhitelist with BrowserAllowList * Add xmlpull library dependency for tests because some of the unit tests fail without it. * Override constructors and add a deprecation migration test * Update library version to 0.8.0-skyscanner * Fix build after upgrading gradle plugin (as result of syncing up with openid repo) Co-authored-by: Iain McGinniss <iainmcgin@gmail.com> Co-authored-by: Iain McGinniss <iain.mcginniss@wework.com> Co-authored-by: Henning Lund <40355805+hnljp@users.noreply.github.com> Co-authored-by: Øyvind Robertsen <oyvindrobertsen@gmail.com> Co-authored-by: Rahul Ravikumar <tikurahul@users.noreply.github.com> Co-authored-by: Thomas Rebouillon <thomas.rebouillon@gmail.com> Co-authored-by: Mark Kowalski <mark.kowalski86@googlemail.com> Co-authored-by: Ahmad El-Melegy <ahmad.melegy@gmail.com> Co-authored-by: Paul Blundell <blundell.app.dev@gmail.com> Co-authored-by: Jon F Hancock <jonfhancock@gmail.com> Co-authored-by: Athila Santos <athilahs>

Respect default browser of the user

9e1f33e

a possible default browser of the user is respected. fixes: openid#372 split from openid#369 Signed-off-by: Henning Nielsen Lund <henning.n.lund@jp.dk>

hnljp mentioned this pull request Jul 20, 2018

Add support for custom tabs in Firefox #369

Closed

hnljp mentioned this pull request Jul 20, 2018

Make it possible to create a CustomTabsIntent, where the package name is set. #380

Closed

Firefox customtabs (openid#378)

e1671e7

Add support for custom tabs in Firefox. Support for custom tabs was added to stable Firefox in version 57.0 Fixes: openid#368 Signed-off-by: Henning Nielsen Lund <henning.n.lund@jp.dk>

iainmcgin requested changes Jul 27, 2018

View reviewed changes

hnljp added 6 commits July 27, 2018 23:33

Respect default browser of the user

edf7051

a possible default browser of the user is respected. fixes: openid#372 split from openid#369 Signed-off-by: Henning Nielsen Lund <henning.n.lund@jp.dk>

Merge branch 'default-browser' of https://github.com/hnljp/AppAuth-An…

02b1515

…droid into default-browser

Add description by iainmcgin

e6e7d78

openid#379 (comment) Signed-off-by: Henning Nielsen Lund <henning.n.lund@jp.dk>

Add tests

1a075f1

Add tests for when a default browser is set. Signed-off-by: Henning Nielsen Lund <henning.n.lund@jp.dk>

modify test

f2dc2dc

The test is now also checking if the BrowserDesriptors are added in the correct order, by checking useCustomTab. Signed-off-by: Henning Nielsen Lund <henning.n.lund@jp.dk>

modify test

48100cd

Now also check if there is no browser supporting custom tabs, but a browser is set as the default browser, so it should be returned. Signed-off-by: Henning Nielsen Lund <henning.n.lund@jp.dk>

WilliamDenniss added the openid-cla: yes label Jul 30, 2018

iainmcgin approved these changes Aug 7, 2018

View reviewed changes

iainmcgin merged commit a42f28a into openid:master Aug 7, 2018

hnljp deleted the default-browser branch August 7, 2018 19:14

Hkoma95 approved these changes Aug 30, 2024

View reviewed changes

Sunbelt Computer Software

PL/B Language Development and Support

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Respect default browser of the user#379

Respect default browser of the user#379
iainmcgin merged 8 commits into
openid:masterfrom
hnljp:default-browser

hnljp commented Jul 20, 2018

Uh oh!

codecov-io commented Jul 20, 2018 •

edited

Loading

Uh oh!

hnljp commented Jul 20, 2018

Uh oh!

hnljp commented Jul 20, 2018

Uh oh!

iainmcgin left a comment

Uh oh!

iainmcgin Jul 27, 2018

Uh oh!

iainmcgin Jul 27, 2018

Uh oh!

hnljp Jul 28, 2018

Uh oh!

hnljp commented Aug 4, 2018

Uh oh!

iainmcgin left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Conversation

hnljp commented Jul 20, 2018

Uh oh!

codecov-io commented Jul 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

hnljp commented Jul 20, 2018

Uh oh!

hnljp commented Jul 20, 2018

Uh oh!

iainmcgin left a comment

Choose a reason for hiding this comment

Uh oh!

iainmcgin Jul 27, 2018

Choose a reason for hiding this comment

Uh oh!

iainmcgin Jul 27, 2018

Choose a reason for hiding this comment

Uh oh!

hnljp Jul 28, 2018

Choose a reason for hiding this comment

Uh oh!

hnljp commented Aug 4, 2018

Uh oh!

iainmcgin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-io commented Jul 20, 2018 •

edited

Loading