Fix: Removed use of service to open Custom Tabs

joaoeudes7 commented 3 years ago

Description:

Removed use of services to create a Custom tabs, the create service and bind is not available to all devices, this cause impact to open the custom tabs and not show errors about this.

Testing details:

[x] Verified basic functionality of change
[x] Added tests

Other considerations:

[ ] Has security implications
[ ] Has UX changes

RESOLVES:

OKTA-XXXXX - No permission to create or open

JayNewstrom commented 3 years ago

Thanks for the PR. Could you give us a little more information on the things you're fixing and the reasons why. It might be better to break these up into separate PRs.

marlonbrgomes commented 3 years ago

The code from @joaoeudes7 is solving the problem on most Xiaomi and Huawei devices, issue reference from here: #150

joaoeudes7 commented 3 years ago

Hi @JayNewstrom, these commits solves problems as #247, #150.

You can validate the features using:

    allprojects {
        repositories {
            ...
            maven { url 'https://jitpack.io' }
        }
    }

    dependencies {
            implementation 'com.github.klever-io:okta-oidc-android:1.0.19-klever1'
    }

JayNewstrom commented 3 years ago

Ok, thanks for the high level detail. I'll work on testing this. I'll likely have more questions for you.

This weekend is a long weekend for me due to holiday, so it will likely take longer than usual to provide feedback, with the eventual goal to pull all or some of this in.

Thanks again for taking the time to prepare the PR.

PS, could you make sure you've signed our CLA

JayNewstrom commented 3 years ago

I've created an internal task for this, OKTA-409616, I'll work on it this week.

@joaoeudes7 can you please confirm you've signed the CLA?

JayNewstrom commented 3 years ago

@ghsantos we'll need you to sign our CLA too, since you've got commits on this PR.

JayNewstrom commented 3 years ago

@joaoeudes7 looks like the build is failing due to checkstyle errors.

> Task :library:checkAllSource
[ant:checkstyle] [WARN] /home/travis/build/okta/okta-oidc-android/library/src/main/java/com/okta/oidc/Okta.java:147: Line is longer than 100 characters (found 103). [LineLength]
[ant:checkstyle] [WARN] /home/travis/build/okta/okta-oidc-android/library/src/main/java/com/okta/oidc/OktaAuthenticationActivity.java:0: File does not end with a newline. [NewlineAtEndOfFile]
[ant:checkstyle] [WARN] /home/travis/build/okta/okta-oidc-android/library/src/main/java/com/okta/oidc/OktaAuthenticationActivity.java:15: 'package' should be separated from previous statement. [EmptyLineSeparator]
[ant:checkstyle] [WARN] /home/travis/build/okta/okta-oidc-android/library/src/main/java/com/okta/oidc/OktaAuthenticationActivity.java:30: 'java.util.ArrayList' should be separated from previous import group by one line. [CustomImportOrder]
[ant:checkstyle] [WARN] /home/travis/build/okta/okta-oidc-android/library/src/main/java/com/okta/oidc/OktaAuthenticationActivity.java:35: 'com.okta.oidc.net.ConnectionParameters.USER_AGENT_HEADER' should be separated from previous import group by one line. [CustomImportOrder]
[ant:checkstyle] [WARN] /home/travis/build/okta/okta-oidc-android/library/src/main/java/com/okta/oidc/OktaAuthenticationActivity.java:35: 'import' should be separated from previous statement. [EmptyLineSeparator]
[ant:checkstyle] [WARN] /home/travis/build/okta/okta-oidc-android/library/src/main/java/com/okta/oidc/OktaAuthenticationActivity.java:208: Line is longer than 100 characters (found 102). [LineLength]
[ant:checkstyle] [WARN] /home/travis/build/okta/okta-oidc-android/library/src/main/java/com/okta/oidc/OktaAuthenticationActivity.java:213: Line is longer than 100 characters (found 108). [LineLength]
Checkstyle rule violations were found. See the report at: file:///home/travis/build/okta/okta-oidc-android/library/build/reports/checkstyle/checkAllSource.html
Checkstyle files with violations: 2
Checkstyle violations by severity: [warning:8]

joaoeudes7 commented 3 years ago

@joaoeudes7 looks like the build is failing due to checkstyle errors.

> Task :library:checkAllSource
[ant:checkstyle] [WARN] /home/travis/build/okta/okta-oidc-android/library/src/main/java/com/okta/oidc/Okta.java:147: Line is longer than 100 characters (found 103). [LineLength]
[ant:checkstyle] [WARN] /home/travis/build/okta/okta-oidc-android/library/src/main/java/com/okta/oidc/OktaAuthenticationActivity.java:0: File does not end with a newline. [NewlineAtEndOfFile]
[ant:checkstyle] [WARN] /home/travis/build/okta/okta-oidc-android/library/src/main/java/com/okta/oidc/OktaAuthenticationActivity.java:15: 'package' should be separated from previous statement. [EmptyLineSeparator]
[ant:checkstyle] [WARN] /home/travis/build/okta/okta-oidc-android/library/src/main/java/com/okta/oidc/OktaAuthenticationActivity.java:30: 'java.util.ArrayList' should be separated from previous import group by one line. [CustomImportOrder]
[ant:checkstyle] [WARN] /home/travis/build/okta/okta-oidc-android/library/src/main/java/com/okta/oidc/OktaAuthenticationActivity.java:35: 'com.okta.oidc.net.ConnectionParameters.USER_AGENT_HEADER' should be separated from previous import group by one line. [CustomImportOrder]
[ant:checkstyle] [WARN] /home/travis/build/okta/okta-oidc-android/library/src/main/java/com/okta/oidc/OktaAuthenticationActivity.java:35: 'import' should be separated from previous statement. [EmptyLineSeparator]
[ant:checkstyle] [WARN] /home/travis/build/okta/okta-oidc-android/library/src/main/java/com/okta/oidc/OktaAuthenticationActivity.java:208: Line is longer than 100 characters (found 102). [LineLength]
[ant:checkstyle] [WARN] /home/travis/build/okta/okta-oidc-android/library/src/main/java/com/okta/oidc/OktaAuthenticationActivity.java:213: Line is longer than 100 characters (found 108). [LineLength]
Checkstyle rule violations were found. See the report at: file:///home/travis/build/okta/okta-oidc-android/library/build/reports/checkstyle/checkAllSource.html
Checkstyle files with violations: 2
Checkstyle violations by severity: [warning:8]

I do not have access to checkstyle/checkstyle.xml with configs to validate these errors of lint

joaoeudes7 commented 3 years ago

I've created an internal task for this, OKTA-409616, I'll work on it this week.

@joaoeudes7 can you please confirm you've signed the CLA?

Sorry, but the idea of CLA is controversial, i don't wanna sign, you can Close this contribution if CLA is required, Thanks.

JayNewstrom commented 3 years ago

Sorry it's taken a while to get back to you. I do think this contribution is beneficial, and I'd like to see us be able to merge it.

I reached out to our legal team to see what we could do, and they said:

The CLA is used to protect the rights of you the Contributor, Okta, and Okta’s users. The CLA does not take away any right, title, or interest you have in your contribution and only assigns the rights specified in the CLA to Okta in order to allow Okta and Okta’s users to use your contribution.

If you'd be willing to sign the CLA, we can merge this PR, otherwise we have to close it. Thanks for putting forth the effort for this PR!

JayNewstrom commented 3 years ago

I'm going to close this given we haven't heard back. If you decide to change your mind on the CLA, please comment or reopen.

Thanks again for taking the time to prepare a PR!

okta / okta-oidc-android