Skip to content

Commit cb79a74

Browse files
fadymakfadymak
authored
fix: additional provider and issuer checks (#2326)
Adds additional issuer checks
1 parent b9d0500 commit cb79a74

File tree

2 files changed

+131
-18
lines changed

2 files changed

+131
-18
lines changed

‎internal/api/token_oidc.go‎

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"fmt"
77
"net/http"
88
"slices"
9+
"strings"
910

1011
"github.com/coreos/go-oidc/v3/oidc"
1112
"github.com/supabase/auth/internal/api/apierrors"
@@ -36,23 +37,29 @@ func (p *IdTokenGrantParams) getProvider(ctx context.Context, config *conf.Globa
3637
var providerType string
3738
var acceptableClientIDs []string
3839

40+
if p.Issuer != "" {
41+
log.WithField("issuer", p.Issuer).WithField("provider", p.Provider).Info("Issuer provided in request.")
42+
}
43+
3944
switch true {
4045
case p.Provider == "apple" || provider.IsAppleIssuer(p.Issuer):
4146
cfg = &config.External.Apple
4247
providerType = "apple"
43-
issuer = p.Issuer
44-
if issuer == "" {
45-
detectedIssuer, err := provider.DetectAppleIDTokenIssuer(ctx, p.IdToken)
46-
if err != nil {
47-
return nil, false, "", nil, false, apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Unable to detect issuer in ID token for Apple provider").WithInternalError(err)
48-
}
4948

50-
if provider.IsAppleIssuer(detectedIssuer) {
51-
issuer = detectedIssuer
52-
} else {
53-
return nil, false, "", nil, false, apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Detected ID token issuer is not an Apple ID token issuer")
54-
}
49+
detectedIssuer, err := provider.DetectAppleIDTokenIssuer(ctx, p.IdToken)
50+
if err != nil {
51+
return nil, false, "", nil, false, apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Unable to detect issuer in ID token for Apple provider").WithInternalError(err)
52+
}
53+
54+
if !provider.IsAppleIssuer(detectedIssuer) {
55+
return nil, false, "", nil, false, apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Detected ID token issuer is not an Apple ID token issuer")
56+
}
57+
58+
if p.Issuer != "" && p.Issuer != detectedIssuer {
59+
return nil, false, "", nil, false, apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Provided issuer does not match ID token issuer")
5560
}
61+
62+
issuer = detectedIssuer
5663
acceptableClientIDs = append(acceptableClientIDs, config.External.Apple.ClientID...)
5764

5865
if config.External.IosBundleId != "" {
@@ -66,14 +73,20 @@ func (p *IdTokenGrantParams) getProvider(ctx context.Context, config *conf.Globa
6673
acceptableClientIDs = append(acceptableClientIDs, config.External.Google.ClientID...)
6774

6875
case p.Provider == "azure" || provider.IsAzureIssuer(p.Issuer):
69-
issuer = p.Issuer
70-
if issuer == "" || !provider.IsAzureIssuer(issuer) {
71-
detectedIssuer, err := provider.DetectAzureIDTokenIssuer(ctx, p.IdToken)
72-
if err != nil {
73-
return nil, false, "", nil, false, apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Unable to detect issuer in ID token for Azure provider").WithInternalError(err)
74-
}
75-
issuer = detectedIssuer
76+
detectedIssuer, err := provider.DetectAzureIDTokenIssuer(ctx, p.IdToken)
77+
if err != nil {
78+
return nil, false, "", nil, false, apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Unable to detect issuer in ID token for Azure provider").WithInternalError(err)
79+
}
80+
81+
if !strings.HasPrefix(detectedIssuer, "https://login.microsoftonline.com/") && !strings.HasPrefix(detectedIssuer, "https://sts.windows.net/") {
82+
return nil, false, "", nil, false, apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Detected ID token issuer is not an Azure ID token issuer")
83+
}
84+
85+
if p.Issuer != "" && p.Issuer != detectedIssuer {
86+
return nil, false, "", nil, false, apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "Provided issuer does not match ID token issuer")
7687
}
88+
89+
issuer = detectedIssuer
7790
cfg = &config.External.Azure
7891
providerType = "azure"
7992
acceptableClientIDs = append(acceptableClientIDs, config.External.Azure.ClientID...)

‎internal/api/token_oidc_test.go‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package api
22

33
import (
44
"context"
5+
"encoding/base64"
6+
"encoding/json"
57
"net/http"
68
"net/http/httptest"
79
"testing"
@@ -68,3 +70,101 @@ func (ts *TokenOIDCTestSuite) TestGetProvider() {
6870
require.Equal(ts.T(), params.Provider, providerType)
6971
require.NotEmpty(ts.T(), acceptableClientIds)
7072
}
73+
74+
// createFakeIDToken creates a fake JWT token with a specific issuer claim for testing
75+
// WARNING: This is for testing purposes only and creates an unsigned token
76+
func createFakeIDToken(issuer string, sub string) string {
77+
header := map[string]interface{}{
78+
"alg": "RS256",
79+
"typ": "JWT",
80+
}
81+
82+
payload := map[string]interface{}{
83+
"iss": issuer,
84+
"sub": sub,
85+
"aud": "test-client-id",
86+
"exp": 9999999999,
87+
"iat": 1234567890,
88+
}
89+
90+
headerJSON, _ := json.Marshal(header)
91+
payloadJSON, _ := json.Marshal(payload)
92+
93+
headerEncoded := base64.RawURLEncoding.EncodeToString(headerJSON)
94+
payloadEncoded := base64.RawURLEncoding.EncodeToString(payloadJSON)
95+
96+
// Note: signature is fake, but the issuer detection only looks at the payload
97+
return headerEncoded + "." + payloadEncoded + ".fake-signature"
98+
}
99+
100+
func (ts *TokenOIDCTestSuite) TestGetProviderAppleWithIncorrectIssuer() {
101+
incorrectIssuer := SetupTestOIDCProvider(ts)
102+
defer incorrectIssuer.Close()
103+
104+
ts.Config.External.Apple.Enabled = true
105+
ts.Config.External.Apple.ClientID = []string{"com.example.app"}
106+
107+
// Create a token with an invalid issuer
108+
nonAppleToken := createFakeIDToken(incorrectIssuer.URL, "user123")
109+
110+
// provider="apple" but with an incorrect issuer in the token
111+
params := &IdTokenGrantParams{
112+
IdToken: nonAppleToken,
113+
Provider: "apple",
114+
Issuer: incorrectIssuer.URL,
115+
}
116+
117+
req := httptest.NewRequest(http.MethodPost, "http://localhost", nil)
118+
_, _, _, _, _, err := params.getProvider(context.Background(), ts.Config, req)
119+
120+
require.Error(ts.T(), err)
121+
require.Contains(ts.T(), err.Error(), "not an Apple ID token issuer")
122+
}
123+
124+
// TestGetProviderAzureWithNonAzureTokenIssuer tests that Azure provider only
125+
// accepts tokens from login.microsoftonline.com and sts.windows.net
126+
func (ts *TokenOIDCTestSuite) TestGetProviderAzureWithNonAzureTokenIssuer() {
127+
ts.Config.External.Azure.Enabled = true
128+
ts.Config.External.Azure.ClientID = []string{"test-client-id"}
129+
130+
// Create a token with an incorrect issuer
131+
nonAzureIssuer := "https://non-azure-issuer.example.com"
132+
nonAzureToken := createFakeIDToken(nonAzureIssuer, "user123")
133+
134+
params := &IdTokenGrantParams{
135+
IdToken: nonAzureToken,
136+
Provider: "azure",
137+
Issuer: nonAzureIssuer,
138+
}
139+
140+
req := httptest.NewRequest(http.MethodPost, "http://localhost", nil)
141+
_, _, _, _, _, err := params.getProvider(context.Background(), ts.Config, req)
142+
143+
// This should fail - the token's issuer is not an accepted issuer
144+
require.Error(ts.T(), err)
145+
require.Contains(ts.T(), err.Error(), "not an Azure ID token issuer")
146+
}
147+
148+
// TestGetProviderAppleWithInvalidIssuerInToken tests that Apple provider rejects
149+
// tokens when the actual token issuer does not match the expected issuer
150+
func (ts *TokenOIDCTestSuite) TestGetProviderAppleWithNonAppleIssuerInToken() {
151+
ts.Config.External.Apple.Enabled = true
152+
ts.Config.External.Apple.ClientID = []string{"com.example.app"}
153+
154+
// Create a token with an incorrect issuer
155+
nonAppleIssuer := "https://non-apple-issuer.example.com"
156+
nonAppleToken := createFakeIDToken(nonAppleIssuer, "user123")
157+
158+
params := &IdTokenGrantParams{
159+
IdToken: nonAppleToken,
160+
Provider: "apple",
161+
Issuer: "https://appleid.apple.com",
162+
}
163+
164+
req := httptest.NewRequest(http.MethodPost, "http://localhost", nil)
165+
_, _, _, _, _, err := params.getProvider(context.Background(), ts.Config, req)
166+
167+
// This should fail - the token's actual issuer is not appleid.apple.com
168+
require.Error(ts.T(), err)
169+
require.Contains(ts.T(), err.Error(), "not an Apple ID token issuer")
170+
}

0 commit comments

Comments
 (0)