Skip to content

Commit 1efd4e4

Browse files
authored
Merge pull request #17816 from wordpress-mobile/fix/publicize-connection-refactor
Refactors Publicize connection URL detection
2 parents 7cda265 + 0b54622 commit 1efd4e4

4 files changed

Lines changed: 275 additions & 87 deletions

File tree

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
import Foundation
2+
3+
/// Used to detect whether a URL matches a particular Publicize authorization success or failure route.
4+
struct PublicizeConnectionURLMatcher {
5+
enum MatchComponent {
6+
case verifyActionItem
7+
case denyActionItem
8+
case requestActionItem
9+
case stateItem
10+
case codeItem
11+
case errorItem
12+
13+
case authorizationPrefix
14+
case declinePath
15+
case accessDenied
16+
17+
// Special handling for the inconsistent way that services respond to a user's choice to decline
18+
// oauth authorization.
19+
// Right now we have no clear way to know if Tumblr fails. This is something we should try
20+
// fixing moving forward.
21+
// Path does not set the action param or call the callback. It forwards to its own URL ending in /decline.
22+
case userRefused
23+
24+
// In most cases, we attempt to find a matching URL by checking for a specific URL component
25+
fileprivate var queryItem: URLQueryItem? {
26+
switch self {
27+
case .verifyActionItem:
28+
return URLQueryItem(name: "action", value: "verify")
29+
case .denyActionItem:
30+
return URLQueryItem(name: "action", value: "deny")
31+
case .requestActionItem:
32+
return URLQueryItem(name: "action", value: "request")
33+
case .accessDenied:
34+
return URLQueryItem(name: "error", value: "access_denied")
35+
case .stateItem:
36+
return URLQueryItem(name: "state", value: nil)
37+
case .codeItem:
38+
return URLQueryItem(name: "code", value: nil)
39+
case .errorItem:
40+
return URLQueryItem(name: "error", value: nil)
41+
case .userRefused:
42+
return URLQueryItem(name: "oauth_problem", value: "user_refused")
43+
default:
44+
return nil
45+
}
46+
}
47+
48+
// In a handful of cases, we're just looking for a substring or prefix in the URL
49+
fileprivate var matchString: String? {
50+
switch self {
51+
case .declinePath:
52+
return "/decline"
53+
case .authorizationPrefix:
54+
return "https://public-api.wordpress.com/connect"
55+
default:
56+
return nil
57+
}
58+
}
59+
}
60+
61+
/// @return True if the url matches the current authorization component
62+
///
63+
static func url(_ url: URL, contains matchComponent: MatchComponent) -> Bool {
64+
if let queryItem = matchComponent.queryItem {
65+
return self.url(url, contains: queryItem)
66+
}
67+
68+
if let matchString = matchComponent.matchString {
69+
switch matchComponent {
70+
case .declinePath:
71+
return url.path.contains(matchString)
72+
case .authorizationPrefix:
73+
return url.absoluteString.hasPrefix(matchString)
74+
default:
75+
return url.absoluteString.contains(matchString)
76+
}
77+
}
78+
79+
return false
80+
}
81+
82+
// Checks to see if the current QueryItem is present in the specified URL
83+
private static func url(_ url: URL, contains queryItem: URLQueryItem) -> Bool {
84+
guard let queryItems = URLComponents(url: url, resolvingAgainstBaseURL: true)?.queryItems else {
85+
return false
86+
}
87+
88+
return queryItems.contains(where: { urlItem in
89+
var result = urlItem.name == queryItem.name
90+
91+
if let value = queryItem.value {
92+
result = result && (urlItem.value == value)
93+
}
94+
95+
return result
96+
})
97+
}
98+
99+
// MARK: - Authorization Actions
100+
101+
/// Classify actions taken by the web API
102+
///
103+
enum AuthorizeAction: Int {
104+
case none
105+
case unknown
106+
case request
107+
case verify
108+
case deny
109+
}
110+
111+
static func authorizeAction(for matchURL: URL) -> AuthorizeAction {
112+
// Path oauth declines are handled by a redirect to a path.com URL, so check this first.
113+
if url(matchURL, contains: .declinePath) {
114+
return .deny
115+
}
116+
117+
if !url(matchURL, contains: .authorizationPrefix) {
118+
return .none
119+
}
120+
121+
if url(matchURL, contains: .requestActionItem) {
122+
return .request
123+
}
124+
125+
// Check the rest of the various decline ranges
126+
if url(matchURL, contains: .denyActionItem) {
127+
return .deny
128+
}
129+
130+
// LinkedIn
131+
if url(matchURL, contains: .userRefused) {
132+
return .deny
133+
}
134+
135+
// Facebook and Google+
136+
if url(matchURL, contains: .accessDenied) {
137+
return .deny
138+
}
139+
140+
// If we've made it this far and the `action=verify` query param is present then we're
141+
// *probably* verifying the oauth request. There are edge cases ( :cough: tumblr :cough: )
142+
// where verification is declined and we get a false positive.
143+
if url(matchURL, contains: .verifyActionItem) {
144+
return .verify
145+
}
146+
147+
// Facebook
148+
if url(matchURL, contains: .stateItem) && url(matchURL, contains: .codeItem) {
149+
return .verify
150+
}
151+
152+
// Facebook failure
153+
if url(matchURL, contains: .stateItem) && url(matchURL, contains: .errorItem) {
154+
return .unknown
155+
}
156+
157+
return .unknown
158+
}
159+
}

