Skip to content

Make HTTP origin value an instance member #12821

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.

Sign up for GitHub

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

Merged
JunielKatarn merged 6 commits into from
Mar 13, 2024
Merged

Make HTTP origin value an instance member #12821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f4a8bb9
Make origin an instance member
JunielKatarn Mar 12, 2024
ae7432e
Change files
JunielKatarn Mar 12, 2024
12f5d8e
Remove s_origin
JunielKatarn Mar 12, 2024
f6dd238
Remove redundant namespace
JunielKatarn Mar 12, 2024
5eadcaa
Merge branch 'main' of github.com:microsoft/react-native-windows into…
JunielKatarn Mar 12, 2024
ed1f285
Merge branch 'main' into OC/8822139/origprop
JunielKatarn Mar 12, 2024
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 change/react-native-windows-bc10ca83-c1a9-4f4a-ac53-6d54378c3aca.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Make HTTP origin value an instance member",
"packageName": "react-native-windows",
"email": "julio.rocha@microsoft.com",
"dependentChangeType": "patch"
}
10 changes: 2 additions & 8 deletions vnext/Desktop.UnitTests/OriginPolicyHttpFilterTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,23 +280,20 @@ TEST_CLASS (OriginPolicyHttpFilterTest) {
// Should implicitly set Content-Length and Content-Type
request.Content(HttpStringContent{L"PreflightContent"});

auto filter = winrt::make<OriginPolicyHttpFilter>(mockFilter);
auto filter = winrt::make<OriginPolicyHttpFilter>("http://somehost", mockFilter);
auto opFilter = filter.as<OriginPolicyHttpFilter>();

OriginPolicyHttpFilter::SetStaticOrigin("http://somehost");
try {
auto sendOp = opFilter->SendPreflightAsync(request);
sendOp.get();

auto response = sendOp.GetResults();
opFilter->ValidatePreflightResponse(request, response);

OriginPolicyHttpFilter::SetStaticOrigin({});
Assert::IsTrue(boost::iequals(
response.Headers().Lookup(L"Access-Control-Allow-Headers").c_str(),
L"ChangeMyCase, Content-Length, Content-Type"));
} catch (const winrt::hresult_error &e) {
OriginPolicyHttpFilter::SetStaticOrigin({});
Assert::Fail(e.message().c_str());
}
}
Expand All @@ -323,23 +320,20 @@ TEST_CLASS (OriginPolicyHttpFilterTest) {
// Should implicitly set Content-Length and Content-Type
request.Content(HttpStringContent{L"PreflightContent"});

auto filter = winrt::make<OriginPolicyHttpFilter>(mockFilter);
auto filter = winrt::make<OriginPolicyHttpFilter>("http://somehost", mockFilter);
auto opFilter = filter.as<OriginPolicyHttpFilter>();

OriginPolicyHttpFilter::SetStaticOrigin("http://somehost");
try {
auto sendOp = opFilter->SendPreflightAsync(request);
sendOp.get();

auto response = sendOp.GetResults();
opFilter->ValidatePreflightResponse(request, response);

OriginPolicyHttpFilter::SetStaticOrigin({});
Assert::AreEqual(
L"Authorization, Content-Length, Content-Type",
response.Headers().Lookup(L"Access-Control-Allow-Headers").c_str());
} catch (const winrt::hresult_error &e) {
OriginPolicyHttpFilter::SetStaticOrigin({});
Assert::Fail(e.message().c_str());
}
}
Expand Down
42 changes: 19 additions & 23 deletions vnext/Shared/Networking/OriginPolicyHttpFilter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <regex>

using std::set;
using std::string;
using std::wstring;

