Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDK-less Google SignIn – Part 5 – Connect the pieces #743

Merged
merged 28 commits into from
Mar 9, 2023
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
03ff3dc
Tentatively add "New" `NewGoogleAuthenticator`
mokagio Dec 5, 2022
76dd45a
Use `NewGoogleAuthenticator` in demo app
mokagio Dec 5, 2022
0eb0bbb
Add missing `audience` parameter to token request in new GoogleSignIn
mokagio Dec 8, 2022
56fe71e
Read `idToken` not `accessToken` from OAuth response
mokagio Dec 8, 2022
c9b0bb7
Add WPCom authentication call in demo app
mokagio Dec 8, 2022
69bf8c4
Add `WordPressLogginDelegate` implementation to demo app
mokagio Jan 18, 2023
ab9ab95
Fetch user display name after Google login
mokagio Jan 19, 2023
a134808
Rename `authenticate` to `getOAuthToken` for clarity
mokagio Jan 27, 2023
44a8f5c
Conform VCs involved in the Google SignIn flow to protocol for OAuth
mokagio Jan 27, 2023
a8c30e0
Remove unnecessary duplicated WP.com auth logic from demo app
mokagio Jan 27, 2023
d6a8154
Add option to login with and without SDK to demo app
mokagio Jan 27, 2023
dffdd5d
Use WordPressShared for `ConsoleLogger` by pointing to the lib to 2.1.0
mokagio Jan 30, 2023
1216c97
Return `IDToken` from `getOAuthToken()`
mokagio Feb 7, 2023
32bd679
Tidy up tests by consolidating JWT fixtures in dedicated extension
mokagio Feb 7, 2023
6f2659e
Add SDK-less flow call to existing `GoogleAuthenticator` method
mokagio Jan 27, 2023
ecf997c
Add changelog entry for #743
mokagio Feb 17, 2023
18eb889
Use easier to follow `withGoogleSDK` instead of `withoutGoogleSDK`
mokagio Feb 22, 2023
d4ed389
Remove an unnecessary import from the demo app
mokagio Feb 22, 2023
0607d2b
Use concise `Task {}` instead of `Task.init {}`
mokagio Feb 22, 2023
9af0e0f
Add note with rationale for hardcoding web sessions to be non ephemeral
mokagio Feb 22, 2023
923fa78
Fix `guard` statement checking for SDK-less flag
mokagio Feb 22, 2023
d5ed7eb
Allow `NewGoogleAuthenticator` to init with `UIViewController`
mokagio Mar 7, 2023
ecd5888
Add note about main thread dispatch in `NewGoogleAuthenticator`
mokagio Mar 8, 2023
485b906
Clarify when we test getting the token on in the demo app
mokagio Mar 8, 2023
4a4e0ed
Remove unnecessary `ASWebAuthenticationPresentationContextProviding`
mokagio Mar 8, 2023
d1b6c91
Remove unused `onSynced` function parameter
mokagio Mar 9, 2023
3c49abd
Make `OAuthError` `public` so consumers can access it
mokagio Mar 9, 2023
76fd3c4
Ensure a couple of `Task` run on the main thread via `@MainActor`
mokagio Mar 9, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ _None._

### New Features

_None._
- It's now possible to authenticate with a Google account without using the Google SDK, via the `googleLoginWithoutSDK` configuration. [#743]

### Bug Fixes

Expand Down
4 changes: 4 additions & 0 deletions Demo/AuthenticatorDemo/AppDelegate.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import UIKit
import WordPressKit

