Skip to content

Javascript Binding - Add ability to limit access to JavaScript Bound objects to specific origins #5085

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

amaitland
Copy link
Member

Fixes:
#5001

Summary:

  • Add new JavascriptBindingSettings.JavascriptBindingApiAllowOrigins property

Changes:

  • Don't create CefSharp objects when non matching origin
  • Doesn't currently impact Legacy binding
  • Added xunit tests

How Has This Been Tested?

  • New xUnit tests have been added

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Updated documentation

Checklist:

  • Tested the code(if applicable)
  • Commented my code
  • Changed the documentation(if applicable)
  • New files have a license disclaimer
  • The formatting is consistent with the project (project supports .editorconfig)

@amaitland amaitland self-assigned this Apr 12, 2025
cefSharpObj->SetValue(kRenderProcessId, CefV8Value::CreateInt(processId), CefV8Value::PropertyAttribute::V8_PROPERTY_ATTRIBUTE_NONE);

global->SetValue(_jsBindingPropertyName, cefSharpObj, CefV8Value::PropertyAttribute::V8_PROPERTY_ATTRIBUTE_READONLY);
createObjects = false;
Copy link
Member Author

Choose a reason for hiding this comment

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

Extract this into a helper method would probably make this a lot cleaner

@@ -33,6 +35,24 @@ public bool JavascriptBindingApiEnabled
}
}

/// <summary>
/// When <see cref="JavascriptBindingApiEnabled"/> is set to true, set a collection
Copy link
Member Author

Choose a reason for hiding this comment

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

Needs review

@@ -95,5 +115,17 @@ public bool AlwaysInterceptAsynchronously
alwaysInterceptAsynchronously = value;
}
}

/// <summary>
/// HasJavascriptBindingApiAllowOrigins
Copy link
Member Author

Choose a reason for hiding this comment

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

Needs review.

@amaitland
Copy link
Member Author

Could probably use some more tests, specifically for multiple origins in the list

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@amaitland amaitland force-pushed the feature/jsbindingalloworiginlist branch from b98cfbe to 218de9c Compare April 12, 2025 04:36
@amaitland amaitland force-pushed the feature/jsbindingalloworiginlist branch from 218de9c to 60203e5 Compare April 12, 2025 04:58

auto clrOrigin = StringUtils::ToClr(origin);

auto originEqual = String::Compare(clrframeUrlOrigin, clrOrigin, StringComparison::InvariantCultureIgnoreCase);
Copy link
Member Author

Choose a reason for hiding this comment

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

Need to find a more efficient string comparison, ideally compare the native strings

@AppVeyorBot
Copy link

/// <remarks>
/// If you wish to create the CefSharp object for a limited set of origins then set this property
/// </remarks>
public string[] JavascriptBindingApiAllowOrigins
Copy link
Member Author

Choose a reason for hiding this comment

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

The origin returned from CEF when it's parsed always has a trailing /, we should ensure that there's always a trailing slash programmatically

@@ -29,6 +29,8 @@ namespace CefSharp
bool _focusedNodeChangedEnabled;
bool _legacyBindingEnabled;
bool _jsBindingApiEnabled = true;
Copy link
Member Author

Choose a reason for hiding this comment

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

Storing these on a per browser basis would provide greater flexibility, we could create a class that has a lookup by browser id.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants