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

Using PKCE flow for Gemini Authentication, adding shared oauth utility #6306

Merged
merged 3 commits into from
Aug 3, 2020

Conversation

ryanml
Copy link
Contributor

@ryanml ryanml commented Jul 31, 2020

Fixes brave/brave-browser#11043

Submitter Checklist:

Test Plan:

General authentication regression for Binance and Gemini

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@ryanml ryanml added this to the 1.14.x - Nightly milestone Jul 31, 2020
@ryanml ryanml requested review from bbondy and emerick July 31, 2020 22:46
@ryanml ryanml self-assigned this Jul 31, 2020
@@ -312,6 +312,12 @@ source_set("browser_process") {
]
}

if (gemini_enabled || binance_enabled) {
deps += [
"//brave/components/crypto_exchange/browser",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can use this module for more code sharing utilities

crypto::kSHA256Length),
&code_challenge);

if (strip_chars) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This works for Binance, but Gemini expects characters preserved so these steps are optional

browser/BUILD.gn Outdated Show resolved Hide resolved
Copy link
Contributor

@emerick emerick left a comment

Choose a reason for hiding this comment

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

LGTM

@ryanml ryanml force-pushed the gemini-pkce branch 3 times, most recently from f0c72f1 to 8b57ee4 Compare August 1, 2020 02:36
@ryanml ryanml merged commit 83c7844 into master Aug 3, 2020
@ryanml ryanml deleted the gemini-pkce branch August 3, 2020 21:53
ryanml added a commit that referenced this pull request Aug 3, 2020
Using PKCE flow for Gemini Authentication, adding shared oauth utility
ryanml added a commit that referenced this pull request Aug 7, 2020
Using PKCE flow for Gemini Authentication, adding shared oauth utility
ryanml added a commit that referenced this pull request Aug 15, 2020
Using PKCE flow for Gemini Authentication, adding shared oauth utility
@kjozwiak
Copy link
Member

Verification PASSED on macOS 10.15.6 x64 using the following build:

Brave | 1.14.44 Chromium: 85.0.4183.69 (Official Build) nightly (64-bit)
-- | --
Revision | 4554ea1a1171bd8d06951a4b7d9336afe6c59967-refs/branch-heads/4183@{#1426}
OS | macOS Version 10.15.6 (Build 19G73)

Went through the following cases via the Binance widget as per #6306 (comment):

  • ensured that you can connect without any issues and the Summary, Deposit, Convert and Buy tabs are displayed
  • ensured that disconnecting/reconnecting via the widget worked as expected and displayed the correct blanaces
  • ensured that deposit addresses are being displayed correctly under the Deposit tab
  • ensured that clicking on the QR code images under the Deposit tab correctly enlarges/displays the QR codes
  • ensured that tokens such as HBAR display both a deposit address and memo address
  • ensured that selecting Learn More under the kabob/settings menu opened https://brave.com/binance
  • ensured that selecting tokens via the Buy tab correctly updates the Buy [Token] button
  • ensured that clicking on the Buy button under the Buy tab correctly opens a new tab with the correct amounts already filled out

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.

Update Gemini widget Oauth flow to use PKCE
4 participants