diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c85b1fc..2fdc641 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,10 +14,12 @@ jobs: - name: Checkout uses: actions/checkout@v4 + # Installs the Tuist version pinned in mise.toml, keeping CI and local dev + # in sync. Not Homebrew: the tuist tap's cask declares `conflicts_with + # formula:`, a key Homebrew 6 dropped, which aborts any `tuist` lookup in + # the tap. See tuist/homebrew-tuist#41. - name: Install Tuist - run: | - brew tap tuist/tuist - brew install --formula tuist + uses: jdx/mise-action@c37c93293d6b742fc901e1406b8f764f6fb19dac # v2 - name: Lint run: swift run --package-path BuildTools swiftformat . --lint diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 2c637a3..e5fa243 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -70,10 +70,9 @@ jobs: exit 1 fi + # Same pinned-Tuist-via-mise setup as ci.yml; see the note there. - name: Install Tuist - run: | - brew tap tuist/tuist - brew install --formula tuist + uses: jdx/mise-action@c37c93293d6b742fc901e1406b8f764f6fb19dac # v2 - name: Set up Ruby and fastlane uses: ruby/setup-ruby@v1 diff --git a/Allkdic/AppDelegate.swift b/Allkdic/AppDelegate.swift index fc28f5b..86ed8d8 100644 --- a/Allkdic/AppDelegate.swift +++ b/Allkdic/AppDelegate.swift @@ -51,6 +51,7 @@ class AppDelegate: NSObject, NSApplicationDelegate { self.popover.contentViewController = NSHostingController(rootView: contentView) self.popover.contentSize = NSSize(width: 420, height: 580) self.popover.behavior = .transient + self.popover.delegate = self } private func setupEventMonitor() { @@ -58,19 +59,27 @@ class AppDelegate: NSObject, NSApplicationDelegate { self?.closePopover() } NSEvent.addLocalMonitorForEvents(matching: .keyDown) { [weak self] event in + guard let self, self.isPopoverEvent(event) else { return event } if event.keyCode == 53 { // Escape key - self?.closePopover() + self.closePopover() return nil } if event.modifierFlags.contains(.command), event.charactersIgnoringModifiers == "," { - self?.closePopover() - self?.openPreferencesWindow() + self.closePopover() + self.openPreferencesWindow() return nil } return event } } + /// This monitor is app-wide and never removed, so without a window check it + /// would also swallow keys aimed at the login, preferences and about windows. + func isPopoverEvent(_ event: NSEvent) -> Bool { + guard self.popover.isShown, let popoverWindow = popover.contentViewController?.view.window else { return false } + return event.window === popoverWindow + } + @objc func togglePopover() { if self.popover.isShown { self.closePopover() @@ -132,3 +141,11 @@ class AppDelegate: NSObject, NSApplicationDelegate { AnalyticsHelper.shared.trackAboutOpened() } } + +extension AppDelegate: NSPopoverDelegate { + /// A `.transient` popover also closes on its own, e.g. when the login window + /// takes focus, which bypasses `closePopover()` and leaves the button lit. + func popoverDidClose(_: Notification) { + self.statusItem.button?.state = .off + } +} diff --git a/Allkdic/Types/PopupRequest.swift b/Allkdic/Types/PopupRequest.swift new file mode 100644 index 0000000..4e680fa --- /dev/null +++ b/Allkdic/Types/PopupRequest.swift @@ -0,0 +1,67 @@ +import WebKit + +enum PopupDisposition: Equatable { + /// Opened inside the app so the session lands in the same cookie store as the + /// dictionary web view. + case inAppLogin + /// Handed to the default browser: full-site pages are unusable at popover size. + case externalBrowser + case deny +} + +/// The parts of a `WKNavigationAction` that popup routing depends on. +/// +/// Modelled as plain values because `WKNavigationAction` and `WKWindowFeatures` +/// are framework-produced and cannot be constructed in tests. +struct PopupRequest: Equatable { + let url: URL? + let sourceURL: URL? + let navigationType: WKNavigationType + + /// Exact hosts, never suffix matches: `*.naver.com` or `*.daum.net` would + /// gradually promote unrelated content and ad popups into in-app windows that + /// share the logged-in cookie store. + private static let authenticationHosts: Set = [ + "nid.naver.com", + "logins.daum.net", + "accounts.kakao.com", + ] + + private static let dictionaryHosts: Set = [ + "dict.naver.com", + "dic.daum.net", + ] + + var disposition: PopupDisposition { + if self.isBlankBootstrap { + let opensLoginWindow = self.navigationType == .other && self.isFromDictionary + return opensLoginWindow ? .inAppLogin : .deny + } + + // The scheme is checked before the host so that a host we trust cannot pull + // an unsupported scheme, such as file://nid.naver.com/, into a window that + // shares logged-in cookies. + guard let url = self.url, let scheme = url.scheme?.lowercased(), scheme == "http" || scheme == "https" else { + return .deny + } + + if let host = url.host?.lowercased(), Self.authenticationHosts.contains(host) { + return .inAppLogin + } + + return .externalBrowser + } + + /// `window.open("", name, features)` opens an empty window and navigates it + /// afterwards, so the login URL isn't known yet when the window is requested. + /// Only a genuinely blank URL qualifies; other `about:` URLs are not. + private var isBlankBootstrap: Bool { + guard let url = self.url else { return true } + return url.absoluteString.lowercased() == "about:blank" + } + + private var isFromDictionary: Bool { + guard let host = self.sourceURL?.host?.lowercased() else { return false } + return Self.dictionaryHosts.contains(host) || Self.dictionaryHosts.contains { host.hasSuffix(".\($0)") } + } +} diff --git a/Allkdic/Views/DictionaryWebView.swift b/Allkdic/Views/DictionaryWebView.swift index 24d5782..83c6ceb 100644 --- a/Allkdic/Views/DictionaryWebView.swift +++ b/Allkdic/Views/DictionaryWebView.swift @@ -63,6 +63,8 @@ private struct WebView: NSViewRepresentable { var lastLoadedURL: String? weak var webView: WKWebView? private nonisolated(unsafe) var popoverObserver: NSObjectProtocol? + private var loginPopupController: LoginPopupController? + private var needsSessionReload = false init(isLoading: Binding, dictionary: DictionaryType) { _isLoading = isLoading @@ -91,7 +93,18 @@ private struct WebView: NSViewRepresentable { } private func reloadIfNeeded() { - guard self.lastLoadedURL == nil, let url = URL(string: self.dictionary.URLString) else { return } + guard self.lastLoadedURL != nil else { + self.loadDictionary() + return + } + guard self.needsSessionReload else { return } + self.isLoading = true + self.webView?.reload() + } + + private func loadDictionary() { + guard let url = URL(string: self.dictionary.URLString) else { return } + self.lastLoadedURL = self.dictionary.URLString self.isLoading = true self.webView?.load(URLRequest(url: url)) } @@ -108,6 +121,9 @@ private struct WebView: NSViewRepresentable { func webView(_ webView: WKWebView, didFinish _: WKNavigation!) { self.isLoading = false + // Cleared only once a load actually lands, so a reload that fails while + // offline is retried on the next popover open instead of being lost. + self.needsSessionReload = false if let css = dictionary.customCSS { let script = """ var style = document.createElement('style'); @@ -133,14 +149,45 @@ private struct WebView: NSViewRepresentable { func webView( _: WKWebView, - createWebViewWith _: WKWebViewConfiguration, + createWebViewWith configuration: WKWebViewConfiguration, for navigationAction: WKNavigationAction, windowFeatures _: WKWindowFeatures, ) -> WKWebView? { - if let url = navigationAction.request.url { - NSWorkspace.shared.open(url) + let request = PopupRequest( + url: navigationAction.request.url, + sourceURL: navigationAction.sourceFrame.request.url, + navigationType: navigationAction.navigationType, + ) + + switch request.disposition { + case .inAppLogin: + return self.presentLoginPopup(configuration: configuration) + + case .externalBrowser: + if let url = request.url { + NSWorkspace.shared.open(url) + } + return nil + + case .deny: + return nil } - return nil + } + + /// Reuses the configuration WebKit passes in, which is what ties the popup + /// to the dictionary web view's cookie store and makes the login stick. + private func presentLoginPopup(configuration: WKWebViewConfiguration) -> WKWebView? { + self.loginPopupController?.close() + + let controller = LoginPopupController(configuration: configuration, title: self.dictionary.title) + controller.onClose = { [weak self] in + self?.loginPopupController = nil + self?.needsSessionReload = true + } + controller.show() + self.loginPopupController = controller + + return controller.webView } } } diff --git a/Allkdic/Views/LoginPopupController.swift b/Allkdic/Views/LoginPopupController.swift new file mode 100644 index 0000000..defc1b3 --- /dev/null +++ b/Allkdic/Views/LoginPopupController.swift @@ -0,0 +1,136 @@ +import Cocoa +import WebKit + +/// The app has no menu bar (`LSUIElement`), so there is no Close item to route +/// Cmd+W here and no responder handling for Escape. Both are wired up manually. +private final class LoginPopupWindow: NSWindow { + override func cancelOperation(_: Any?) { + self.performClose(nil) + } + + override func performKeyEquivalent(with event: NSEvent) -> Bool { + guard event.modifierFlags.intersection(.deviceIndependentFlagsMask) == .command, + event.charactersIgnoringModifiers == "w" + else { + return super.performKeyEquivalent(with: event) + } + self.performClose(nil) + return true + } +} + +/// A login window opened by a dictionary page via `window.open`. +/// +/// It deliberately does not reuse the dictionary's `Coordinator` as its delegate: +/// those callbacks drive the main view's loading indicator, custom CSS and input +/// focusing, none of which may be triggered by popup navigations. +@MainActor +final class LoginPopupController: NSObject { + let webView: WKWebView + var onClose: (() -> Void)? + + private let window: NSWindow + private var hasFinished = false + + init(configuration: WKWebViewConfiguration, title: String) { + self.webView = WKWebView( + frame: NSRect(x: 0, y: 0, width: 480, height: 640), + configuration: configuration, + ) + self.window = LoginPopupWindow( + contentRect: self.webView.frame, + styleMask: [.titled, .closable, .resizable], + backing: .buffered, + defer: false, + ) + super.init() + + self.webView.uiDelegate = self + self.webView.navigationDelegate = self + + // Under ARC the controller owns the window; the AppKit-era auto-release + // would over-release it. + self.window.isReleasedWhenClosed = false + self.window.title = title + self.window.contentView = self.webView + self.window.delegate = self + self.window.center() + } + + func show() { + // Plain activate(): `ignoringOtherApps` makes Stage Manager treat this as a + // scene switch and slide other apps' windows aside (see 0746a14). + NSApp.activate() + self.window.makeKeyAndOrderFront(nil) + } + + func close() { + self.window.close() + } + + /// Both `webViewDidClose` and `windowWillClose` can fire for a single close. + private func finish() { + guard !self.hasFinished else { return } + self.hasFinished = true + + self.webView.uiDelegate = nil + self.webView.navigationDelegate = nil + self.window.delegate = nil + + let onClose = self.onClose + self.onClose = nil + onClose?() + } +} + +extension LoginPopupController: WKUIDelegate { + func webViewDidClose(_: WKWebView) { + self.close() + } + + func webView( + _ webView: WKWebView, + createWebViewWith _: WKWebViewConfiguration, + for navigationAction: WKNavigationAction, + windowFeatures _: WKWindowFeatures, + ) -> WKWebView? { + // Keep secondary flows ("find ID/password") in this window rather than + // stacking more windows on top. + guard navigationAction.targetFrame == nil else { return nil } + webView.load(navigationAction.request) + return nil + } +} + +extension LoginPopupController: WKNavigationDelegate { + func webView( + _: WKWebView, + decidePolicyFor navigationAction: WKNavigationAction, + decisionHandler: @escaping (WKNavigationActionPolicy) -> Void, + ) { + guard let url = navigationAction.request.url, let scheme = url.scheme?.lowercased() else { + decisionHandler(.cancel) + return + } + + switch scheme { + case "http", "https", "about", "data", "blob": + decisionHandler(.allow) + + // "Log in with the app" hands off to a native app WebKit cannot load. Only + // known schemes are forwarded, so a page cannot launch arbitrary apps. + case "naversearchapp", "kakaotalk": + decisionHandler(.cancel) + NSWorkspace.shared.open(url) + + default: + decisionHandler(.cancel) + } + } +} + +extension LoginPopupController: NSWindowDelegate { + func windowWillClose(_: Notification) { + self.finish() + } +} diff --git a/AllkdicTests/PopupRequestTests.swift b/AllkdicTests/PopupRequestTests.swift new file mode 100644 index 0000000..9511011 --- /dev/null +++ b/AllkdicTests/PopupRequestTests.swift @@ -0,0 +1,139 @@ +@testable import Allkdic +import WebKit +import XCTest + +final class PopupRequestTests: XCTestCase { + private func makeRequest( + url: String?, + sourceURL: String? = "https://en.dict.naver.com/#/mini/main", + navigationType: WKNavigationType = .other, + ) -> PopupRequest { + PopupRequest( + url: url.flatMap { URL(string: $0) }, + sourceURL: sourceURL.flatMap { URL(string: $0) }, + navigationType: navigationType, + ) + } + + // MARK: - Login popups open in the app + + func test_disposition_naverLoginHost() { + let request = self.makeRequest(url: "https://nid.naver.com/nidlogin.login") + XCTAssertEqual(request.disposition, .inAppLogin) + } + + func test_disposition_daumLoginHost() { + let request = self.makeRequest( + url: "https://logins.daum.net/accounts/loginform.do", + sourceURL: "https://small.dic.daum.net/top/search.do", + ) + XCTAssertEqual(request.disposition, .inAppLogin) + } + + func test_disposition_kakaoAccountsHost() { + let request = self.makeRequest( + url: "https://accounts.kakao.com/login", + sourceURL: "https://small.dic.daum.net/top/search.do", + ) + XCTAssertEqual(request.disposition, .inAppLogin) + } + + // A login link may arrive as a plain link activation rather than script. + func test_disposition_loginHost_viaLinkActivated() { + let request = self.makeRequest(url: "https://nid.naver.com/nidlogin.login", navigationType: .linkActivated) + XCTAssertEqual(request.disposition, .inAppLogin) + } + + // MARK: - Content links stay in the browser (#66) + + func test_disposition_contentLink_viaLinkActivated() { + let request = self.makeRequest( + url: "https://dict.naver.com/frdict/#/entry/frko/conjugation", + navigationType: .linkActivated, + ) + XCTAssertEqual(request.disposition, .externalBrowser) + } + + // The same link once the page intercepts the click with JavaScript. + func test_disposition_contentLink_viaScript() { + let request = self.makeRequest(url: "https://dict.naver.com/frdict/#/entry/frko/conjugation") + XCTAssertEqual(request.disposition, .externalBrowser) + } + + func test_disposition_unrelatedHostOnSameDomain() { + let request = self.makeRequest(url: "https://ad.naver.com/promotion") + XCTAssertEqual(request.disposition, .externalBrowser) + } + + // A lookalike host must never inherit the login window's cookie store. + func test_disposition_lookalikeLoginHost() { + let request = self.makeRequest(url: "https://nid.naver.com.example.com/nidlogin.login") + XCTAssertEqual(request.disposition, .externalBrowser) + } + + // A trusted host must not drag an unsupported scheme into the login window. + func test_disposition_loginHostWithFileScheme() { + let request = self.makeRequest(url: "file://nid.naver.com/nidlogin.login") + XCTAssertEqual(request.disposition, .deny) + } + + func test_disposition_loginHostWithCustomScheme() { + let request = self.makeRequest(url: "naversearchapp://nid.naver.com/nidlogin.login") + XCTAssertEqual(request.disposition, .deny) + } + + // MARK: - Blank popups navigated after opening + + func test_disposition_blankPopupFromDictionary() { + let request = self.makeRequest(url: "about:blank") + XCTAssertEqual(request.disposition, .inAppLogin) + } + + func test_disposition_missingURLFromDictionary() { + let request = self.makeRequest(url: nil) + XCTAssertEqual(request.disposition, .inAppLogin) + } + + func test_disposition_blankPopupFromSubdomainOfDictionary() { + let request = self.makeRequest(url: "about:blank", sourceURL: "https://small.dic.daum.net/top/search.do") + XCTAssertEqual(request.disposition, .inAppLogin) + } + + func test_disposition_blankPopupFromUntrustedSource() { + let request = self.makeRequest(url: "about:blank", sourceURL: "https://example.com") + XCTAssertEqual(request.disposition, .deny) + } + + func test_disposition_blankPopupFromLookalikeSource() { + let request = self.makeRequest(url: "about:blank", sourceURL: "https://dict.naver.com.example.com") + XCTAssertEqual(request.disposition, .deny) + } + + func test_disposition_blankPopup_viaLinkActivated() { + let request = self.makeRequest(url: nil, navigationType: .linkActivated) + XCTAssertEqual(request.disposition, .deny) + } + + // Only a genuinely blank URL is a bootstrap; other about: URLs are not. + func test_disposition_nonBlankAboutURL() { + let request = self.makeRequest(url: "about:srcdoc") + XCTAssertEqual(request.disposition, .deny) + } + + // MARK: - Everything else is denied + + func test_disposition_customScheme() { + let request = self.makeRequest(url: "naversearchapp://search?query=test") + XCTAssertEqual(request.disposition, .deny) + } + + func test_disposition_fileScheme() { + let request = self.makeRequest(url: "file:///etc/passwd") + XCTAssertEqual(request.disposition, .deny) + } + + func test_disposition_javascriptScheme() { + let request = self.makeRequest(url: "javascript:alert(1)") + XCTAssertEqual(request.disposition, .deny) + } +} diff --git a/mise.toml b/mise.toml new file mode 100644 index 0000000..3dbbb61 --- /dev/null +++ b/mise.toml @@ -0,0 +1,2 @@ +[tools] +tuist = "4.200.5"