Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd0a534e

Browse files
authored
chore: prevent authentication of non-unique oidc subjects (#16498)
Any IdP returning an empty field here breaks the assumption of aunique subject id. This is defined in the OIDC spec.
1 parent695d552 commitd0a534e

File tree

6 files changed

+92
-1
lines changed

6 files changed

+92
-1
lines changed

‎coderd/oauthpki/okidcpki_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313

1414
"github.com/coreos/go-oidc/v3/oidc"
1515
"github.com/golang-jwt/jwt/v4"
16+
"github.com/google/uuid"
1617
"github.com/stretchr/testify/assert"
1718
"github.com/stretchr/testify/require"
1819
"golang.org/x/oauth2"
@@ -169,6 +170,7 @@ func TestAzureAKPKIWithCoderd(t *testing.T) {
169170
constemail="alice@coder.com"
170171
claims:= jwt.MapClaims{
171172
"email":email,
173+
"sub":uuid.NewString(),
172174
}
173175
helper:=oidctest.NewLoginHelper(owner,fake)
174176
user,_:=helper.Login(t,claims)

‎coderd/userauth.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,6 +1112,20 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
11121112
return
11131113
}
11141114

1115+
ifidToken.Subject=="" {
1116+
logger.Error(ctx,"oauth2: missing 'sub' claim field in OIDC token",
1117+
slog.F("source","id_token"),
1118+
slog.F("claim_fields",claimFields(idtokenClaims)),
1119+
slog.F("blank",blankFields(idtokenClaims)),
1120+
)
1121+
httpapi.Write(ctx,rw,http.StatusBadRequest, codersdk.Response{
1122+
Message:"OIDC token missing 'sub' claim field or 'sub' claim field is empty.",
1123+
Detail:"'sub' claim field is required to be unique for all users by a given issue, "+
1124+
"an empty field is invalid and this authentication attempt is rejected.",
1125+
})
1126+
return
1127+
}
1128+
11151129
logger.Debug(ctx,"got oidc claims",
11161130
slog.F("source","id_token"),
11171131
slog.F("claim_fields",claimFields(idtokenClaims)),

‎coderd/userauth_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ func TestOIDCOauthLoginWithExisting(t *testing.T) {
7272
"email":"alice@coder.com",
7373
"email_verified":true,
7474
"preferred_username":username,
75+
"sub":uuid.NewString(),
7576
}
7677

7778
helper:=oidctest.NewLoginHelper(client,fake)
@@ -899,10 +900,19 @@ func TestUserOIDC(t *testing.T) {
899900
IgnoreEmailVerifiedbool
900901
IgnoreUserInfobool
901902
}{
903+
{
904+
Name:"NoSub",
905+
IDTokenClaims: jwt.MapClaims{
906+
"email":"kyle@kwc.io",
907+
},
908+
AllowSignups:true,
909+
StatusCode:http.StatusBadRequest,
910+
},
902911
{
903912
Name:"EmailOnly",
904913
IDTokenClaims: jwt.MapClaims{
905914
"email":"kyle@kwc.io",
915+
"sub":uuid.NewString(),
906916
},
907917
AllowSignups:true,
908918
StatusCode:http.StatusOK,
@@ -915,6 +925,7 @@ func TestUserOIDC(t *testing.T) {
915925
IDTokenClaims: jwt.MapClaims{
916926
"email":"kyle@kwc.io",
917927
"email_verified":false,
928+
"sub":uuid.NewString(),
918929
},
919930
AllowSignups:true,
920931
StatusCode:http.StatusForbidden,
@@ -924,6 +935,7 @@ func TestUserOIDC(t *testing.T) {
924935
IDTokenClaims: jwt.MapClaims{
925936
"email":3.14159,
926937
"email_verified":false,
938+
"sub":uuid.NewString(),
927939
},
928940
AllowSignups:true,
929941
StatusCode:http.StatusBadRequest,
@@ -933,6 +945,7 @@ func TestUserOIDC(t *testing.T) {
933945
IDTokenClaims: jwt.MapClaims{
934946
"email":"kyle@kwc.io",
935947
"email_verified":false,
948+
"sub":uuid.NewString(),
936949
},
937950
AllowSignups:true,
938951
StatusCode:http.StatusOK,
@@ -946,6 +959,7 @@ func TestUserOIDC(t *testing.T) {
946959
IDTokenClaims: jwt.MapClaims{
947960
"email":"kyle@kwc.io",
948961
"email_verified":true,
962+
"sub":uuid.NewString(),
949963
},
950964
AllowSignups:true,
951965
EmailDomain: []string{
@@ -958,6 +972,7 @@ func TestUserOIDC(t *testing.T) {
958972
IDTokenClaims: jwt.MapClaims{
959973
"email":"cian@coder.com",
960974
"email_verified":true,
975+
"sub":uuid.NewString(),
961976
},
962977
AllowSignups:true,
963978
EmailDomain: []string{
@@ -970,6 +985,7 @@ func TestUserOIDC(t *testing.T) {
970985
IDTokenClaims: jwt.MapClaims{
971986
"email":"kyle@kwc.io",
972987
"email_verified":true,
988+
"sub":uuid.NewString(),
973989
},
974990
AllowSignups:true,
975991
EmailDomain: []string{
@@ -982,6 +998,7 @@ func TestUserOIDC(t *testing.T) {
982998
IDTokenClaims: jwt.MapClaims{
983999
"email":"kyle@KWC.io",
9841000
"email_verified":true,
1001+
"sub":uuid.NewString(),
9851002
},
9861003
AllowSignups:true,
9871004
AssertUser:func(t testing.TB,u codersdk.User) {
@@ -997,6 +1014,7 @@ func TestUserOIDC(t *testing.T) {
9971014
IDTokenClaims: jwt.MapClaims{
9981015
"email":"colin@gmail.com",
9991016
"email_verified":true,
1017+
"sub":uuid.NewString(),
10001018
},
10011019
AllowSignups:true,
10021020
EmailDomain: []string{
@@ -1015,6 +1033,7 @@ func TestUserOIDC(t *testing.T) {
10151033
IDTokenClaims: jwt.MapClaims{
10161034
"email":"kyle@kwc.io",
10171035
"email_verified":true,
1036+
"sub":uuid.NewString(),
10181037
},
10191038
StatusCode:http.StatusForbidden,
10201039
},
@@ -1023,6 +1042,7 @@ func TestUserOIDC(t *testing.T) {
10231042
IDTokenClaims: jwt.MapClaims{
10241043
"email":"kyle@kwc.io",
10251044
"email_verified":true,
1045+
"sub":uuid.NewString(),
10261046
},
10271047
AssertUser:func(t testing.TB,u codersdk.User) {
10281048
assert.Equal(t,"kyle",u.Username)
@@ -1036,6 +1056,7 @@ func TestUserOIDC(t *testing.T) {
10361056
"email":"kyle@kwc.io",
10371057
"email_verified":true,
10381058
"preferred_username":"hotdog",
1059+
"sub":uuid.NewString(),
10391060
},
10401061
AssertUser:func(t testing.TB,u codersdk.User) {
10411062
assert.Equal(t,"hotdog",u.Username)
@@ -1049,6 +1070,7 @@ func TestUserOIDC(t *testing.T) {
10491070
"email":"kyle@kwc.io",
10501071
"email_verified":true,
10511072
"name":"Hot Dog",
1073+
"sub":uuid.NewString(),
10521074
},
10531075
AssertUser:func(t testing.TB,u codersdk.User) {
10541076
assert.Equal(t,"Hot Dog",u.Name)
@@ -1065,6 +1087,7 @@ func TestUserOIDC(t *testing.T) {
10651087
// However, we should not fail to log someone in if their name is too long.
10661088
// Just truncate it.
10671089
"name":strings.Repeat("a",129),
1090+
"sub":uuid.NewString(),
10681091
},
10691092
AllowSignups:true,
10701093
StatusCode:http.StatusOK,
@@ -1080,6 +1103,7 @@ func TestUserOIDC(t *testing.T) {
10801103
// Full names must not have leading or trailing whitespace, but this is a
10811104
// daft reason to fail a login.
10821105
"name":" Bobby Whitespace ",
1106+
"sub":uuid.NewString(),
10831107
},
10841108
AllowSignups:true,
10851109
StatusCode:http.StatusOK,
@@ -1096,6 +1120,7 @@ func TestUserOIDC(t *testing.T) {
10961120
"email_verified":true,
10971121
"name":"Kylium Carbonate",
10981122
"preferred_username":"kyle@kwc.io",
1123+
"sub":uuid.NewString(),
10991124
},
11001125
AssertUser:func(t testing.TB,u codersdk.User) {
11011126
assert.Equal(t,"kyle",u.Username)
@@ -1108,6 +1133,7 @@ func TestUserOIDC(t *testing.T) {
11081133
Name:"UsernameIsEmail",
11091134
IDTokenClaims: jwt.MapClaims{
11101135
"preferred_username":"kyle@kwc.io",
1136+
"sub":uuid.NewString(),
11111137
},
11121138
AssertUser:func(t testing.TB,u codersdk.User) {
11131139
assert.Equal(t,"kyle",u.Username)
@@ -1123,6 +1149,7 @@ func TestUserOIDC(t *testing.T) {
11231149
"email_verified":true,
11241150
"preferred_username":"kyle",
11251151
"picture":"/example.png",
1152+
"sub":uuid.NewString(),
11261153
},
11271154
AssertUser:func(t testing.TB,u codersdk.User) {
11281155
assert.Equal(t,"/example.png",u.AvatarURL)
@@ -1136,6 +1163,7 @@ func TestUserOIDC(t *testing.T) {
11361163
IDTokenClaims: jwt.MapClaims{
11371164
"email":"kyle@kwc.io",
11381165
"email_verified":true,
1166+
"sub":uuid.NewString(),
11391167
},
11401168
UserInfoClaims: jwt.MapClaims{
11411169
"preferred_username":"potato",
@@ -1155,6 +1183,7 @@ func TestUserOIDC(t *testing.T) {
11551183
IDTokenClaims: jwt.MapClaims{
11561184
"email":"coolin@coder.com",
11571185
"groups": []string{"pingpong"},
1186+
"sub":uuid.NewString(),
11581187
},
11591188
AllowSignups:true,
11601189
StatusCode:http.StatusOK,
@@ -1164,6 +1193,7 @@ func TestUserOIDC(t *testing.T) {
11641193
IDTokenClaims: jwt.MapClaims{
11651194
"email":"internaluser@internal.domain",
11661195
"email_verified":false,
1196+
"sub":uuid.NewString(),
11671197
},
11681198
UserInfoClaims: jwt.MapClaims{
11691199
"email":"externaluser@external.domain",
@@ -1182,6 +1212,7 @@ func TestUserOIDC(t *testing.T) {
11821212
IDTokenClaims: jwt.MapClaims{
11831213
"email":"internaluser@internal.domain",
11841214
"email_verified":false,
1215+
"sub":uuid.NewString(),
11851216
},
11861217
UserInfoClaims: jwt.MapClaims{
11871218
"email":1,
@@ -1197,6 +1228,7 @@ func TestUserOIDC(t *testing.T) {
11971228
"email_verified":true,
11981229
"name":"User McName",
11991230
"preferred_username":"user",
1231+
"sub":uuid.NewString(),
12001232
},
12011233
UserInfoClaims: jwt.MapClaims{
12021234
"email":"user.mcname@external.domain",
@@ -1216,6 +1248,7 @@ func TestUserOIDC(t *testing.T) {
12161248
IDTokenClaims:inflateClaims(t, jwt.MapClaims{
12171249
"email":"user@domain.tld",
12181250
"email_verified":true,
1251+
"sub":uuid.NewString(),
12191252
},65536),
12201253
AssertUser:func(t testing.TB,u codersdk.User) {
12211254
assert.Equal(t,"user",u.Username)
@@ -1228,6 +1261,7 @@ func TestUserOIDC(t *testing.T) {
12281261
IDTokenClaims: jwt.MapClaims{
12291262
"email":"user@domain.tld",
12301263
"email_verified":true,
1264+
"sub":uuid.NewString(),
12311265
},
12321266
UserInfoClaims:inflateClaims(t, jwt.MapClaims{},65536),
12331267
AssertUser:func(t testing.TB,u codersdk.User) {
@@ -1242,6 +1276,7 @@ func TestUserOIDC(t *testing.T) {
12421276
"iss":"https://mismatch.com",
12431277
"email":"user@domain.tld",
12441278
"email_verified":true,
1279+
"sub":uuid.NewString(),
12451280
},
12461281
AllowSignups:true,
12471282
StatusCode:http.StatusBadRequest,
@@ -1331,6 +1366,7 @@ func TestUserOIDC(t *testing.T) {
13311366

13321367
client,resp:=fake.AttemptLogin(t,owner, jwt.MapClaims{
13331368
"email":user.Email,
1369+
"sub":uuid.NewString(),
13341370
})
13351371
require.Equal(t,http.StatusOK,resp.StatusCode)
13361372

@@ -1369,6 +1405,7 @@ func TestUserOIDC(t *testing.T) {
13691405

13701406
claims:= jwt.MapClaims{
13711407
"email":userData.Email,
1408+
"sub":uuid.NewString(),
13721409
}
13731410
varerrerror
13741411
user.HTTPClient.Jar,err=cookiejar.New(nil)
@@ -1439,6 +1476,7 @@ func TestUserOIDC(t *testing.T) {
14391476

14401477
claims:= jwt.MapClaims{
14411478
"email":userData.Email,
1479+
"sub":uuid.NewString(),
14421480
}
14431481
user.HTTPClient.Jar,err=cookiejar.New(nil)
14441482
require.NoError(t,err)
@@ -1509,6 +1547,7 @@ func TestUserOIDC(t *testing.T) {
15091547
numLogs:=len(auditor.AuditLogs())
15101548
claims:= jwt.MapClaims{
15111549
"email":"jon@coder.com",
1550+
"sub":uuid.NewString(),
15121551
}
15131552

15141553
userClient,_:=fake.Login(t,client,claims)
@@ -1629,6 +1668,7 @@ func TestUserOIDC(t *testing.T) {
16291668
claims:= jwt.MapClaims{
16301669
"email":"user@example.com",
16311670
"email_verified":true,
1671+
"sub":uuid.NewString(),
16321672
}
16331673

16341674
// Perform the login
@@ -1794,6 +1834,7 @@ func TestOIDCSkipIssuer(t *testing.T) {
17941834
userClient,_:=fake.Login(t,owner, jwt.MapClaims{
17951835
"iss":secondaryURLString,
17961836
"email":"alice@coder.com",
1837+
"sub":uuid.NewString(),
17971838
})
17981839
found,err:=userClient.User(ctx,"me")
17991840
require.NoError(t,err)

‎coderd/users_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -831,6 +831,7 @@ func TestPostUsers(t *testing.T) {
831831
// Try to log in with OIDC.
832832
userClient,_:=fake.Login(t,client, jwt.MapClaims{
833833
"email":email,
834+
"sub":uuid.NewString(),
834835
})
835836

836837
found,err:=userClient.User(ctx,"me")

‎enterprise/coderd/scim_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"testing"
1111

1212
"github.com/golang-jwt/jwt/v4"
13+
"github.com/google/uuid"
1314
"github.com/imulab/go-scim/pkg/v2/handlerutil"
1415
"github.com/imulab/go-scim/pkg/v2/spec"
1516
"github.com/stretchr/testify/assert"
@@ -568,6 +569,7 @@ func TestScim(t *testing.T) {
568569
//nolint:bodyclose
569570
scimUserClient,_:=fake.Login(t,client, jwt.MapClaims{
570571
"email":sUser.Emails[0].Value,
572+
"sub":uuid.NewString(),
571573
})
572574
scimUser,err=scimUserClient.User(ctx,codersdk.Me)
573575
require.NoError(t,err)
@@ -836,6 +838,7 @@ func TestScim(t *testing.T) {
836838
//nolint:bodyclose
837839
scimUserClient,_:=fake.Login(t,client, jwt.MapClaims{
838840
"email":sUser.Emails[0].Value,
841+
"sub":uuid.NewString(),
839842
})
840843
scimUser,err=scimUserClient.User(ctx,codersdk.Me)
841844
require.NoError(t,err)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp