mirror of https://github.com/coder/coder.git
fix: use `UserInfo` endpoint with OIDC (#5735)
This resolves a user issue surfaced in Discord: https://discord.com/channels/747933592273027093/1064566338875576361/1064566338875576361 Both methods of obtaining claims need to be used according to the OIDC specification.
This commit is contained in:
parent
592ce3b118
commit
bbc1a9a1d8
|
@ -543,6 +543,7 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
|
|||
Endpoint: oidcProvider.Endpoint(),
|
||||
Scopes: cfg.OIDC.Scopes.Value,
|
||||
},
|
||||
Provider: oidcProvider,
|
||||
Verifier: oidcProvider.Verifier(&oidc.Config{
|
||||
ClientID: cfg.OIDC.ClientID.Value,
|
||||
}),
|
||||
|
|
|
@ -887,7 +887,23 @@ func (o *OIDCConfig) EncodeClaims(t *testing.T, claims jwt.MapClaims) string {
|
|||
return base64.StdEncoding.EncodeToString([]byte(signed))
|
||||
}
|
||||
|
||||
func (o *OIDCConfig) OIDCConfig() *coderd.OIDCConfig {
|
||||
func (o *OIDCConfig) OIDCConfig(t *testing.T, userInfoClaims jwt.MapClaims) *coderd.OIDCConfig {
|
||||
// By default, the provider can be empty.
|
||||
// This means it won't support any endpoints!
|
||||
provider := &oidc.Provider{}
|
||||
if userInfoClaims != nil {
|
||||
resp, err := json.Marshal(userInfoClaims)
|
||||
require.NoError(t, err)
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
_, _ = w.Write(resp)
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
cfg := &oidc.ProviderConfig{
|
||||
UserInfoURL: srv.URL,
|
||||
}
|
||||
provider = cfg.NewProvider(context.Background())
|
||||
}
|
||||
return &coderd.OIDCConfig{
|
||||
OAuth2Config: o,
|
||||
Verifier: oidc.NewVerifier(o.issuer, &oidc.StaticKeySet{
|
||||
|
@ -895,6 +911,7 @@ func (o *OIDCConfig) OIDCConfig() *coderd.OIDCConfig {
|
|||
}, &oidc.Config{
|
||||
SkipClientIDCheck: true,
|
||||
}),
|
||||
Provider: provider,
|
||||
UsernameField: "preferred_username",
|
||||
}
|
||||
}
|
||||
|
|
|
@ -204,6 +204,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
|
|||
type OIDCConfig struct {
|
||||
httpmw.OAuth2Config
|
||||
|
||||
Provider *oidc.Provider
|
||||
Verifier *oidc.IDTokenVerifier
|
||||
// EmailDomains are the domains to enforce when a user authenticates.
|
||||
EmailDomain []string
|
||||
|
@ -258,6 +259,35 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
|
|||
})
|
||||
return
|
||||
}
|
||||
|
||||
// Not all claims are necessarily embedded in the `id_token`.
|
||||
// In GitLab, the username is left empty and must be fetched in UserInfo.
|
||||
//
|
||||
// The OIDC specification says claims can be in either place, so we fetch
|
||||
// user info and merge the two claim sets to be sure we have all of
|
||||
// the correct data.
|
||||
userInfo, err := api.OIDCConfig.Provider.UserInfo(ctx, oauth2.StaticTokenSource(state.Token))
|
||||
if err == nil {
|
||||
userInfoClaims := map[string]interface{}{}
|
||||
err = userInfo.Claims(&userInfoClaims)
|
||||
if err != nil {
|
||||
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
|
||||
Message: "Failed to unmarshal user info claims.",
|
||||
Detail: err.Error(),
|
||||
})
|
||||
return
|
||||
}
|
||||
for k, v := range userInfoClaims {
|
||||
claims[k] = v
|
||||
}
|
||||
} else if !strings.Contains(err.Error(), "user info endpoint is not supported by this provider") {
|
||||
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
|
||||
Message: "Failed to obtain user information claims.",
|
||||
Detail: "The OIDC provider returned no claims as part of the `id_token`. The attempt to fetch claims via the UserInfo endpoint failed: " + err.Error(),
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
usernameRaw, ok := claims[api.OIDCConfig.UsernameField]
|
||||
var username string
|
||||
if ok {
|
||||
|
|
|
@ -480,7 +480,8 @@ func TestUserOIDC(t *testing.T) {
|
|||
|
||||
for _, tc := range []struct {
|
||||
Name string
|
||||
Claims jwt.MapClaims
|
||||
IDTokenClaims jwt.MapClaims
|
||||
UserInfoClaims jwt.MapClaims
|
||||
AllowSignups bool
|
||||
EmailDomain []string
|
||||
Username string
|
||||
|
@ -489,7 +490,7 @@ func TestUserOIDC(t *testing.T) {
|
|||
IgnoreEmailVerified bool
|
||||
}{{
|
||||
Name: "EmailOnly",
|
||||
Claims: jwt.MapClaims{
|
||||
IDTokenClaims: jwt.MapClaims{
|
||||
"email": "kyle@kwc.io",
|
||||
},
|
||||
AllowSignups: true,
|
||||
|
@ -497,7 +498,7 @@ func TestUserOIDC(t *testing.T) {
|
|||
Username: "kyle",
|
||||
}, {
|
||||
Name: "EmailNotVerified",
|
||||
Claims: jwt.MapClaims{
|
||||
IDTokenClaims: jwt.MapClaims{
|
||||
"email": "kyle@kwc.io",
|
||||
"email_verified": false,
|
||||
},
|
||||
|
@ -505,7 +506,7 @@ func TestUserOIDC(t *testing.T) {
|
|||
StatusCode: http.StatusForbidden,
|
||||
}, {
|
||||
Name: "EmailNotAString",
|
||||
Claims: jwt.MapClaims{
|
||||
IDTokenClaims: jwt.MapClaims{
|
||||
"email": 3.14159,
|
||||
"email_verified": false,
|
||||
},
|
||||
|
@ -513,7 +514,7 @@ func TestUserOIDC(t *testing.T) {
|
|||
StatusCode: http.StatusBadRequest,
|
||||
}, {
|
||||
Name: "EmailNotVerifiedIgnored",
|
||||
Claims: jwt.MapClaims{
|
||||
IDTokenClaims: jwt.MapClaims{
|
||||
"email": "kyle@kwc.io",
|
||||
"email_verified": false,
|
||||
},
|
||||
|
@ -523,7 +524,7 @@ func TestUserOIDC(t *testing.T) {
|
|||
IgnoreEmailVerified: true,
|
||||
}, {
|
||||
Name: "NotInRequiredEmailDomain",
|
||||
Claims: jwt.MapClaims{
|
||||
IDTokenClaims: jwt.MapClaims{
|
||||
"email": "kyle@kwc.io",
|
||||
"email_verified": true,
|
||||
},
|
||||
|
@ -534,7 +535,7 @@ func TestUserOIDC(t *testing.T) {
|
|||
StatusCode: http.StatusForbidden,
|
||||
}, {
|
||||
Name: "EmailDomainCaseInsensitive",
|
||||
Claims: jwt.MapClaims{
|
||||
IDTokenClaims: jwt.MapClaims{
|
||||
"email": "kyle@KWC.io",
|
||||
"email_verified": true,
|
||||
},
|
||||
|
@ -544,20 +545,20 @@ func TestUserOIDC(t *testing.T) {
|
|||
},
|
||||
StatusCode: http.StatusTemporaryRedirect,
|
||||
}, {
|
||||
Name: "EmptyClaims",
|
||||
Claims: jwt.MapClaims{},
|
||||
AllowSignups: true,
|
||||
StatusCode: http.StatusBadRequest,
|
||||
Name: "EmptyClaims",
|
||||
IDTokenClaims: jwt.MapClaims{},
|
||||
AllowSignups: true,
|
||||
StatusCode: http.StatusBadRequest,
|
||||
}, {
|
||||
Name: "NoSignups",
|
||||
Claims: jwt.MapClaims{
|
||||
IDTokenClaims: jwt.MapClaims{
|
||||
"email": "kyle@kwc.io",
|
||||
"email_verified": true,
|
||||
},
|
||||
StatusCode: http.StatusForbidden,
|
||||
}, {
|
||||
Name: "UsernameFromEmail",
|
||||
Claims: jwt.MapClaims{
|
||||
IDTokenClaims: jwt.MapClaims{
|
||||
"email": "kyle@kwc.io",
|
||||
"email_verified": true,
|
||||
},
|
||||
|
@ -566,7 +567,7 @@ func TestUserOIDC(t *testing.T) {
|
|||
StatusCode: http.StatusTemporaryRedirect,
|
||||
}, {
|
||||
Name: "UsernameFromClaims",
|
||||
Claims: jwt.MapClaims{
|
||||
IDTokenClaims: jwt.MapClaims{
|
||||
"email": "kyle@kwc.io",
|
||||
"email_verified": true,
|
||||
"preferred_username": "hotdog",
|
||||
|
@ -578,7 +579,7 @@ func TestUserOIDC(t *testing.T) {
|
|||
// Services like Okta return the email as the username:
|
||||
// https://developer.okta.com/docs/reference/api/oidc/#base-claims-always-present
|
||||
Name: "UsernameAsEmail",
|
||||
Claims: jwt.MapClaims{
|
||||
IDTokenClaims: jwt.MapClaims{
|
||||
"email": "kyle@kwc.io",
|
||||
"email_verified": true,
|
||||
"preferred_username": "kyle@kwc.io",
|
||||
|
@ -589,7 +590,7 @@ func TestUserOIDC(t *testing.T) {
|
|||
}, {
|
||||
// See: https://github.com/coder/coder/issues/4472
|
||||
Name: "UsernameIsEmail",
|
||||
Claims: jwt.MapClaims{
|
||||
IDTokenClaims: jwt.MapClaims{
|
||||
"preferred_username": "kyle@kwc.io",
|
||||
},
|
||||
Username: "kyle",
|
||||
|
@ -597,23 +598,37 @@ func TestUserOIDC(t *testing.T) {
|
|||
StatusCode: http.StatusTemporaryRedirect,
|
||||
}, {
|
||||
Name: "WithPicture",
|
||||
Claims: jwt.MapClaims{
|
||||
"email": "kyle@kwc.io",
|
||||
"email_verified": true,
|
||||
"username": "kyle",
|
||||
"picture": "/example.png",
|
||||
IDTokenClaims: jwt.MapClaims{
|
||||
"email": "kyle@kwc.io",
|
||||
"email_verified": true,
|
||||
"preferred_username": "kyle",
|
||||
"picture": "/example.png",
|
||||
},
|
||||
Username: "kyle",
|
||||
AllowSignups: true,
|
||||
AvatarURL: "/example.png",
|
||||
StatusCode: http.StatusTemporaryRedirect,
|
||||
}, {
|
||||
Name: "WithUserInfoClaims",
|
||||
IDTokenClaims: jwt.MapClaims{
|
||||
"email": "kyle@kwc.io",
|
||||
"email_verified": true,
|
||||
},
|
||||
UserInfoClaims: jwt.MapClaims{
|
||||
"preferred_username": "potato",
|
||||
"picture": "/example.png",
|
||||
},
|
||||
Username: "potato",
|
||||
AllowSignups: true,
|
||||
AvatarURL: "/example.png",
|
||||
StatusCode: http.StatusTemporaryRedirect,
|
||||
}} {
|
||||
tc := tc
|
||||
t.Run(tc.Name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
conf := coderdtest.NewOIDCConfig(t, "")
|
||||
|
||||
config := conf.OIDCConfig()
|
||||
config := conf.OIDCConfig(t, tc.UserInfoClaims)
|
||||
config.AllowSignups = tc.AllowSignups
|
||||
config.EmailDomain = tc.EmailDomain
|
||||
config.IgnoreEmailVerified = tc.IgnoreEmailVerified
|
||||
|
@ -621,7 +636,7 @@ func TestUserOIDC(t *testing.T) {
|
|||
client := coderdtest.New(t, &coderdtest.Options{
|
||||
OIDCConfig: config,
|
||||
})
|
||||
resp := oidcCallback(t, client, conf.EncodeClaims(t, tc.Claims))
|
||||
resp := oidcCallback(t, client, conf.EncodeClaims(t, tc.IDTokenClaims))
|
||||
assert.Equal(t, tc.StatusCode, resp.StatusCode)
|
||||
|
||||
ctx, _ := testutil.Context(t)
|
||||
|
@ -647,7 +662,7 @@ func TestUserOIDC(t *testing.T) {
|
|||
|
||||
conf := coderdtest.NewOIDCConfig(t, "")
|
||||
|
||||
config := conf.OIDCConfig()
|
||||
config := conf.OIDCConfig(t, nil)
|
||||
config.AllowSignups = true
|
||||
|
||||
client := coderdtest.New(t, &coderdtest.Options{
|
||||
|
@ -705,6 +720,7 @@ func TestUserOIDC(t *testing.T) {
|
|||
verifier := oidc.NewVerifier("", &oidc.StaticKeySet{
|
||||
PublicKeys: []crypto.PublicKey{},
|
||||
}, &oidc.Config{})
|
||||
provider := &oidc.Provider{}
|
||||
|
||||
client := coderdtest.New(t, &coderdtest.Options{
|
||||
OIDCConfig: &coderd.OIDCConfig{
|
||||
|
@ -715,6 +731,7 @@ func TestUserOIDC(t *testing.T) {
|
|||
"id_token": "invalid",
|
||||
}),
|
||||
},
|
||||
Provider: provider,
|
||||
Verifier: verifier,
|
||||
},
|
||||
})
|
||||
|
|
Loading…
Reference in New Issue