This repository was archived by the owner on Feb 5, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 11
SDK-less Google-SignIn – Part 2 of n #717
Merged
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
f15815f
Add object modelling Proof Key For Code Exchange
mokagio 9c8b1d6
Add computation of `code_challenge_method`
mokagio 7f93ab0
Add PKCE parameter to GoogleSignIn URL
mokagio 7c8b61c
Add `scope` parameter to GoogleSignIn URL
mokagio 6b1db97
Add `FIXME` note regarding GoogleSignIn URL error handling
mokagio ccdfc0b
Extract base URL for first step of GoogleSignIn flow and force unwrap it
mokagio b20fbe7
Throw error in iOS <= 15.0 branch if `URLComponents` fails to init
mokagio 5c5e160
Add object modelling request body for OAuth token
mokagio d5bdf02
Throw errors from `asURLEncodedData()`
mokagio 93c78bc
Add object modelling OAuth token response
mokagio 6e34f3a
Add method to create preconfigured `URLRequest` for OAuth token
mokagio 1798c5a
Add method to init `OAuthRequestBody` configured for GoogleSignIn
mokagio 39efd42
Remove an outdated TODO note
mokagio bccc9c3
Remove another outdated TODO note
mokagio d749c80
Simplify `googleSignInAuthURL` method
mokagio d5c6fd0
Do not use Alamofire code in `URL+GoogleSignIn.swift`
mokagio e963a39
Add additional documentation references to PKCE type
mokagio 8abbb29
Rename PKCE `Mode` to `Method` to be in line with spec
mokagio 8b6f6a8
Remove redundant error handling from `asURLEncodedData`
mokagio File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
24 changes: 24 additions & 0 deletions
24
WordPressAuthenticator/GoogleSignIn/OAuthRequestBody+GoogleSignIn.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| extension OAuthTokenRequestBody { | ||
|
|
||
| static func googleSignInRequestBody( | ||
| clientId: String, | ||
| authCode: String, | ||
| pkce: ProofKeyForCodeExchange | ||
| ) -> Self { | ||
| .init( | ||
| clientId: clientId, | ||
| // "The client secret obtained from the API Console Credentials page." | ||
| // - https://developers.google.com/identity/protocols/oauth2/native-app#step-2:-send-a-request-to-googles-oauth-2.0-server | ||
| // | ||
| // There doesn't seem to be any secret for iOS app credentials. | ||
| // The process works with an empty string... | ||
| clientSecret: "", | ||
| code: authCode, | ||
| codeVerifier: pkce.codeVerifier, | ||
| // As defined in the OAuth 2.0 specification, this field's value must be set to authorization_code. | ||
| // – https://developers.google.com/identity/protocols/oauth2/native-app#exchange-authorization-code | ||
| grantType: "authorization_code", | ||
| redirectURI: URL.redirectURI(from: clientId) | ||
| ) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,26 +2,46 @@ import Foundation | |
|
|
||
| extension URL { | ||
|
|
||
| // TODO: This is incomplete | ||
| static func googleSignInAuthURL(clientId: String) throws -> URL { | ||
| let baseURL = "https://accounts.google.com/o/oauth2/v2/auth" | ||
| // It's acceptable to force-unwrap here because, for this call to fail we'd need a developer | ||
| // error, which we would catch because the unit tests would crash. | ||
| static var googleSignInBaseURL = URL(string: "https://accounts.google.com/o/oauth2/v2/auth")! | ||
|
|
||
| static func googleSignInAuthURL(clientId: String, pkce: ProofKeyForCodeExchange) throws -> URL { | ||
| let queryItems = [ | ||
| ("client_id", clientId), | ||
| ("code_challenge", pkce.codeCallenge), | ||
| ("code_challenge_method", pkce.method.urlQueryParameterValue), | ||
| ("redirect_uri", redirectURI(from: clientId)), | ||
| ("response_type", "code") | ||
| ("response_type", "code"), | ||
| // TODO: We might want to add some of these or them configurable | ||
| // | ||
| // The request we make with the SDK asks for: | ||
| // | ||
| // - profile | ||
| // - https://www.googleapis.com/auth/userinfo.email | ||
| // - https://www.googleapis.com/auth/userinfo.profile | ||
| // - openid | ||
| // | ||
| // See https://developers.google.com/identity/protocols/oauth2/scopes | ||
| ("scope", "https://www.googleapis.com/auth/userinfo.email") | ||
|
Comment on lines
+16
to
+27
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spoiler: I updated this in my WIP branch but haven't seen any difference in the behavior, but that's likely due to my implementation being quite minimal at this point. Or, it's possible that only the backend would notice the difference, when completing the authentication using the OAuth token. |
||
| ].map { URLQueryItem(name: $0.0, value: $0.1) } | ||
|
|
||
| if #available(iOS 16.0, *) { | ||
| return URL(string: baseURL)!.appending(queryItems: queryItems) | ||
| return googleSignInBaseURL.appending(queryItems: queryItems) | ||
| } else { | ||
| var components = URLComponents(string: baseURL)! | ||
| // Given `googleSignInBaseURL` is assumed as a valid URL, a `URLComponents` instance | ||
| // should always be available. | ||
| var components = URLComponents(url: googleSignInBaseURL, resolvingAgainstBaseURL: false)! | ||
| components.queryItems = queryItems | ||
| return try components.asURL() | ||
| // Likewise, we can as long as the given `queryItems` are valid, we can assume `url` to | ||
| // not be nil. If `queryItems` are invalid, a developer error has been committed, and | ||
| // crashing is appropriate. | ||
| return components.url! | ||
| } | ||
| } | ||
|
|
||
| private static func redirectURI(from clientId: String) -> String { | ||
| static func redirectURI(from clientId: String) -> String { | ||
| // Google's client id is in the form: 123-abc245def.apps.googleusercontent.com | ||
| // The redirect URI uses the reverse-DNS notation. | ||
| let reverseDNSClientId = clientId.split(separator: ".").reversed().joined(separator: ".") | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| /// Models the request to send for an OAuth token | ||
| /// | ||
| /// - Note: See documentation at https://developers.google.com/identity/protocols/oauth2/native-app#exchange-authorization-code | ||
| struct OAuthTokenRequestBody: Encodable { | ||
| let clientId: String | ||
| let clientSecret: String | ||
| let code: String | ||
| let codeVerifier: String | ||
| let grantType: String | ||
| let redirectURI: String | ||
|
|
||
| enum CodingKeys: String, CodingKey { | ||
| case clientId = "client_id" | ||
| case clientSecret = "client_secret" | ||
| case code | ||
| case codeVerifier = "code_verifier" | ||
| case grantType = "grant_type" | ||
| case redirectURI = "redirect_uri" | ||
| } | ||
|
|
||
| func asURLEncodedData() throws -> Data { | ||
| let params = [ | ||
| (CodingKeys.clientId.rawValue, clientId), | ||
| (CodingKeys.clientSecret.rawValue, clientSecret), | ||
| (CodingKeys.code.rawValue, code), | ||
| (CodingKeys.codeVerifier.rawValue, codeVerifier), | ||
| (CodingKeys.grantType.rawValue, grantType), | ||
| (CodingKeys.redirectURI.rawValue, redirectURI), | ||
| ] | ||
|
|
||
| let items = params.map { URLQueryItem(name: $0.0, value: $0.1) } | ||
|
|
||
| var components = URLComponents() | ||
| components.queryItems = items | ||
|
|
||
| // We can assume `query` to never be nil because we set `queryItems` in the line above. | ||
| return Data(components.query!.utf8) | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we are essentially implement OAuth 2, I think we can replace "Google Sign In" with a more generic term? Like, this function becomes
oauth2AuthenticationURL, what do you think?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm making assumption here, but I imagine the whole Google Sign In workflow can be used in "Sign In via Facebook", or even the WordPress OAuth2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I was thinking about this before going AFK and realized I may be using unclear boundaries here.
You might have noticed I made an
OAuthfolder and aGoogleSignInone. My intent was to separate the components that are GoogleSignIn specific from those that are part of the OAuth spec. One issue with how I executed this is that I haven't been cross checking between the OAuth and Google documentations so there might be stuff in the OAuth folder that's still Google specific, and vice versa.Thinking about it since, I came to the conclusion that I'd prefer to separate OAuth- from Google- specific code only once we'll need to add a new OAuth client (likely WordPress OAuth2). As such, I'm tempted to follow up this PR with one that moves all the
OAuth/code intoGoogleSignIn/, renaming some of the types if necessary.My rationale for the suggestion above is that I'm more interested in having a working version of the SDK-less authentication than a general purpose OAuth client, even though building a client first might make it easier to add further authentication implementation in the future.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me. We probably don't need to "pre-abstract" stuff if there is no immediate plan to support another OAuth provider (service? server?). I trust you'll strike the right balance 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the confidence.