Skip to content

fix: use UserInfo endpoint with OIDC #5735

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

Merged
merged 1 commit into from
Jan 16, 2023
Merged

fix: use UserInfo endpoint with OIDC #5735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 1 addition & 0 deletions cli/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}),
Expand Down
19 changes: 18 additions & 1 deletion coderd/coderdtest/coderdtest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -887,14 +887,31 @@ 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{
PublicKeys: []crypto.PublicKey{o.key.Public()},
}, &oidc.Config{
SkipClientIDCheck: true,
}),
Provider: provider,
UsernameField: "preferred_username",
}
}
Expand Down
30 changes: 30 additions & 0 deletions coderd/userauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
63 changes: 40 additions & 23 deletions coderd/userauth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -489,31 +490,31 @@ func TestUserOIDC(t *testing.T) {
IgnoreEmailVerified bool
}{{
Name: "EmailOnly",
Claims: jwt.MapClaims{
IDTokenClaims: jwt.MapClaims{
"email": "kyle@kwc.io",
},
AllowSignups: true,
StatusCode: http.StatusTemporaryRedirect,
Username: "kyle",
}, {
Name: "EmailNotVerified",
Claims: jwt.MapClaims{
IDTokenClaims: jwt.MapClaims{
"email": "kyle@kwc.io",
"email_verified": false,
},
AllowSignups: true,
StatusCode: http.StatusForbidden,
}, {
Name: "EmailNotAString",
Claims: jwt.MapClaims{
IDTokenClaims: jwt.MapClaims{
"email": 3.14159,
"email_verified": false,
},
AllowSignups: true,
StatusCode: http.StatusBadRequest,
}, {
Name: "EmailNotVerifiedIgnored",
Claims: jwt.MapClaims{
IDTokenClaims: jwt.MapClaims{
"email": "kyle@kwc.io",
"email_verified": false,
},
Expand All @@ -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,
},
Expand All @@ -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,
},
Expand All @@ -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,
},
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -589,21 +590,35 @@ 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",
AllowSignups: true,
StatusCode: http.StatusTemporaryRedirect,
}, {
Name: "WithPicture",
Claims: jwt.MapClaims{
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,
"username": "kyle",
"picture": "/example.png",
},
Username: "kyle",
UserInfoClaims: jwt.MapClaims{
"preferred_username": "potato",
"picture": "/example.png",
},
Username: "potato",
AllowSignups: true,
AvatarURL: "/example.png",
StatusCode: http.StatusTemporaryRedirect,
Expand All @@ -613,15 +628,15 @@ func TestUserOIDC(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

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)
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand All @@ -715,6 +731,7 @@ func TestUserOIDC(t *testing.T) {
"id_token": "invalid",
}),
},
Provider: provider,
Verifier: verifier,
},
})
Expand Down