Skip to content

Commit 6d9f315

Browse files
hfLashaJini
authored andcommitted
feat: complete OIDC support for Apple and Google providers (supabase#1108)
Previously OIDC sign in (i.e. sign-in using an ID token) for Apple, Google and a few other providers was not properly supported. There was no account linking available, and there were a few security issues found with the implementation. This PR attempts to resolve all of the issues, specifically targeting Apple and Google providers, which enables native Sign in with Apple and Google with mobile or desktop apps. Furthermore, this PR paves the way towards SSO with OIDC support. Basically, the whole `POST /token?grant_type=id_token` endpoint is refactored to use the central `createAccountFromExternalIdentity` method which supports both regular and SSO accounts with automatic account linking. For both Apple and Google flows, the important thing to realize is that their OAuth2 flows are in-fact OIDC authentication flows. The Apple OAuth2 flow already used the Apple OIDC ID token to extract user information. The Google OAuth2 flow is refactored to use the OIDC ID token when available (appears to be always) or fall back to the previous implementation. Since it does not matter whether the flow is OAuth2 or OIDC, automatic account linking can take place. The remaining OIDC supported providers -- Azure, Facebook, Keycloak -- remain supported though with upgraded account linking support; however their implementations are best-effort at this point. Furthermore, the Keycloak implementation should be deprecated as it's actually solving a SSO-with-OIDC problem.
1 parent 82c7b07 commit 6d9f315

40 files changed

+630
-478
lines changed

internal/api/external.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ func (a *API) Provider(ctx context.Context, name string, scopes string) (provide
514514

515515
switch name {
516516
case "apple":
517-
return provider.NewAppleProvider(config.External.Apple)
517+
return provider.NewAppleProvider(ctx, config.External.Apple)
518518
case "azure":
519519
return provider.NewAzureProvider(config.External.Azure, scopes)
520520
case "bitbucket":
@@ -526,7 +526,7 @@ func (a *API) Provider(ctx context.Context, name string, scopes string) (provide
526526
case "gitlab":
527527
return provider.NewGitlabProvider(config.External.Gitlab, scopes)
528528
case "google":
529-
return provider.NewGoogleProvider(config.External.Google, scopes)
529+
return provider.NewGoogleProvider(ctx, config.External.Google, scopes)
530530
case "kakao":
531531
return provider.NewKakaoProvider(config.External.Kakao, scopes)
532532
case "keycloak":

internal/api/external_apple_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ func (ts *ExternalTestSuite) TestSignupExternalApple() {
1717
ts.Require().NoError(err, "redirect url parse failed")
1818
q := u.Query()
1919
ts.Equal(ts.Config.External.Apple.RedirectURI, q.Get("redirect_uri"))
20-
ts.Equal(ts.Config.External.Apple.ClientID, q.Get("client_id"))
20+
ts.Equal(ts.Config.External.Apple.ClientID, []string{q.Get("client_id")})
2121
ts.Equal("code", q.Get("response_type"))
2222
ts.Equal("email name", q.Get("scope"))
2323

internal/api/external_azure_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ func (ts *ExternalTestSuite) TestSignupExternalAzure() {
2323
ts.Require().NoError(err, "redirect url parse failed")
2424
q := u.Query()
2525
ts.Equal(ts.Config.External.Azure.RedirectURI, q.Get("redirect_uri"))
26-
ts.Equal(ts.Config.External.Azure.ClientID, q.Get("client_id"))
26+
ts.Equal(ts.Config.External.Azure.ClientID, []string{q.Get("client_id")})
2727
ts.Equal("code", q.Get("response_type"))
2828
ts.Equal("openid", q.Get("scope"))
2929

internal/api/external_bitbucket_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func (ts *ExternalTestSuite) TestSignupExternalBitbucket() {
2222
ts.Require().NoError(err, "redirect url parse failed")
2323
q := u.Query()
2424
ts.Equal(ts.Config.External.Bitbucket.RedirectURI, q.Get("redirect_uri"))
25-
ts.Equal(ts.Config.External.Bitbucket.ClientID, q.Get("client_id"))
25+
ts.Equal(ts.Config.External.Bitbucket.ClientID, []string{q.Get("client_id")})
2626
ts.Equal("code", q.Get("response_type"))
2727
ts.Equal("account email", q.Get("scope"))
2828

internal/api/external_discord_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func (ts *ExternalTestSuite) TestSignupExternalDiscord() {
2424
ts.Require().NoError(err, "redirect url parse failed")
2525
q := u.Query()
2626
ts.Equal(ts.Config.External.Discord.RedirectURI, q.Get("redirect_uri"))
27-
ts.Equal(ts.Config.External.Discord.ClientID, q.Get("client_id"))
27+
ts.Equal(ts.Config.External.Discord.ClientID, []string{q.Get("client_id")})
2828
ts.Equal("code", q.Get("response_type"))
2929
ts.Equal("email identify", q.Get("scope"))
3030

internal/api/external_facebook_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func (ts *ExternalTestSuite) TestSignupExternalFacebook() {
2424
ts.Require().NoError(err, "redirect url parse failed")
2525
q := u.Query()
2626
ts.Equal(ts.Config.External.Facebook.RedirectURI, q.Get("redirect_uri"))
27-
ts.Equal(ts.Config.External.Facebook.ClientID, q.Get("client_id"))
27+
ts.Equal(ts.Config.External.Facebook.ClientID, []string{q.Get("client_id")})
2828
ts.Equal("code", q.Get("response_type"))
2929
ts.Equal("email", q.Get("scope"))
3030

internal/api/external_github_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func (ts *ExternalTestSuite) TestSignupExternalGithub() {
2525
ts.Require().NoError(err, "redirect url parse failed")
2626
q := u.Query()
2727
ts.Equal(ts.Config.External.Github.RedirectURI, q.Get("redirect_uri"))
28-
ts.Equal(ts.Config.External.Github.ClientID, q.Get("client_id"))
28+
ts.Equal(ts.Config.External.Github.ClientID, []string{q.Get("client_id")})
2929
ts.Equal("code", q.Get("response_type"))
3030
ts.Equal("user:email", q.Get("scope"))
3131

internal/api/external_gitlab_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func (ts *ExternalTestSuite) TestSignupExternalGitlab() {
2424
ts.Require().NoError(err, "redirect url parse failed")
2525
q := u.Query()
2626
ts.Equal(ts.Config.External.Gitlab.RedirectURI, q.Get("redirect_uri"))
27-
ts.Equal(ts.Config.External.Gitlab.ClientID, q.Get("client_id"))
27+
ts.Equal(ts.Config.External.Gitlab.ClientID, []string{q.Get("client_id")})
2828
ts.Equal("code", q.Get("response_type"))
2929
ts.Equal("read_user", q.Get("scope"))
3030

internal/api/external_google_test.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
package api
22

33
import (
4+
"encoding/json"
45
"fmt"
56
"net/http"
67
"net/http/httptest"
78
"net/url"
89

910
jwt "github.com/golang-jwt/jwt"
11+
"github.com/stretchr/testify/require"
12+
"github.com/supabase/gotrue/internal/api/provider"
1013
)
1114

1215
const (
@@ -16,6 +19,8 @@ const (
1619
)
1720

1821
func (ts *ExternalTestSuite) TestSignupExternalGoogle() {
22+
provider.ResetGoogleProvider()
23+
1924
req := httptest.NewRequest(http.MethodGet, "http://localhost/authorize?provider=google", nil)
2025
w := httptest.NewRecorder()
2126
ts.API.handler.ServeHTTP(w, req)
@@ -24,7 +29,7 @@ func (ts *ExternalTestSuite) TestSignupExternalGoogle() {
2429
ts.Require().NoError(err, "redirect url parse failed")
2530
q := u.Query()
2631
ts.Equal(ts.Config.External.Google.RedirectURI, q.Get("redirect_uri"))
27-
ts.Equal(ts.Config.External.Google.ClientID, q.Get("client_id"))
32+
ts.Equal(ts.Config.External.Google.ClientID, []string{q.Get("client_id")})
2833
ts.Equal("code", q.Get("response_type"))
2934
ts.Equal("email profile", q.Get("scope"))
3035

@@ -40,8 +45,17 @@ func (ts *ExternalTestSuite) TestSignupExternalGoogle() {
4045
}
4146

4247
func GoogleTestSignupSetup(ts *ExternalTestSuite, tokenCount *int, userCount *int, code string, user string) *httptest.Server {
43-
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
48+
provider.ResetGoogleProvider()
49+
50+
var server *httptest.Server
51+
server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
4452
switch r.URL.Path {
53+
case "/.well-known/openid-configuration":
54+
w.Header().Add("Content-Type", "application/json")
55+
require.NoError(ts.T(), json.NewEncoder(w).Encode(map[string]any{
56+
"issuer": server.URL,
57+
"token_endpoint": server.URL + "/o/oauth2/token",
58+
}))
4559
case "/o/oauth2/token":
4660
*tokenCount++
4761
ts.Equal(code, r.FormValue("code"))
@@ -60,7 +74,7 @@ func GoogleTestSignupSetup(ts *ExternalTestSuite, tokenCount *int, userCount *in
6074
}
6175
}))
6276

63-
ts.Config.External.Google.URL = server.URL
77+
provider.OverrideGoogleProvider(server.URL, server.URL+"/userinfo/v2/me")
6478

6579
return server
6680
}

internal/api/external_kakao_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ func (ts *ExternalTestSuite) TestSignupExternalKakao() {
2323
ts.Require().NoError(err, "redirect url parse failed")
2424
q := u.Query()
2525
ts.Equal(ts.Config.External.Kakao.RedirectURI, q.Get("redirect_uri"))
26-
ts.Equal(ts.Config.External.Kakao.ClientID, q.Get("client_id"))
26+
ts.Equal(ts.Config.External.Kakao.ClientID, []string{q.Get("client_id")})
2727
ts.Equal("code", q.Get("response_type"))
2828

2929
claims := ExternalProviderClaims{}

0 commit comments

Comments
 (0)