@main
class AppDelegate: UIResponder, UIApplicationDelegate {
Expand All @@ -10,7 +11,10 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
// crash the app if the value it finds is nil.
var window: UIWindow?

let logger = ConsoleLogger()

func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool {
WPKitSetLoggingDelegate(logger)
return true
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import WebKit
import WordPressAuthenticator
import WordPressKit
mokagio marked this conversation as resolved.
Show resolved Hide resolved

extension ViewController: WordPressAuthenticatorDelegate {

Expand Down Expand Up @@ -68,11 +70,7 @@ extension ViewController: WordPressAuthenticatorDelegate {

func sync(credentials: AuthenticatorCredentials, onCompletion: @escaping () -> Void) {
dismiss(animated: true) { [weak self] in
self?.presentAlert(
title: "Authentication Successful",
message: "Next step will be syncing credentials",
onDismiss: {}
)
self?.sync(credentials: credentials, onSynced: onCompletion)
}
}

Expand All @@ -90,3 +88,46 @@ extension ViewController: WordPressAuthenticatorDelegate {
print(error)
}
}

extension ViewController {

// This is just so we can avoid nesting within a dismiss block and the weak self dance.
//
// See WordPress iOS
//
// - WordPressAuthenticationManager sync(credentials:, onCompletion:)
// - WordPressAuthenticationManager syncWPCom(authToken:, isJetpackLogin:, onCompletion:)
// - AccountService createOrUpdateAccountWithAuthToken:success:failure:
private func sync(credentials: AuthenticatorCredentials, onSynced: @escaping () -> Void) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This onSynced is not called anywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good eyes!

Addressed in d1b6c91

Thanks 🙇‍♂️

switch (credentials.wpcom, credentials.wporg) {
case (.none, .none), (.some, .some):
fatalError("Inconsistent state!")
case (.none, .some):
fatalError("Not implemented yet")
case (.some(let wpComCredentials), .none):
let api = WordPressComRestApi(
oAuthToken: wpComCredentials.authToken,
// TODO: there should be a way to read the user agent from the library configs
userAgent: WKWebView.userAgent
)
let remote = AccountServiceRemoteREST(wordPressComRestApi: api)

remote.getAccountDetails(
success: { [weak self] remoteUser in
guard let remoteUser else {
fatalError("Received no RemoteUser – Likely an Objective-C types byproduct.")
}

self?.presentAlert(
title: "🎉",
message: "Welcome \(remoteUser.displayName ?? "'no display name'")",
onDismiss: {}
)
},
failure: { error in
print(error!.localizedDescription)
}
)
}
}
}
26 changes: 24 additions & 2 deletions Demo/AuthenticatorDemo/ViewController+WordPressAuthenticator.swift
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import WebKit
import WordPressAuthenticator
import WordPressKit

extension ViewController {

func initializeWordPressAuthenticator() {
func initializeWordPressAuthenticator(withGoogleSDK: Bool) {
// In a proper app, we'd want to split this call to keep the code readable. Here, it's
// useful to keep it all in one block to show how insanely long it is.
WordPressAuthenticator.initialize(
Expand All @@ -28,7 +29,8 @@ extension ViewController {
enableUnifiedCarousel: true,
// Notice that this is required as well as `enableSignupWithGoogle` to show the
// option to login with Google.
enableSocialLogin: true
enableSocialLogin: true,
googleLoginWithoutSDK: withGoogleSDK == false
),
style: WordPressAuthenticatorStyle(
// Primary (normal and highlight) is the color of buttons such as "Log in or signup
Expand Down Expand Up @@ -76,5 +78,25 @@ extension ViewController {
navTitleTextColor: .white
)
)

WordPressAuthenticator.shared.delegate = self
}

// Note that this method does not try to authenticate the user with the WordPress backend.
// It only verifies that we can get a token from Google.
func getAuthTokenFromGoogle() {
Task {
do {
let token = try await self.googleAuthenticator.getOAuthToken()

presentAlert(
title: "🎉",
message: "Successfully authenticated with Google.\n\nEmail in received token: \(token.email)",
onDismiss: {}
)
} catch {
presentAlert(title: "❌", message: error.localizedDescription, onDismiss: {})
Copy link
Contributor

Choose a reason for hiding this comment

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

Does presentAlert give us any guarantees that it runs on the main thread?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find anything in the docs about whether the UIKit method runs on the main thread.

This is a custom method so we can definitely force it to run on the main thread.

However, I'm not sure why we would go to "such effort" in the context of the demo app? What am I missing?

FWIW, TSan doesn't complain in the successful branch of the do catch.

}
}
}
}
28 changes: 24 additions & 4 deletions Demo/AuthenticatorDemo/ViewController.swift
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import AuthenticationServices
import UIKit
import WordPressAuthenticator

Expand All @@ -7,24 +8,43 @@ class ViewController: UIViewController {

/// Add `CellConfiguration` items to add new actionable rows to the table view in `ViewController`.
lazy var configuration: [CellConfiguration] = [
CellConfiguration(text: "Show Login") { [weak self] in
CellConfiguration(text: "Show Login - With Google SDK") { [weak self] in
guard let self else { fatalError() }

self.initializeWordPressAuthenticator(withGoogleSDK: true)
WordPressAuthenticator.showLoginFromPresenter(self, animated: true)
},
CellConfiguration(text: "Show Login - Without Google SDK") { [weak self] in
guard let self else { fatalError() }

self.initializeWordPressAuthenticator(withGoogleSDK: false)
WordPressAuthenticator.showLoginFromPresenter(self, animated: true)
},
CellConfiguration(text: "Get Google token only - Standalone, Wihout SDK") { [weak self] in
guard let self else { fatalError() }

self.initializeWordPressAuthenticator(withGoogleSDK: false)
self.getAuthTokenFromGoogle()
}
]

let tableView = UITableView(frame: .zero, style: .grouped)
let reuseIdentifier = "cell"

lazy var googleAuthenticator = NewGoogleAuthenticator(
clientId: GoogleClientId(string: APICredentials.googleLoginClientId)!,
scheme: APICredentials.googleLoginSchemeId,
audience: APICredentials.googleLoginServerClientId,
viewController: self,
urlSession: URLSession.shared
)

override func viewDidLoad() {
super.viewDidLoad()

title = "Authenticator Demo 🔐"

setUpTableView()

initializeWordPressAuthenticator()
WordPressAuthenticator.shared.delegate = self
}

func setUpTableView() {
Expand Down
2 changes: 1 addition & 1 deletion Podfile
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def wordpress_authenticator_pods
pod 'Gridicons', '~> 1.0-beta' # Don't change this until we hit 2.0 in Gridicons
pod 'WordPressUI', '~> 1.7-beta' # Don't change this until we hit 2.0 in WordPressUI
pod 'WordPressKit', '~> 6.0-beta' # Don't change this until we hit 5.0 in WPKit
pod 'WordPressShared', '~> 2.0-beta' # Don't change this until we hit 2.0 in WPShared
pod 'WordPressShared', '~> 2.1-beta'

third_party_pods
end
Expand Down
12 changes: 6 additions & 6 deletions Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ PODS:
- "NSURL+IDN (= 0.4)"
- SVProgressHUD (~> 2.2.5)
- WordPressKit (~> 6.0-beta)
- WordPressShared (~> 2.0-beta)
- WordPressShared (~> 2.1-beta)
- WordPressUI (~> 1.7-beta)
- WordPressKit (6.0.0-beta.1):
- Alamofire (~> 4.8.0)
- NSObject-SafeExpectations (~> 0.0.4)
- UIDeviceIdentifier (~> 2.0)
- WordPressShared (~> 2.0-beta)
- wpxmlrpc (~> 0.10)
- WordPressShared (2.0.0-beta.2)
- WordPressShared (2.1.0)
- WordPressUI (1.7.0)
- wpxmlrpc (0.10.0)

Expand All @@ -51,7 +51,7 @@ DEPENDENCIES:
- SwiftLint (~> 0.49)
- WordPressAuthenticator (from `.`)
- WordPressKit (~> 6.0-beta)
- WordPressShared (~> 2.0-beta)
- WordPressShared (~> 2.1-beta)
- WordPressUI (~> 1.7-beta)

SPEC REPOS:
Expand Down Expand Up @@ -94,12 +94,12 @@ SPEC CHECKSUMS:
SVProgressHUD: 1428aafac632c1f86f62aa4243ec12008d7a51d6
SwiftLint: 32ee33ded0636d0905ef6911b2b67bbaeeedafa5
UIDeviceIdentifier: f33af270ba9045ea18b31d9aab88e42a0082ea67
WordPressAuthenticator: fb233ed409d2cf86edf01ce5616dd15d4e9c9ca4
WordPressAuthenticator: 0620a9f3a367af505d9bc946975184a73906e2e5
WordPressKit: 08da0bc981f6398ef7a32e523fd054de7d7c7069
WordPressShared: 04403b43f821c4ed2b84a2112ef9f64f1e7cdceb
WordPressShared: 0aa459e5257a77184db87805a998f447443c9706
WordPressUI: 1cf47a3b78154faf69caa18569ee7ece1e510fa0
wpxmlrpc: 68db063041e85d186db21f674adf08d9c70627fd

PODFILE CHECKSUM: f1d40bf767881fbe9b0328b875768ee4a60a113f
PODFILE CHECKSUM: 8746ca27deaec1c319801f91d81dab582ee1c0f5

COCOAPODS: 1.11.3
2 changes: 1 addition & 1 deletion WordPressAuthenticator.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,5 @@ Pod::Spec.new do |s|
# If you want to update which of these is used, specify it in the host app.
s.dependency 'WordPressUI', '~> 1.7-beta'
s.dependency 'WordPressKit', '~> 6.0-beta'
s.dependency 'WordPressShared', '~> 2.0-beta'
s.dependency 'WordPressShared', '~> 2.1-beta'
end
Loading