WordPress/Classes/ViewRelated/Blog/SharingAuthorizationWebViewController.swift

Lines changed: 2 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import WebKit
2+
import CoreMedia
23

34
@objc
45
protocol SharingAuthorizationDelegate: NSObjectProtocol {
@@ -14,43 +15,8 @@ protocol SharingAuthorizationDelegate: NSObjectProtocol {
1415

1516
@objc
1617
class SharingAuthorizationWebViewController: WPWebViewController {
17-
/// Classify actions taken by the web API
18-
///
19-
private enum AuthorizeAction: Int {
20-
case none
21-
case unknown
22-
case request
23-
case verify
24-
case deny
25-
}
26-
2718
private static let loginURL = "https://wordpress.com/wp-login.php"
2819

29-
private enum AuthorizeURLComponents: String {
30-
case verifyActionParameter = "action=verify"
31-
case denyActionParameter = "action=deny"
32-
case requestActionParameter = "action=request"
33-
34-
case declinePath = "/decline"
35-
case authorizationPrefix = "https://public-api.wordpress.com/connect/"
36-
case accessDenied = "error=access_denied"
37-
38-
case state = "state"
39-
case code = "code"
40-
case error = "error"
41-
42-
// Special handling for the inconsistent way that services respond to a user's choice to decline
43-
// oauth authorization.
44-
// Right now we have no clear way to know if Tumblr fails. This is something we should try
45-
// fixing moving forward.
46-
// Path does not set the action param or call the callback. It forwards to its own URL ending in /decline.
47-
case userRefused = "oauth_problem=user_refused"
48-
49-
func containedIn(_ url: URL) -> Bool {
50-
url.absoluteString.contains(rawValue)
51-
}
52-
}
53-
5420
/// Verification loading -- dismiss on completion
5521
///
5622
private var loadingVerify: Bool = false
@@ -152,57 +118,6 @@ class SharingAuthorizationWebViewController: WPWebViewController {
152118
private func displayLoadError(error: NSError) {
153119
delegate?.authorize(self.publicizer, didFailWithError: error)
154120
}
155-
156-
// MARK: - URL Interpretation
157-
158-
private func authorizeAction(from url: URL) -> AuthorizeAction {
159-
// Path oauth declines are handled by a redirect to a path.com URL, so check this first.
160-
if AuthorizeURLComponents.declinePath.containedIn(url) {
161-
return .deny
162-
}
163-
164-
if !url.absoluteString.hasPrefix(AuthorizeURLComponents.authorizationPrefix.rawValue) {
165-
return .none
166-
}
167-
168-
if AuthorizeURLComponents.requestActionParameter.containedIn(url) {
169-
return .request
170-
}
171-
172-
// Check the rest of the various decline ranges
173-
if AuthorizeURLComponents.denyActionParameter.containedIn(url) {
174-
return .deny
175-
}
176-
177-
// LinkedIn
178-
if AuthorizeURLComponents.userRefused.containedIn(url) {
179-
return .deny
180-
}
181-
182-
// Facebook and Google+
183-
if AuthorizeURLComponents.accessDenied.containedIn(url) {
184-
return .deny
185-
}
186-
187-
// If we've made it this far and verifyRange is found then we're *probably*
188-
// verifying the oauth request. There are edge cases ( :cough: tumblr :cough: )
189-
// where verification is declined and we get a false positive.
190-
if AuthorizeURLComponents.verifyActionParameter.containedIn(url) {
191-
return .verify
192-
}
193-
194-
// Facebook
195-
if AuthorizeURLComponents.state.containedIn(url) && AuthorizeURLComponents.code.containedIn(url) {
196-
return .verify
197-
}
198-
199-
// Facebook failure
200-
if AuthorizeURLComponents.state.containedIn(url) && AuthorizeURLComponents.error.containedIn(url) {
201-
return .unknown
202-
}
203-
204-
return .unknown
205-
}
206121
}
207122

208123
// MARK: - WKNavigationDelegate
@@ -218,7 +133,7 @@ extension SharingAuthorizationWebViewController {
218133
return
219134
}
220135

221-
let action = authorizeAction(from: url)
136+
let action = PublicizeConnectionURLMatcher.authorizeAction(for: url)
222137

223138
switch action {
224139
case .none:

WordPress/WordPress.xcodeproj/project.pbxproj

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,8 @@
244244
1752D4FA238D702E002B79E7 /* KeyValueDatabase.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1751E5901CE0E552000CA08D /* KeyValueDatabase.swift */; };
245245
1752D4FB238D702F002B79E7 /* KeyValueDatabase.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1751E5901CE0E552000CA08D /* KeyValueDatabase.swift */; };
246246
1752D4FC238D703A002B79E7 /* KeyValueDatabase.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1751E5901CE0E552000CA08D /* KeyValueDatabase.swift */; };
247+
175507B327A062980038ED28 /* PublicizeConnectionURLMatcher.swift in Sources */ = {isa = PBXBuildFile; fileRef = 175507B227A062980038ED28 /* PublicizeConnectionURLMatcher.swift */; };
248+
175507B427A062980038ED28 /* PublicizeConnectionURLMatcher.swift in Sources */ = {isa = PBXBuildFile; fileRef = 175507B227A062980038ED28 /* PublicizeConnectionURLMatcher.swift */; };
247249
175721162754D31F00DE38BC /* AppIcon.swift in Sources */ = {isa = PBXBuildFile; fileRef = 175721152754D31F00DE38BC /* AppIcon.swift */; };
248250
175721172754D31F00DE38BC /* AppIcon.swift in Sources */ = {isa = PBXBuildFile; fileRef = 175721152754D31F00DE38BC /* AppIcon.swift */; };
249251
1759F1701FE017BF0003EC81 /* Queue.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1759F16F1FE017BF0003EC81 /* Queue.swift */; };
@@ -313,6 +315,7 @@
313315
178DDD31266D7576006C68C4 /* BloggingRemindersFlowCompletionViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 178DDD2F266D7576006C68C4 /* BloggingRemindersFlowCompletionViewController.swift */; };
314316
178DDD57266E4165006C68C4 /* CalendarDayToggleButton.swift in Sources */ = {isa = PBXBuildFile; fileRef = 178DDD56266E4165006C68C4 /* CalendarDayToggleButton.swift */; };
315317
1790A4531E28F0ED00AE54C2 /* UINavigationController+Helpers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1790A4521E28F0ED00AE54C2 /* UINavigationController+Helpers.swift */; };
318+
179501CD27A01D4100882787 /* PublicizeAuthorizationURLComponentsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 179501CC27A01D4100882787 /* PublicizeAuthorizationURLComponentsTests.swift */; };
316319
1797373720EBAA4100377B4E /* RouteMatcherTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1797373620EBAA4100377B4E /* RouteMatcherTests.swift */; };
317320
179A70F02729834B006DAC0A /* Binding+OnChange.swift in Sources */ = {isa = PBXBuildFile; fileRef = 179A70EF2729834B006DAC0A /* Binding+OnChange.swift */; };
318321
179A70F12729834B006DAC0A /* Binding+OnChange.swift in Sources */ = {isa = PBXBuildFile; fileRef = 179A70EF2729834B006DAC0A /* Binding+OnChange.swift */; };
@@ -4886,6 +4889,7 @@
48864889
1751E5901CE0E552000CA08D /* KeyValueDatabase.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = KeyValueDatabase.swift; sourceTree = "<group>"; };
48874890
1751E5921CE23801000CA08D /* NSAttributedString+StyledHTML.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "NSAttributedString+StyledHTML.swift"; sourceTree = "<group>"; };
48884891
17523380246C4F9200870B4A /* HomepageSettingsViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HomepageSettingsViewController.swift; sourceTree = "<group>"; };
4892+
175507B227A062980038ED28 /* PublicizeConnectionURLMatcher.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PublicizeConnectionURLMatcher.swift; sourceTree = "<group>"; };
48894893
175721152754D31F00DE38BC /* AppIcon.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppIcon.swift; sourceTree = "<group>"; };
48904894
1759F16F1FE017BF0003EC81 /* Queue.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Queue.swift; sourceTree = "<group>"; };
48914895
1759F1711FE017F20003EC81 /* QueueTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = QueueTests.swift; sourceTree = "<group>"; };
@@ -4947,6 +4951,7 @@
49474951
178DDD2F266D7576006C68C4 /* BloggingRemindersFlowCompletionViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BloggingRemindersFlowCompletionViewController.swift; sourceTree = "<group>"; };
49484952
178DDD56266E4165006C68C4 /* CalendarDayToggleButton.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CalendarDayToggleButton.swift; sourceTree = "<group>"; };
49494953
1790A4521E28F0ED00AE54C2 /* UINavigationController+Helpers.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "UINavigationController+Helpers.swift"; sourceTree = "<group>"; };
4954+
179501CC27A01D4100882787 /* PublicizeAuthorizationURLComponentsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PublicizeAuthorizationURLComponentsTests.swift; sourceTree = "<group>"; };
49504955
1797373620EBAA4100377B4E /* RouteMatcherTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RouteMatcherTests.swift; sourceTree = "<group>"; };
49514956
179A70EF2729834B006DAC0A /* Binding+OnChange.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Binding+OnChange.swift"; sourceTree = "<group>"; };
49524957
17A09B98238FE13B0022AE0D /* FeatureFlagOverrideStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeatureFlagOverrideStore.swift; sourceTree = "<group>"; };
@@ -11092,6 +11097,7 @@
1109211097
24B1AE3024FEC79900B9F334 /* RemoteFeatureFlagTests.swift */,
1109311098
F551E7F623FC9A5C00751212 /* Collection+RotateTests.swift */,
1109411099
FAE8EE9B273AD0A800A65307 /* QuickStartSettingsTests.swift */,
11100+
179501CC27A01D4100882787 /* PublicizeAuthorizationURLComponentsTests.swift */,
1109511101
);
1109611102
name = Utility;
1109711103
sourceTree = "<group>";
@@ -14069,6 +14075,7 @@
1406914075
E63BBC941C5168BE00598BE8 /* SharingAuthorizationHelper.h */,
1407014076
E63BBC951C5168BE00598BE8 /* SharingAuthorizationHelper.m */,
1407114077
F16601C323E9E783007950AE /* SharingAuthorizationWebViewController.swift */,
14078+
175507B227A062980038ED28 /* PublicizeConnectionURLMatcher.swift */,
1407214079
E663D18F1C65383E0017F109 /* SharingAccountViewController.swift */,
1407314080
E6431DE11C4E892900FD8D90 /* SharingDetailViewController.h */,
1407414081
E6431DE21C4E892900FD8D90 /* SharingDetailViewController.m */,
@@ -17461,6 +17468,7 @@
1746117468
E64384831C628FCC0052ADB5 /* WPStyleGuide+Sharing.swift in Sources */,
1746217469
F504D2B025D60C5900A2764C /* StoryPoster.swift in Sources */,
1746317470
981C82B62193A7B900A06E84 /* Double+Stats.swift in Sources */,
17471+
175507B327A062980038ED28 /* PublicizeConnectionURLMatcher.swift in Sources */,
1746417472
177074851FB209F100951A4A /* CircularProgressView.swift in Sources */,
1746517473
98458CB821A39D350025D232 /* StatsNoDataRow.swift in Sources */,
1746617474
3234BB172530DFCA0068DA40 /* ReaderTableCardCell.swift in Sources */,
@@ -19255,6 +19263,7 @@
1925519263
246D0A0325E97D5D0028B83F /* Blog+ObjcTests.m in Sources */,
1925619264
9A9D34FF2360A4E200BC95A3 /* StatsPeriodAsyncOperationTests.swift in Sources */,
1925719265
B5EFB1C91B333C5A007608A3 /* NotificationSettingsServiceTests.swift in Sources */,
19266+
179501CD27A01D4100882787 /* PublicizeAuthorizationURLComponentsTests.swift in Sources */,
1925819267
4089C51422371EE30031CE78 /* TodayStatsTests.swift in Sources */,
1925919268
7E53AB0A20FE83A9005796FE /* MockContentCoordinator.swift in Sources */,
1926019269
BE1071FF1BC75FFA00906AFF /* WPStyleGuide+BlogTests.swift in Sources */,
@@ -20177,6 +20186,7 @@
2017720186
FABB23C22602FC2C00C8785C /* PrepublishingHeaderView.swift in Sources */,
2017820187
3FB1929126C6C56E000F5AA3 /* TimeSelectionButton.swift in Sources */,
2017920188
FABB23C32602FC2C00C8785C /* StatsWidgetsStore.swift in Sources */,
20189+
175507B427A062980038ED28 /* PublicizeConnectionURLMatcher.swift in Sources */,
2018020190
FABB23C42602FC2C00C8785C /* MenuItemsVisualOrderingView.m in Sources */,
2018120191
FABB23C52602FC2C00C8785C /* JetpackScanViewController.swift in Sources */,
2018220192
FABB23C62602FC2C00C8785C /* ReaderSubscribingNotificationAction.swift in Sources */,

0 commit comments

Comments
 (0)