Skip to content

fix: grant camera permission based on origin#36

Merged
mzndako merged 4 commits intomasterfrom
mzndako/fix/camera-permission-whitelist
Feb 21, 2025
Merged

fix: grant camera permission based on origin#36
mzndako merged 4 commits intomasterfrom
mzndako/fix/camera-permission-whitelist

Conversation

@mzndako
Copy link
Copy Markdown

@mzndako mzndako commented Feb 20, 2025

Addressing some improve feedback from this PR

@mzndako mzndako added the enhancement New feature or request label Feb 20, 2025
@mzndako mzndako self-assigned this Feb 20, 2025
Copy link
Copy Markdown

@633kh4ck 633kh4ck left a comment

Choose a reason for hiding this comment

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

nACK

@633kh4ck 633kh4ck changed the title fix: grant camera permission base on origin fix: grant camera permission based on origin Feb 20, 2025
return;
NSString *originString = [NSString stringWithFormat:@"%@://%@", origin.protocol, origin.host];

if (origin.port > 0 && (([origin.protocol isEqualToString:@"http"] && origin.port != 80) || ([origin.protocol isEqualToString:@"https"] && origin.port != 443))) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

TODO: test if protocol is normalized, i.e., ifHttP is treated as http, just in case

Copy link
Copy Markdown
Author

@mzndako mzndako Feb 20, 2025

Choose a reason for hiding this comment

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

Does this actually matter? If HttP is treated as http, the constructed originString won’t match the check below and will be denied.

// Example:
// originString: 'HttPs://camerasite.com'
// cameraPermissionOriginWhitelist: ['https://camerasite.com'] 
if ([self.cameraPermissionOriginWhitelist containsObject:originString]) { // wouldn't match hence denied

I think it’s fine for this case to result in denial, so there’s no need to normalize the protocol.

Copy link
Copy Markdown

@633kh4ck 633kh4ck left a comment

Choose a reason for hiding this comment

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

utACK

@mzndako mzndako merged commit a9e394f into master Feb 21, 2025
@mzndako mzndako deleted the mzndako/fix/camera-permission-whitelist branch February 21, 2025 13:03
@joshua-rogers-exodus
Copy link
Copy Markdown

joshua-rogers-exodus commented Sep 5, 2025

post-merge nACK

https://github.com/exodusmovement/react-native-webview/blob/e609accac4f4f36754ad480f81454a7ad5d2598f/android/src/main/java/com/reactnativecommunity/webview/RNCWebViewManager.java#L1076-L1085

For port condition check, this is always going to fail for scheme == "http" because == in java compares object references, not values, so the origin is always going to remain without the port.

fixed in #46

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants