Repository: manusa/electronim
Pull request #257 (2023-01-07 06:32:17)

🐛 Slack files downloaded from ElectronIM
Relates to #254 

> Trying to download a file from slack doesn't work either. Instead is spawns a browser page and requests to login into the web interface.

Should be fixed now

sonarcloud[bot]: Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=manusa_electronim&pullRequest=257)

[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=257&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=257&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=257&resolved=false&types=BUG)  
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=257&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=257&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=257&resolved=false&types=VULNERABILITY)  
[![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=manusa_electronim&pullRequest=257&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=manusa_electronim&pullRequest=257&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=manusa_electronim&pullRequest=257&resolved=false&types=SECURITY_HOTSPOT)  
[![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=257&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=257&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=257&resolved=false&types=CODE_SMELL)

[![100.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/100-16px.png '100.0%')](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=257&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=257&metric=new_coverage&view=list)  
[![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=257&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=257&metric=new_duplicated_lines_density&view=list)



Patch:
diff --git a/src/tab-manager/redirect.js b/src/tab-manager/redirect.js
index a529840..0e778d5 100644
--- a/src/tab-manager/redirect.js
+++ b/src/tab-manager/redirect.js
@@ -40,12 +40,17 @@ const isOAuth = matchUrls([
   /^https:\/\/.+\.zoom\.us\/signin.*/ // NOSONAR
 ]);
 
+const isHandledInternally = matchUrls([
+  /^https:\/\/files\.slack\.com\/.*/ // NOSONAR
+]);
+
 const isSameOrigin = (browserViewUrl, url) => url.origin === browserViewUrl.origin;
 
 const shouldOpenInExternalBrowser = (browserView, url) => {
+  const browserViewUrl = new URL(browserView.webContents.getURL());
   let ret = true;
-  [isSameOrigin, isOAuth].forEach(f => {
-    if (ret && f(new URL(browserView.webContents.getURL()), url)) {
+  [isSameOrigin, isOAuth, isHandledInternally].forEach(f => {
+    if (ret && f(browserViewUrl, url)) {
       ret = false;
     }
   });
@@ -61,7 +66,7 @@ const handleRedirect = browserView => (e, urlString) => {
 };
 
 const windowOpenHandler = browserView => ({url}) => {
-  if (isSameOrigin(new URL(browserView.webContents.getURL()), new URL(url))) {
+  if (!shouldOpenInExternalBrowser(browserView, new URL(url))) {
     return {action: 'allow'};
   }
   shell.openExternal(url);


Test patch:
diff --git a/src/tab-manager/__tests__/redirect.test.js b/src/tab-manager/__tests__/redirect.test.js
index fe185d8..19d5262 100644
--- a/src/tab-manager/__tests__/redirect.test.js
+++ b/src/tab-manager/__tests__/redirect.test.js
@@ -35,6 +35,14 @@ describe('Tab Manager Redirect module test suite', () => {
         expect(result).toBe(true);
       });
     });
+    test.each([
+      'https://files.slack.com/files-pri/ID123/download/image.png?origin_team=ID456'
+    ])('URLs handled internally -isHandledInternally- (%s) , should return false', url => {
+      // When
+      const result = redirect.shouldOpenInExternalBrowser(mockBrowserView, new URL(url));
+      // Then
+      expect(result).toBe(false);
+    });
     describe('Google OAuth', () => {
       test('From OAuth to Slack, should return false', () => {
         // Given
@@ -223,14 +231,43 @@ describe('Tab Manager Redirect module test suite', () => {
       });
     });
   });
+  describe('handleRedirect', () => {
+    let event;
+    beforeEach(() => {
+      event = {preventDefault: jest.fn()};
+    });
+    describe('different origin and not handled', () => {
+      beforeEach(() => {
+        redirect.handleRedirect(mockBrowserView)(event, 'https://example.com/site-page');
+      });
+      test('should prevent default', () => {
+        expect(event.preventDefault).toHaveBeenCalled();
+      });
+      test('should open window in external browser', () => {
+        expect(require('electron').shell.openExternal).toHaveBeenCalledWith('https://example.com/site-page');
+      });
+    });
+  });
   describe('windowOpenHandler', () => {
-    test('same origin, opens window in Electron', () => {
+    test('same origin, opens window in Electron popup', () => {
       // When
       const result = redirect.windowOpenHandler(mockBrowserView)({url: 'http://localhost/terms-and-conditions'});
       // Then
       expect(result).toEqual({action: 'allow'});
     });
-    test('different origin, opens window in external browser', () => {
+    test('handled oauth, opens window in Electron popup', () => {
+      // When
+      const result = redirect.windowOpenHandler(mockBrowserView)({url: 'https://accounts.google.com/o/oauth2/auth'});
+      // Then
+      expect(result).toEqual({action: 'allow'});
+    });
+    test('handled internally, opens window in Electron popup', () => {
+      // When
+      const result = redirect.windowOpenHandler(mockBrowserView)({url: 'https://files.slack.com/files-pri/ID123/download/image.png?origin_team=ID456'});
+      // Then
+      expect(result).toEqual({action: 'allow'});
+    });
+    test('different origin and not handled, opens window in external browser', () => {
       // When
       const result = redirect.windowOpenHandler(mockBrowserView)({url: 'https://example.com/site-page'});
       // Then