fix: Optionally consume `email_verified` if it's provided (#3957)

This reduces our OIDC requirement claims to only `email`. If `email_verified`
is provided and is `false`, we will block authentication.

Fixes #3954.
This commit is contained in:
Kyle Carberry 2022-09-08 09:06:00 -05:00 committed by GitHub
parent bb4a681833
commit e1afec6db4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 49 additions and 20 deletions

View File

@ -204,12 +204,10 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
return
}
var claims struct {
Email string `json:"email"`
Verified bool `json:"email_verified"`
Username string `json:"preferred_username"`
Picture string `json:"picture"`
}
// "email_verified" is an optional claim that changes the behavior
// of our OIDC handler, so each property must be pulled manually out
// of the claim mapping.
claims := map[string]interface{}{}
err = idToken.Claims(&claims)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
@ -218,47 +216,69 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
})
return
}
if claims.Email == "" {
emailRaw, ok := claims["email"]
if !ok {
httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{
Message: "No email found in OIDC payload!",
})
return
}
if !claims.Verified {
httpapi.Write(rw, http.StatusForbidden, codersdk.Response{
Message: fmt.Sprintf("Verify the %q email address on your OIDC provider to authenticate!", claims.Email),
email, ok := emailRaw.(string)
if !ok {
httpapi.Write(rw, http.StatusBadRequest, codersdk.Response{
Message: fmt.Sprintf("Email in OIDC payload isn't a string. Got: %t", emailRaw),
})
return
}
verifiedRaw, ok := claims["email_verified"]
if ok {
verified, ok := verifiedRaw.(bool)
if ok && !verified {
httpapi.Write(rw, http.StatusForbidden, codersdk.Response{
Message: fmt.Sprintf("Verify the %q email address on your OIDC provider to authenticate!", email),
})
return
}
}
usernameRaw, ok := claims["preferred_username"]
var username string
if ok {
username, _ = usernameRaw.(string)
}
// The username is a required property in Coder. We make a best-effort
// attempt at using what the claims provide, but if that fails we will
// generate a random username.
if !httpapi.UsernameValid(claims.Username) {
if !httpapi.UsernameValid(username) {
// If no username is provided, we can default to use the email address.
// This will be converted in the from function below, so it's safe
// to keep the domain.
if claims.Username == "" {
claims.Username = claims.Email
if username == "" {
username = email
}
claims.Username = httpapi.UsernameFrom(claims.Username)
username = httpapi.UsernameFrom(username)
}
if api.OIDCConfig.EmailDomain != "" {
if !strings.HasSuffix(claims.Email, api.OIDCConfig.EmailDomain) {
if !strings.HasSuffix(email, api.OIDCConfig.EmailDomain) {
httpapi.Write(rw, http.StatusForbidden, codersdk.Response{
Message: fmt.Sprintf("Your email %q is not a part of the %q domain!", claims.Email, api.OIDCConfig.EmailDomain),
Message: fmt.Sprintf("Your email %q is not a part of the %q domain!", email, api.OIDCConfig.EmailDomain),
})
return
}
}
var picture string
pictureRaw, ok := claims["picture"]
if ok {
picture, _ = pictureRaw.(string)
}
cookie, err := api.oauthLogin(r, oauthLoginParams{
State: state,
LinkedID: oidcLinkedID(idToken),
LoginType: database.LoginTypeOIDC,
AllowSignups: api.OIDCConfig.AllowSignups,
Email: claims.Email,
Username: claims.Username,
AvatarURL: claims.Picture,
Email: email,
Username: username,
AvatarURL: picture,
})
var httpErr httpError
if xerrors.As(err, &httpErr) {

View File

@ -302,11 +302,20 @@ func TestUserOIDC(t *testing.T) {
AvatarURL string
StatusCode int
}{{
Name: "EmailNotVerified",
Name: "EmailOnly",
Claims: jwt.MapClaims{
"email": "kyle@kwc.io",
},
AllowSignups: true,
StatusCode: http.StatusTemporaryRedirect,
Username: "kyle",
}, {
Name: "EmailNotVerified",
Claims: jwt.MapClaims{
"email": "kyle@kwc.io",
"email_verified": false,
},
AllowSignups: true,
StatusCode: http.StatusForbidden,
}, {
Name: "NotInRequiredEmailDomain",