using winrt::hresult_error;
Expand Down Expand Up @@ -114,15 +115,6 @@ bool OriginPolicyHttpFilter::CaseInsensitiveComparer::operator()(const wstring &
/*static*/ set<const wchar_t *, OriginPolicyHttpFilter::CaseInsensitiveComparer>
OriginPolicyHttpFilter::s_corsForbiddenRequestHeaderNamePrefixes = {L"Proxy-", L"Sec-"};

/*static*/ Uri OriginPolicyHttpFilter::s_origin{nullptr};

/*static*/ void OriginPolicyHttpFilter::SetStaticOrigin(std::string &&url) {
if (!url.empty())
s_origin = Uri{to_hstring(url)};
else
s_origin = nullptr;
}

/*static*/ bool OriginPolicyHttpFilter::IsSameOrigin(Uri const &u1, Uri const &u2) noexcept {
return (u1 && u2) && u1.SchemeName() == u2.SchemeName() && u1.Host() == u2.Host() && u1.Port() == u2.Port();
}
Expand Down Expand Up @@ -387,10 +379,14 @@ bool OriginPolicyHttpFilter::CaseInsensitiveComparer::operator()(const wstring &
}
}

OriginPolicyHttpFilter::OriginPolicyHttpFilter(IHttpFilter const &innerFilter) : m_innerFilter{innerFilter} {}
OriginPolicyHttpFilter::OriginPolicyHttpFilter(string &&origin, IHttpFilter const &innerFilter)
: m_origin{nullptr}, m_innerFilter{innerFilter} {
if (!origin.empty())
m_origin = Uri{to_hstring(origin)};
}

OriginPolicyHttpFilter::OriginPolicyHttpFilter()
: OriginPolicyHttpFilter(winrt::Windows::Web::Http::Filters::HttpBaseProtocolFilter{}) {}
OriginPolicyHttpFilter::OriginPolicyHttpFilter(string &&origin)
: OriginPolicyHttpFilter(std::move(origin), winrt::Windows::Web::Http::Filters::HttpBaseProtocolFilter{}) {}

