From 68da311ed1f5b37e3a86c90a9ed4570e13e98aa4 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Wed, 29 Aug 2018 02:05:08 +1200 Subject: [PATCH] feat: add session.setPermissionCheckHandler (#13925) * feat: add session.setPermissionCheckHandler to handle syncornous permission checks vs requests * spec: add tests for session.setPermissionCheckHandler * docs: add docs for session.setPermissionCheckHandler * feat: add mediaType to media permission checks * chore: cleanup check impl --- atom/browser/api/atom_api_session.cc | 14 +++++++ atom/browser/api/atom_api_session.h | 2 + atom/browser/api/atom_api_web_contents.cc | 4 +- atom/browser/atom_permission_manager.cc | 19 ++++++++++ atom/browser/atom_permission_manager.h | 11 ++++++ .../browser/web_contents_permission_helper.cc | 37 +++++++++++++++++++ atom/browser/web_contents_permission_helper.h | 8 ++++ docs/api/session.md | 26 +++++++++++++ spec/chromium-spec.js | 17 +++++++++ spec/static/main.js | 5 +++ 10 files changed, 142 insertions(+), 1 deletion(-) diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index 658f913bb..7b13b6abd 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -627,6 +627,18 @@ void Session::SetPermissionRequestHandler(v8::Local val, permission_manager->SetPermissionRequestHandler(handler); } +void Session::SetPermissionCheckHandler(v8::Local val, + mate::Arguments* args) { + AtomPermissionManager::CheckHandler handler; + if (!(val->IsNull() || mate::ConvertFromV8(args->isolate(), val, &handler))) { + args->ThrowError("Must pass null or function"); + return; + } + auto* permission_manager = static_cast( + browser_context()->GetPermissionManager()); + permission_manager->SetPermissionCheckHandler(handler); +} + void Session::ClearHostResolverCache(mate::Arguments* args) { base::Closure callback; args->GetNext(&callback); @@ -814,6 +826,8 @@ void Session::BuildPrototype(v8::Isolate* isolate, .SetMethod("setCertificateVerifyProc", &Session::SetCertVerifyProc) .SetMethod("setPermissionRequestHandler", &Session::SetPermissionRequestHandler) + .SetMethod("setPermissionCheckHandler", + &Session::SetPermissionCheckHandler) .SetMethod("clearHostResolverCache", &Session::ClearHostResolverCache) .SetMethod("clearAuthCache", &Session::ClearAuthCache) .SetMethod("allowNTLMCredentialsForDomains", diff --git a/atom/browser/api/atom_api_session.h b/atom/browser/api/atom_api_session.h index 6c8247c5c..3b5a0987e 100644 --- a/atom/browser/api/atom_api_session.h +++ b/atom/browser/api/atom_api_session.h @@ -75,6 +75,8 @@ class Session : public mate::TrackableObject, void SetCertVerifyProc(v8::Local proc, mate::Arguments* args); void SetPermissionRequestHandler(v8::Local val, mate::Arguments* args); + void SetPermissionCheckHandler(v8::Local val, + mate::Arguments* args); void ClearHostResolverCache(mate::Arguments* args); void ClearAuthCache(mate::Arguments* args); void AllowNTLMCredentialsForDomains(const std::string& domains); diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index f23f49f5a..21e21ce77 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -718,7 +718,9 @@ void WebContents::FindReply(content::WebContents* web_contents, bool WebContents::CheckMediaAccessPermission(content::WebContents* web_contents, const GURL& security_origin, content::MediaStreamType type) { - return true; + auto* permission_helper = + WebContentsPermissionHelper::FromWebContents(web_contents); + return permission_helper->CheckMediaAccessPermission(security_origin, type); } void WebContents::RequestMediaAccessPermission( diff --git a/atom/browser/atom_permission_manager.cc b/atom/browser/atom_permission_manager.cc index 06d418f04..7d0e01e7b 100644 --- a/atom/browser/atom_permission_manager.cc +++ b/atom/browser/atom_permission_manager.cc @@ -100,6 +100,11 @@ void AtomPermissionManager::SetPermissionRequestHandler( request_handler_ = handler; } +void AtomPermissionManager::SetPermissionCheckHandler( + const CheckHandler& handler) { + check_handler_ = handler; +} + int AtomPermissionManager::RequestPermission( content::PermissionType permission, content::RenderFrameHost* render_frame_host, @@ -223,4 +228,18 @@ int AtomPermissionManager::SubscribePermissionStatusChange( void AtomPermissionManager::UnsubscribePermissionStatusChange( int subscription_id) {} +bool AtomPermissionManager::CheckPermissionWithDetails( + content::PermissionType permission, + content::RenderFrameHost* render_frame_host, + const GURL& requesting_origin, + const base::DictionaryValue* details) const { + if (check_handler_.is_null()) { + return true; + } + auto* web_contents = + content::WebContents::FromRenderFrameHost(render_frame_host); + return check_handler_.Run(web_contents, permission, requesting_origin, + *details); +} + } // namespace atom diff --git a/atom/browser/atom_permission_manager.h b/atom/browser/atom_permission_manager.h index de2383733..3dd9c2988 100644 --- a/atom/browser/atom_permission_manager.h +++ b/atom/browser/atom_permission_manager.h @@ -31,9 +31,14 @@ class AtomPermissionManager : public content::PermissionManager { content::PermissionType, const StatusCallback&, const base::DictionaryValue&)>; + using CheckHandler = base::Callback; // Handler to dispatch permission requests in JS. void SetPermissionRequestHandler(const RequestHandler& handler); + void SetPermissionCheckHandler(const CheckHandler& handler); // content::PermissionManager: int RequestPermission( @@ -67,6 +72,11 @@ class AtomPermissionManager : public content::PermissionManager { const base::Callback< void(const std::vector&)>& callback); + bool CheckPermissionWithDetails(content::PermissionType permission, + content::RenderFrameHost* render_frame_host, + const GURL& requesting_origin, + const base::DictionaryValue* details) const; + protected: void OnPermissionResponse(int request_id, int permission_id, @@ -93,6 +103,7 @@ class AtomPermissionManager : public content::PermissionManager { using PendingRequestsMap = base::IDMap>; RequestHandler request_handler_; + CheckHandler check_handler_; PendingRequestsMap pending_requests_; diff --git a/atom/browser/web_contents_permission_helper.cc b/atom/browser/web_contents_permission_helper.cc index 1b3d809b1..1a1457473 100644 --- a/atom/browser/web_contents_permission_helper.cc +++ b/atom/browser/web_contents_permission_helper.cc @@ -14,6 +14,21 @@ DEFINE_WEB_CONTENTS_USER_DATA_KEY(atom::WebContentsPermissionHelper); +namespace { + +std::string MediaStreamTypeToString(content::MediaStreamType type) { + switch (type) { + case content::MediaStreamType::MEDIA_DEVICE_AUDIO_CAPTURE: + return "audio"; + case content::MediaStreamType::MEDIA_DEVICE_VIDEO_CAPTURE: + return "video"; + default: + return "unknown"; + } +} + +} // namespace + namespace atom { namespace { @@ -63,6 +78,17 @@ void WebContentsPermissionHelper::RequestPermission( base::Bind(&OnPermissionResponse, callback)); } +bool WebContentsPermissionHelper::CheckPermission( + content::PermissionType permission, + const base::DictionaryValue* details) const { + auto* rfh = web_contents_->GetMainFrame(); + auto* permission_manager = static_cast( + web_contents_->GetBrowserContext()->GetPermissionManager()); + auto origin = web_contents_->GetLastCommittedURL(); + return permission_manager->CheckPermissionWithDetails(permission, rfh, origin, + details); +} + void WebContentsPermissionHelper::RequestFullscreenPermission( const base::Callback& callback) { RequestPermission( @@ -102,4 +128,15 @@ void WebContentsPermissionHelper::RequestOpenExternalPermission( callback, user_gesture, &details); } +bool WebContentsPermissionHelper::CheckMediaAccessPermission( + const GURL& security_origin, + content::MediaStreamType type) const { + base::DictionaryValue details; + details.SetString("securityOrigin", security_origin.spec()); + details.SetString("mediaType", MediaStreamTypeToString(type)); + // The permission type doesn't matter here, AUDIO_CAPTURE/VIDEO_CAPTURE + // are presented as same type in content_converter.h. + return CheckPermission(content::PermissionType::AUDIO_CAPTURE, &details); +} + } // namespace atom diff --git a/atom/browser/web_contents_permission_helper.h b/atom/browser/web_contents_permission_helper.h index 322e0972f..a60b76836 100644 --- a/atom/browser/web_contents_permission_helper.h +++ b/atom/browser/web_contents_permission_helper.h @@ -23,6 +23,7 @@ class WebContentsPermissionHelper OPEN_EXTERNAL, }; + // Asynchronous Requests void RequestFullscreenPermission(const base::Callback& callback); void RequestMediaAccessPermission( const content::MediaStreamRequest& request, @@ -34,6 +35,10 @@ class WebContentsPermissionHelper bool user_gesture, const GURL& url); + // Synchronous Checks + bool CheckMediaAccessPermission(const GURL& security_origin, + content::MediaStreamType type) const; + private: explicit WebContentsPermissionHelper(content::WebContents* web_contents); friend class content::WebContentsUserData; @@ -43,6 +48,9 @@ class WebContentsPermissionHelper bool user_gesture = false, const base::DictionaryValue* details = nullptr); + bool CheckPermission(content::PermissionType permission, + const base::DictionaryValue* details) const; + content::WebContents* web_contents_; DISALLOW_COPY_AND_ASSIGN(WebContentsPermissionHelper); diff --git a/docs/api/session.md b/docs/api/session.md index 0d3c66aca..22f71fda2 100644 --- a/docs/api/session.md +++ b/docs/api/session.md @@ -311,6 +311,32 @@ session.fromPartition('some-partition').setPermissionRequestHandler((webContents }) ``` +#### `ses.setPermissionCheckHandler(handler)` + +* `handler` Function | null + * `webContents` [WebContents](web-contents.md) - WebContents checking the permission. + * `permission` String - Enum of 'media'. + * `requestingOrigin` String - The origin URL of the permission check + * `details` Object - Some properties are only available on certain permission types. + * `securityOrigin` String - The security orign of the `media` check. + * `mediaType` String - The type of media access being requested, can be `video`, + `audio` or `unknown` + +Sets the handler which can be used to respond to permission checks for the `session`. +Returning `true` will allow the permission and `false` will reject it. +To clear the handler, call `setPermissionCheckHandler(null)`. + +```javascript +const {session} = require('electron') +session.fromPartition('some-partition').setPermissionCheckHandler((webContents, permission) => { + if (webContents.getURL() === 'some-host' && permission === 'notifications') { + return false // denied + } + + return true +}) +``` + #### `ses.clearHostResolverCache([callback])` * `callback` Function (optional) - Called when operation is done. diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js index bd9451485..f0de7b146 100644 --- a/spec/chromium-spec.js +++ b/spec/chromium-spec.js @@ -106,6 +106,10 @@ describe('chromium feature', () => { describe('navigator.mediaDevices', () => { if (isCI) return + afterEach(() => { + remote.getGlobal('permissionChecks').allow() + }) + it('can return labels of enumerated devices', (done) => { navigator.mediaDevices.enumerateDevices().then((devices) => { const labels = devices.map((device) => device.label) @@ -118,6 +122,19 @@ describe('chromium feature', () => { }).catch(done) }) + it('does not return labels of enumerated devices when permission denied', (done) => { + remote.getGlobal('permissionChecks').reject() + navigator.mediaDevices.enumerateDevices().then((devices) => { + const labels = devices.map((device) => device.label) + const labelFound = labels.some((label) => !!label) + if (labelFound) { + done(new Error(`Device labels were found: ${JSON.stringify(labels)}`)) + } else { + done() + } + }).catch(done) + }) + it('can return new device id when cookie storage is cleared', (done) => { const options = { origin: null, diff --git a/spec/static/main.js b/spec/static/main.js index 5659c44c8..b0728bbb6 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -81,6 +81,11 @@ ipcMain.on('echo', function (event, msg) { global.setTimeoutPromisified = util.promisify(setTimeout) +global.permissionChecks = { + allow: () => electron.session.defaultSession.setPermissionCheckHandler(null), + reject: () => electron.session.defaultSession.setPermissionCheckHandler(() => false) +} + const coverage = new Coverage({ outputPath: path.join(__dirname, '..', '..', 'out', 'coverage') })