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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions browser/BUILD.gn
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import("//brave/build/config.gni")
import("//brave/components/binance/browser/buildflags/buildflags.gni")
import("//brave/components/brave_together/buildflags/buildflags.gni")
import("//brave/components/crypto_exchange/browser/buildflags/buildflags.gni")
import("//brave/components/gemini/browser/buildflags/buildflags.gni")
import("//brave/browser/tor/buildflags/buildflags.gni")
import("//brave/components/brave_ads/browser/buildflags/buildflags.gni")
Expand Down Expand Up @@ -312,6 +313,12 @@ source_set("browser_process") {
]
}

if (crypto_exchanges_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

]
}

if (brave_together_enabled) {
sources += [
"brave_together/brave_together_util.cc",
Expand Down
1 change: 1 addition & 0 deletions components/binance/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ source_set("browser") {
deps = [
"//base",
"//brave/common:pref_names",
"//brave/components/crypto_exchange/browser",
"//components/country_codes:country_codes",
"//components/keyed_service/content",
"//components/keyed_service/core",
Expand Down
39 changes: 3 additions & 36 deletions components/binance/browser/binance_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,20 @@
#include "base/containers/flat_set.h"
#include "base/files/file_enumerator.h"
#include "base/files/file_util.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/utf_string_conversions.h"
#include "base/task/post_task.h"
#include "base/task_runner_util.h"
#include "base/time/time.h"
#include "base/token.h"
#include "brave/common/pref_names.h"
#include "brave/components/binance/browser/binance_json_parser.h"
#include "brave/components/crypto_exchange/browser/crypto_exchange_oauth_util.h"
#include "components/country_codes/country_codes.h"
#include "components/os_crypt/os_crypt.h"
#include "components/prefs/pref_service.h"
#include "components/user_prefs/user_prefs.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/storage_partition.h"
#include "crypto/random.h"
#include "crypto/sha2.h"
#include "net/base/load_flags.h"
#include "net/base/url_util.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
Expand Down Expand Up @@ -72,15 +70,6 @@ GURL GetURLWithPath(const std::string& host, const std::string& path) {
return GURL(std::string(url::kHttpsScheme) + "://" + host).Resolve(path);
}

std::string GetHexEncodedCryptoRandomSeed() {
const size_t kSeedByteLength = 32;
// crypto::RandBytes is fail safe.
uint8_t random_seed_bytes[kSeedByteLength];
crypto::RandBytes(random_seed_bytes, kSeedByteLength);
return base::HexEncode(
reinterpret_cast<char*>(random_seed_bytes), kSeedByteLength);
}

} // namespace

BinanceService::BinanceService(content::BrowserContext* context)
Expand All @@ -98,36 +87,14 @@ BinanceService::BinanceService(content::BrowserContext* context)
BinanceService::~BinanceService() {
}

// static
std::string BinanceService::GetCodeChallenge(const std::string& code_verifier) {
std::string code_challenge;
char raw[crypto::kSHA256Length] = {0};
crypto::SHA256HashString(code_verifier,
raw,
crypto::kSHA256Length);
base::Base64Encode(base::StringPiece(raw,
crypto::kSHA256Length),
&code_challenge);

// Binance expects the following conversions for the base64 encoded value:
std::replace(code_challenge.begin(), code_challenge.end(), '+', '-');
std::replace(code_challenge.begin(), code_challenge.end(), '/', '_');
code_challenge.erase(std::find_if(code_challenge.rbegin(),
code_challenge.rend(), [](int ch) {
return ch != '=';
}).base(), code_challenge.end());

return code_challenge;
}

std::string BinanceService::GetOAuthClientUrl() {
// The code_challenge_ value is derived from the code_verifier value.
// Step 1 of the oauth process uses the code_challenge_ value.
// Step 4 of the oauth process uess the code_verifer_.
// We never need to persist these values, they are just used to get an
// access token.
code_verifier_ = GetHexEncodedCryptoRandomSeed();
code_challenge_ = GetCodeChallenge(code_verifier_);
code_verifier_ = crypto_exchange::GetCryptoRandomString(true);
code_challenge_ = crypto_exchange::GetCodeChallenge(code_verifier_, true);

GURL url(oauth_url);
url = net::AppendQueryParameter(url, "response_type", "code");
Expand Down
1 change: 0 additions & 1 deletion components/binance/browser/binance_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ class BinanceService : public KeyedService {

std::string GetBinanceTLD();
std::string GetOAuthClientUrl();
static std::string GetCodeChallenge(const std::string& code_verifier);
void SetAuthToken(const std::string& auth_token);

private:
Expand Down
9 changes: 0 additions & 9 deletions components/binance/browser/binance_service_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -457,15 +457,6 @@ class BinanceAPIBrowserTest : public InProcessBrowserTest {
std::unique_ptr<net::EmbeddedTestServer> https_server_;
};


IN_PROC_BROWSER_TEST_F(BinanceAPIBrowserTest, GetCodeChallenge) {
std::string verifier =
"FA87A1758E149A8BCD3A6D43DEAFAA013BCE2F132639ADA66C5BF101";
ASSERT_EQ(
"1vw-WOmdXSW7OHQPgnuMsZjhaQKxi3LO5L7uX0YEtHs",
BinanceService::GetCodeChallenge(verifier));
}

IN_PROC_BROWSER_TEST_F(BinanceAPIBrowserTest, GetOAuthClientURL) {
EXPECT_TRUE(NavigateToNewTabUntilLoadStop());
auto* service = GetBinanceService();
Expand Down
18 changes: 18 additions & 0 deletions components/crypto_exchange/browser/BUILD.gn
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import("//brave/components/crypto_exchange/browser/buildflags/buildflags.gni")

assert(crypto_exchanges_enabled)

source_set("browser") {
# Remove when https://github.com/brave/brave-browser/issues/10640 is resolved
check_includes = false

sources = [
"crypto_exchange_oauth_util.cc",
"crypto_exchange_oauth_util.h",
]

deps = [
"//base",
"//crypto",
]
}
9 changes: 9 additions & 0 deletions components/crypto_exchange/browser/buildflags/BUILD.gn
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import("//build/buildflag_header.gni")
import("//brave/components/crypto_exchange/browser/buildflags/buildflags.gni")

buildflag_header("buildflags") {
header = "buildflags.h"
flags = [
"CRYPTO_EXCHANGES_ENABLED=$crypto_exchanges_enabled",
]
}
7 changes: 7 additions & 0 deletions components/crypto_exchange/browser/buildflags/buildflags.gni
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import("//brave/build/config.gni")
import("//brave/components/binance/browser/buildflags/buildflags.gni")
import("//brave/components/gemini/browser/buildflags/buildflags.gni")

declare_args() {
crypto_exchanges_enabled = binance_enabled || gemini_enabled
}
57 changes: 57 additions & 0 deletions components/crypto_exchange/browser/crypto_exchange_oauth_util.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/* Copyright (c) 2020 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include <string>

#include "brave/components/crypto_exchange/browser/crypto_exchange_oauth_util.h"

#include "base/base64.h"
#include "base/strings/string_number_conversions.h"
#include "crypto/random.h"
#include "crypto/sha2.h"

namespace crypto_exchange {

std::string GetCryptoRandomString(bool hex_encode) {
const size_t kSeedByteLength = 32;
uint8_t random_seed_bytes[kSeedByteLength];
crypto::RandBytes(random_seed_bytes, kSeedByteLength);

if (!hex_encode) {
std::string encoded_string;
base::Base64Encode(
reinterpret_cast<char*>(random_seed_bytes), &encoded_string);
return encoded_string;
}

return base::HexEncode(
reinterpret_cast<char*>(random_seed_bytes), kSeedByteLength);
}

std::string GetCodeChallenge(
const std::string& code_verifier, bool strip_chars) {
std::string code_challenge;
char raw[crypto::kSHA256Length] = {0};
crypto::SHA256HashString(code_verifier,
raw,
crypto::kSHA256Length);
base::Base64Encode(base::StringPiece(raw,
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

std::replace(code_challenge.begin(), code_challenge.end(), '+', '-');
std::replace(code_challenge.begin(), code_challenge.end(), '/', '_');

code_challenge.erase(std::find_if(code_challenge.rbegin(),
code_challenge.rend(), [](int ch) {
return ch != '=';
}).base(), code_challenge.end());
}

return code_challenge;
}

} // namespace crypto_exchange
20 changes: 20 additions & 0 deletions components/crypto_exchange/browser/crypto_exchange_oauth_util.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/* Copyright (c) 2020 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_COMPONENTS_CRYPTO_EXCHANGE_BROWSER_CRYPTO_EXCHANGE_OAUTH_UTIL_H_
#define BRAVE_COMPONENTS_CRYPTO_EXCHANGE_BROWSER_CRYPTO_EXCHANGE_OAUTH_UTIL_H_

#include <string>

namespace crypto_exchange {

std::string GetCryptoRandomString(bool hex_encode);

std::string GetCodeChallenge(
const std::string& code_verifier, bool strip_chars);

}

#endif // BRAVE_COMPONENTS_CRYPTO_EXCHANGE_BROWSER_CRYPTO_EXCHANGE_OAUTH_UTIL_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/* Copyright (c) 2020 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/components/crypto_exchange/browser/crypto_exchange_oauth_util.h"

#include "chrome/test/base/chrome_render_view_host_test_harness.h"

// npm run test -- brave_unit_tests --filter=CryptoExchangeOauthUtilTest.*

namespace {

typedef testing::Test CryptoExchangeOauthUtilTest;

TEST_F(CryptoExchangeOauthUtilTest, GetCodeChallengeStripChars) {
std::string verifier =
"FA87A1758E149A8BCD3A6D43DEAFAA013BCE2F132639ADA66C5BF101";
ASSERT_EQ(
"1vw-WOmdXSW7OHQPgnuMsZjhaQKxi3LO5L7uX0YEtHs",
crypto_exchange::GetCodeChallenge(verifier, true));
}

TEST_F(CryptoExchangeOauthUtilTest, GetCodeChallengeNoStripChars) {
std::string verifier =
"aGVsbG9fd29ybGRfdGhpc19pc19hX3Rlc3Q=";
ASSERT_EQ(
"mTWSN0meBbs9rauVM4rSmWDYVKTWFhkFeECqn6W2ZC0=",
crypto_exchange::GetCodeChallenge(verifier, false));
}

} // namespace
1 change: 1 addition & 0 deletions components/gemini/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ source_set("browser") {

deps = [
"//base",
"//brave/components/crypto_exchange/browser",
"//components/prefs:prefs",
"//components/keyed_service/content",
"//components/keyed_service/core",
Expand Down
22 changes: 10 additions & 12 deletions components/gemini/browser/gemini_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,19 @@
#include "base/files/file_enumerator.h"
#include "base/files/file_util.h"
#include "base/json/json_writer.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/utf_string_conversions.h"
#include "base/task/post_task.h"
#include "base/task_runner_util.h"
#include "base/time/time.h"
#include "base/token.h"
#include "brave/components/crypto_exchange/browser/crypto_exchange_oauth_util.h"
#include "brave/components/gemini/browser/gemini_json_parser.h"
#include "brave/components/gemini/browser/pref_names.h"
#include "components/os_crypt/os_crypt.h"
#include "components/prefs/pref_service.h"
#include "components/user_prefs/user_prefs.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/storage_partition.h"
#include "crypto/random.h"
#include "net/base/load_flags.h"
#include "net/base/url_util.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
Expand Down Expand Up @@ -106,15 +105,6 @@ namespace {
return encoded_payload;
}

std::string GetEncodedCryptoRandomCSRF() {
uint8_t random_seed_bytes[24];
crypto::RandBytes(random_seed_bytes, 24);
std::string encoded_csrf;
base::Base64Encode(
reinterpret_cast<char*>(random_seed_bytes), &encoded_csrf);
return encoded_csrf;
}

} // namespace

GeminiService::GeminiService(content::BrowserContext* context)
Expand All @@ -135,11 +125,16 @@ GeminiService::~GeminiService() {

std::string GeminiService::GetOAuthClientUrl() {
GURL url(oauth_url);
code_verifier_ = crypto_exchange::GetCryptoRandomString(false);
code_challenge_ = crypto_exchange::GetCodeChallenge(code_verifier_, false);
url = net::AppendQueryParameter(url, "response_type", "code");
url = net::AppendQueryParameter(url, "client_id", client_id_);
url = net::AppendQueryParameter(url, "redirect_uri", oauth_callback);
url = net::AppendQueryParameter(url, "scope", oauth_scope);
url = net::AppendQueryParameter(url, "state", GetEncodedCryptoRandomCSRF());
url = net::AppendQueryParameter(url, "code_challenge", code_challenge_);
url = net::AppendQueryParameter(url, "code_challenge_method", "S256");
url = net::AppendQueryParameter(
url, "state", ::crypto_exchange::GetCryptoRandomString(false));
return url.spec();
}

Expand All @@ -157,6 +152,7 @@ bool GeminiService::GetAccessToken(AccessTokenCallback callback) {
dict.SetStringKey("client_secret", client_secret_);
dict.SetStringKey("code", auth_token_);
dict.SetStringKey("redirect_uri", oauth_callback);
dict.SetStringKey("code_verifier", code_verifier_);
dict.SetStringKey("grant_type", "authorization_code");
std::string request_body = CreateJSONRequestBody(dict);

Expand Down Expand Up @@ -278,6 +274,8 @@ void GeminiService::OnRevokeAccessToken(
const std::map<std::string, std::string>& headers) {
bool success = status >= 200 && status <= 299;
if (success) {
code_challenge_ = "";
code_verifier_ = "";
ResetAccessTokens();
}
std::move(callback).Run(success);
Expand Down
2 changes: 2 additions & 0 deletions components/gemini/browser/gemini_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ class GeminiService : public KeyedService {
std::string auth_token_;
std::string access_token_;
std::string refresh_token_;
std::string code_challenge_;
std::string code_verifier_;
std::string client_id_;
std::string client_secret_;
std::string oauth_host_;
Expand Down
6 changes: 6 additions & 0 deletions components/gemini/browser/gemini_service_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -314,11 +314,17 @@ IN_PROC_BROWSER_TEST_F(GeminiAPIBrowserTest, GetOAuthClientURL) {
"client_id=fake-client-id&"
"redirect_uri=com.brave.gemini%3A%2F%2Fauthorization&"
"scope=addresses%3Aread%2Cbalances%3Aread%2Corders%3Acreate&"
ryanml marked this conversation as resolved.
Show resolved Hide resolved
"code_challenge=da0KASk6XZX4ksgvIGAa87iwNSVvmWdys2GYh3kjBZw&"
"code_challenge_method=S256&"
"state=placeholder");
client_url = net::AppendOrReplaceQueryParameter(client_url, "state",
"fake-state");
client_url = net::AppendOrReplaceQueryParameter(client_url, "code_challenge",
"fake-challenge");
expected_url = net::AppendOrReplaceQueryParameter(expected_url,
"state", "fake-state");
expected_url = net::AppendOrReplaceQueryParameter(expected_url,
"code_challenge", "fake-challenge");
ASSERT_EQ(expected_url, client_url);
}

Expand Down
Loading