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.

Sign up for GitHub

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

Jump to bottom

Removed Provider For Apple Login In getOAuthSignInURL method #67

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Removed Provider For Apple Login In getOAuthSignInURL method #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4169ed6
Removed Apple as a provider for getOAuthSignInURL method
ljubiknenad Aug 3, 2023
a6b6c4e
Removed experimental label on signInWithIdToken method
ljubiknenad Aug 4, 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
1 change: 0 additions & 1 deletion Sources/GoTrue/GoTrueClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,6 @@ public final class GoTrueClient {

/// Allows signing in with an ID token issued by certain supported providers.
/// The ID token is verified for validity and a new session is established.
@_spi(Experimental)
@discardableResult
public func signInWithIdToken(credentials: OpenIDConnectCredentials) async throws -> Session {
try await _signIn(
Expand Down
3 changes: 1 addition & 2 deletions Sources/GoTrue/Types.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,6 @@ public struct UserIdentity: Codable, Hashable, Identifiable, Sendable {
}

public enum Provider: String, Codable, CaseIterable, Sendable {
case apple
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I follow why the removal of apple as a provider.

Copy link
Author

Choose a reason for hiding this comment

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

Because we don't need it in the getOAuthSignInURL method. So the users can know in the documentation on this branch (for which i will create a PR) that the only method for Apple Sign In is with the signInWithIdToken method.

We don't want to offer Apple as a provider for getOAuthSignInURL if it's not usable

Copy link
Contributor

Choose a reason for hiding this comment

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

@ljubiknenad The way I think about this is that we should provide a way for the user to sign in using the getOAuthSignInURL even if it makes more sense to use native sign in.

Also removing the apple as a provider will throw an error when decoding a user that was signed in using apple, as apple isn't available on the enum.

Copy link
Author

Choose a reason for hiding this comment

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

@GRSouza if we provide a way for the user to sign in using getOAuthSignInURL, won't the developer of the app be rejected from Apple? Because in Apple's documents it says:

To support Sign in with Apple for iOS, macOS, tvOS, and watchOS apps, see Implementing User Authentication with Sign in with Apple. For website support, see Sign in with Apple JS, and use the Sign in with Apple REST API to communicate with Apple servers.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think to provide a feature-complete library and just warn the user that he shouldn't use the getOAuthSignInURL flow to submit to App Store. It's not on us to decide how the user will implement that, we just provide the features and make recommendations for implementation.

case azure
case bitbucket
case discord
Expand Down Expand Up @@ -340,7 +339,7 @@ public struct OpenIDConnectCredentials: Codable, Hashable, Sendable {
public var gotrueMetaSecurity: GoTrueMetaSecurity?

public init(
provider: Provider? = nil,
provider: Provider?,
idToken: String,
accessToken: String? = nil,
nonce: String? = nil,
Expand Down
Loading