OriginPolicy OriginPolicyHttpFilter::ValidateRequest(HttpRequestMessage const &request) {
auto effectiveOriginPolicy =
Expand All @@ -400,17 +396,17 @@ OriginPolicy OriginPolicyHttpFilter::ValidateRequest(HttpRequestMessage const &r
return effectiveOriginPolicy;

case OriginPolicy::SameOrigin:
if (!IsSameOrigin(s_origin, request.RequestUri()))
if (!IsSameOrigin(m_origin, request.RequestUri()))
throw hresult_error{E_INVALIDARG, L"SOP (same-origin policy) is enforced"};
break;

case OriginPolicy::SimpleCrossOriginResourceSharing:
// Check for disallowed mixed content
if (GetRuntimeOptionBool("Http.BlockMixedContentSimpleCors") &&
s_origin.SchemeName() != request.RequestUri().SchemeName())
m_origin.SchemeName() != request.RequestUri().SchemeName())
throw hresult_error{E_INVALIDARG, L"The origin and request URLs must have the same scheme"};

if (IsSameOrigin(s_origin, request.RequestUri()))
if (IsSameOrigin(m_origin, request.RequestUri()))
// Same origin. Therefore, skip Cross-Origin handling.
effectiveOriginPolicy = OriginPolicy::SameOrigin;
else if (!IsSimpleCorsRequest(request))
Expand All @@ -426,7 +422,7 @@ OriginPolicy OriginPolicyHttpFilter::ValidateRequest(HttpRequestMessage const &r
// Example: On the Edge browser, an XHR request with the "Host" header set gets rejected as unsafe.
// https://fetch.spec.whatwg.org/#forbidden-header-name

if (s_origin.SchemeName() != request.RequestUri().SchemeName())
if (m_origin.SchemeName() != request.RequestUri().SchemeName())
throw hresult_error{E_INVALIDARG, L"The origin and request URLs must have the same scheme"};

if (!AreSafeRequestHeaders(request.Headers()))
Expand All @@ -435,7 +431,7 @@ OriginPolicy OriginPolicyHttpFilter::ValidateRequest(HttpRequestMessage const &r
if (s_forbiddenMethods.find(request.Method().ToString().c_str()) != s_forbiddenMethods.cend())
throw hresult_error{E_INVALIDARG, L"Request method not allowed in cross-origin resource sharing"};

if (IsSameOrigin(s_origin, request.RequestUri()))
if (IsSameOrigin(m_origin, request.RequestUri()))
effectiveOriginPolicy = OriginPolicy::SameOrigin;
else if (IsSimpleCorsRequest(request))
effectiveOriginPolicy = OriginPolicy::SimpleCrossOriginResourceSharing;
Expand Down Expand Up @@ -472,7 +468,7 @@ void OriginPolicyHttpFilter::ValidateAllowOrigin(
// 4.10.4 - Mismatched allow origin
auto taintedOriginProp = props.TryLookup(L"TaintedOrigin");
auto taintedOrigin = taintedOriginProp && winrt::unbox_value<bool>(taintedOriginProp);
auto origin = taintedOrigin ? nullptr : s_origin;
auto origin = taintedOrigin ? nullptr : m_origin;
if (allowedOrigin.empty() || !IsSameOrigin(origin, Uri{allowedOrigin})) {
hstring errorMessage;
if (allowedOrigin.empty())
Expand Down Expand Up @@ -603,7 +599,7 @@ void OriginPolicyHttpFilter::ValidateResponse(HttpResponseMessage const &respons
bool originAllowed = false;
for (const auto &header : response.Headers()) {
if (boost::iequals(header.Key(), L"Access-Control-Allow-Origin")) {
originAllowed |= L"*" == header.Value() || s_origin == Uri{header.Value()};
originAllowed |= L"*" == header.Value() || m_origin == Uri{header.Value()};
}
}

Expand Down Expand Up @@ -691,7 +687,7 @@ ResponseOperation OriginPolicyHttpFilter::SendPreflightAsync(HttpRequestMessage
}

preflightRequest.Headers().Insert(L"Access-Control-Request-Headers", headerNames);
preflightRequest.Headers().Insert(L"Origin", GetOrigin(s_origin));
preflightRequest.Headers().Insert(L"Origin", GetOrigin(m_origin));
preflightRequest.Headers().Insert(L"Sec-Fetch-Mode", L"CORS");

co_return {co_await m_innerFilter.SendRequestAsync(preflightRequest)};
Expand All @@ -708,7 +704,7 @@ bool OriginPolicyHttpFilter::OnRedirecting(
// origin=http://a.com. Since the origin matches the URL, the request is authorized at http://a.com, but it actually
// allows http://b.com to bypass the CORS check at http://a.com since the redirected URL is from http://b.com.
if (!IsSameOrigin(response.Headers().Location(), request.RequestUri()) &&
!IsSameOrigin(s_origin, request.RequestUri())) {
!IsSameOrigin(m_origin, request.RequestUri())) {
// By masking the origin field in the request header, we make it impossible for the server to set a single value for
// the access-control-allow-origin header. It means, the only way to support redirect is that server allows access
// from all sites through wildcard.
Expand Down Expand Up @@ -740,7 +736,7 @@ ResponseOperation OriginPolicyHttpFilter::SendRequestAsync(HttpRequestMessage co
// Allow only HTTP or HTTPS schemes
if (GetRuntimeOptionBool("Http.StrictScheme") && coRequest.RequestUri().SchemeName() != L"https" &&
coRequest.RequestUri().SchemeName() != L"http")
throw hresult_error{E_INVALIDARG, L"Invalid URL scheme: [" + s_origin.SchemeName() + L"]"};
throw hresult_error{E_INVALIDARG, L"Invalid URL scheme: [" + m_origin.SchemeName() + L"]"};

if (!GetRuntimeOptionBool("Http.OmitCredentials")) {
coRequest.Properties().Lookup(L"RequestArgs").as<RequestArgs>()->WithCredentials = false;
Expand Down Expand Up @@ -777,7 +773,7 @@ ResponseOperation OriginPolicyHttpFilter::SendRequestAsync(HttpRequestMessage co

if (originPolicy == OriginPolicy::SimpleCrossOriginResourceSharing ||
originPolicy == OriginPolicy::CrossOriginResourceSharing) {
coRequest.Headers().Insert(L"Origin", GetOrigin(s_origin));
coRequest.Headers().Insert(L"Origin", GetOrigin(m_origin));
}

auto response = co_await m_innerFilter.SendRequestAsync(coRequest);
Expand Down
11 changes: 4 additions & 7 deletions vnext/Shared/Networking/OriginPolicyHttpFilter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ class OriginPolicyHttpFilter
static std::set<const wchar_t *, CaseInsensitiveComparer> s_corsForbiddenRequestHeaderNamePrefixes;
static std::set<const wchar_t *, CaseInsensitiveComparer> s_cookieSettingResponseHeaders;

// NOTE: Assumes static origin through owning client/resource/module/(React) instance's lifetime.
static winrt::Windows::Foundation::Uri s_origin;

struct AccessControlValues {
winrt::hstring AllowedOrigin;
winrt::hstring AllowedCredentials;
Expand All @@ -49,11 +46,11 @@ class OriginPolicyHttpFilter
size_t MaxAge;
};

winrt::Windows::Foundation::Uri m_origin;

winrt::Windows::Web::Http::Filters::IHttpFilter m_innerFilter;

public:
static void SetStaticOrigin(std::string &&url);

static bool IsSameOrigin(
winrt::Windows::Foundation::Uri const &u1,
winrt::Windows::Foundation::Uri const &u2) noexcept;
Expand All @@ -80,9 +77,9 @@ class OriginPolicyHttpFilter
winrt::Windows::Web::Http::HttpResponseMessage const &response,
bool removeAll);

OriginPolicyHttpFilter(winrt::Windows::Web::Http::Filters::IHttpFilter const &innerFilter);
OriginPolicyHttpFilter(std::string &&origin, winrt::Windows::Web::Http::Filters::IHttpFilter const &innerFilter);

OriginPolicyHttpFilter();
OriginPolicyHttpFilter(std::string &&origin);

OriginPolicy ValidateRequest(winrt::Windows::Web::Http::HttpRequestMessage const &request);

Expand Down
6 changes: 2 additions & 4 deletions vnext/Shared/Networking/WinRTHttpResource.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -641,8 +641,7 @@ void WinRTHttpResource::AddResponseHandler(shared_ptr<IResponseHandler> response

#pragma region IHttpResource

/*static*/ shared_ptr<IHttpResource> IHttpResource::Make(
winrt::Windows::Foundation::IInspectable const &inspectableProperties) noexcept {
/*static*/ shared_ptr<IHttpResource> IHttpResource::Make(IInspectable const &inspectableProperties) noexcept {
using namespace winrt::Microsoft::ReactNative;
using winrt::Windows::Web::Http::HttpClient;

Expand All @@ -653,8 +652,7 @@ void WinRTHttpResource::AddResponseHandler(shared_ptr<IResponseHandler> response
client = HttpClient{redirFilter};
} else {
auto globalOrigin = GetRuntimeOptionString("Http.GlobalOrigin");
Copy link
Contributor

@acoates-ms acoates-ms Mar 12, 2024

Choose a reason for hiding this comment

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

We should add a public PropertyId, or a setter for ReactPropertyBag/ReactInstanceSettings to allow setting this option on a per instance basis. If someone wanted to have different origins for different RN instances there would still be a synchronization issue here.

We still need the "legacy" GetRuntimeOptionString option for Office's non-abi code paths, but for public use this should all be done using ReactInstanceSettings.

Copy link
Contributor Author

@JunielKatarn JunielKatarn Mar 12, 2024

Choose a reason for hiding this comment

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

Can that be addressed in a follow-up PR? (I could use that to re-factor other property ID declarations that are done inefficiently).

This fix is required ASAP.

Copy link
Contributor

@acoates-ms acoates-ms Mar 12, 2024

Choose a reason for hiding this comment

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

Sure

OriginPolicyHttpFilter::SetStaticOrigin(std::move(globalOrigin));
auto opFilter = winrt::make<OriginPolicyHttpFilter>(redirFilter);
auto opFilter = winrt::make<OriginPolicyHttpFilter>(std::move(globalOrigin), redirFilter);
redirFilter.as<RedirectHttpFilter>()->SetRedirectSource(opFilter.as<IRedirectEventSource>());

client = HttpClient{opFilter};
Expand Down