feat: add ability for users to convert their password login type to oauth/github login (#8105)

* Currently toggled by experiment flag

---------

Co-authored-by: Bruno Quaresma <bruno@coder.com>
This commit is contained in:
Steven Masley
2023-06-30 08:38:48 -04:00
committed by GitHub
parent 357f3b38f7
commit b5f26d9bdf
50 changed files with 2043 additions and 261 deletions

View File

@ -10,8 +10,11 @@ import (
"sort"
"strconv"
"strings"
"sync"
"time"
"github.com/coreos/go-oidc/v3/oidc"
"github.com/golang-jwt/jwt/v4"
"github.com/google/go-github/v43/github"
"github.com/google/uuid"
"github.com/moby/moby/pkg/namesgenerator"
@ -28,12 +31,174 @@ import (
"github.com/coder/coder/coderd/rbac"
"github.com/coder/coder/coderd/userpassword"
"github.com/coder/coder/codersdk"
"github.com/coder/coder/cryptorand"
)
const (
userAuthLoggerName = "userauth"
userAuthLoggerName = "userauth"
OAuthConvertCookieValue = "coder_oauth_convert_jwt"
mergeStateStringPrefix = "convert-"
)
type OAuthConvertStateClaims struct {
jwt.RegisteredClaims
UserID uuid.UUID `json:"user_id"`
State string `json:"state"`
FromLoginType codersdk.LoginType `json:"from_login_type"`
ToLoginType codersdk.LoginType `json:"to_login_type"`
}
// postConvertLoginType replies with an oauth state token capable of converting
// the user to an oauth user.
//
// @Summary Convert user from password to oauth authentication
// @ID convert-user-from-password-to-oauth-authentication
// @Security CoderSessionToken
// @Accept json
// @Produce json
// @Tags Authorization
// @Param request body codersdk.ConvertLoginRequest true "Convert request"
// @Param user path string true "User ID, name, or me"
// @Success 201 {object} codersdk.OAuthConversionResponse
// @Router /users/{user}/convert-login [post]
func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
if !api.Experiments.Enabled(codersdk.ExperimentConvertToOIDC) {
httpapi.Write(r.Context(), rw, http.StatusForbidden, codersdk.Response{
Message: "Oauth conversion is not allowed, contact an administrator to turn on this feature.",
})
return
}
var (
user = httpmw.UserParam(r)
ctx = r.Context()
auditor = api.Auditor.Load()
aReq, commitAudit = audit.InitRequest[database.AuditOAuthConvertState](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
Action: database.AuditActionCreate,
})
)
aReq.Old = database.AuditOAuthConvertState{}
defer commitAudit()
var req codersdk.ConvertLoginRequest
if !httpapi.Read(ctx, rw, r, &req) {
return
}
switch req.ToType {
case codersdk.LoginTypeGithub, codersdk.LoginTypeOIDC:
// Allowed!
case codersdk.LoginTypeNone, codersdk.LoginTypePassword, codersdk.LoginTypeToken:
// These login types are not allowed to be converted to at this time.
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: fmt.Sprintf("Cannot convert to login type %q.", req.ToType),
})
return
default:
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: fmt.Sprintf("Unknown login type %q.", req.ToType),
})
return
}
// This handles the email/pass checking.
user, _, ok := api.loginRequest(ctx, rw, codersdk.LoginWithPasswordRequest{
Email: user.Email,
Password: req.Password,
})
if !ok {
return
}
// Only support converting from password auth.
if user.LoginType != database.LoginTypePassword {
// This is checked in loginRequest, but checked again here in case that shared
// function changes its checks. Just some defensive programming.
// This login type is **required** to be password based to prevent
// users from converting other login types to OIDC.
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "User account must have password based authentication.",
})
return
}
stateString, err := cryptorand.String(32)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error generating state string.",
Detail: err.Error(),
})
return
}
// The prefix is used to identify this state string as a conversion state
// without needing to hit the database. The random string is the CSRF protection.
stateString = fmt.Sprintf("%s%s", mergeStateStringPrefix, stateString)
// This JWT is the signed payload to authorize the convert to oauth request.
// When the user does the oauth flow, this jwt will be sent back to coderd.
// The included information in this payload links it to a state string, so
// this request is tied 1:1 with an oauth state.
// This JWT also includes information to tie it 1:1 with a coder deployment
// and user account. This is mainly to inform the user if they are accidentally
// switching between coder deployments if the OIDC is misconfigured.
// Eg: Developers with more than 1 deployment.
now := time.Now()
claims := &OAuthConvertStateClaims{
RegisteredClaims: jwt.RegisteredClaims{
Issuer: api.DeploymentID,
Subject: stateString,
Audience: []string{user.ID.String()},
ExpiresAt: jwt.NewNumericDate(now.Add(time.Minute * 5)),
NotBefore: jwt.NewNumericDate(now.Add(time.Second * -1)),
IssuedAt: jwt.NewNumericDate(now),
ID: uuid.NewString(),
},
UserID: user.ID,
State: stateString,
FromLoginType: codersdk.LoginType(user.LoginType),
ToLoginType: req.ToType,
}
token := jwt.NewWithClaims(jwt.SigningMethodHS512, claims)
// Key must be a byte slice, not an array. So make sure to include the [:]
tokenString, err := token.SignedString(api.OAuthSigningKey[:])
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error signing state jwt.",
Detail: err.Error(),
})
return
}
aReq.New = database.AuditOAuthConvertState{
CreatedAt: claims.IssuedAt.Time,
ExpiresAt: claims.ExpiresAt.Time,
FromLoginType: database.LoginType(claims.FromLoginType),
ToLoginType: database.LoginType(claims.ToLoginType),
UserID: claims.UserID,
}
http.SetCookie(rw, &http.Cookie{
Name: OAuthConvertCookieValue,
Path: "/",
Value: tokenString,
Expires: claims.ExpiresAt.Time,
Secure: api.SecureAuthCookie,
HttpOnly: true,
SameSite: http.SameSiteStrictMode,
})
httpapi.Write(ctx, rw, http.StatusCreated, codersdk.OAuthConversionResponse{
StateString: stateString,
ExpiresAt: claims.ExpiresAt.Time,
ToType: claims.ToLoginType,
UserID: claims.UserID,
})
}
// Authenticates the user with an email and password.
//
// @Summary Log in user
@ -48,6 +213,7 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) {
var (
ctx = r.Context()
auditor = api.Auditor.Load()
logger = api.Logger.Named(userAuthLoggerName)
aReq, commitAudit = audit.InitRequest[database.APIKey](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
@ -63,71 +229,12 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) {
return
}
logger := api.Logger.Named(userAuthLoggerName)
//nolint:gocritic // In order to login, we need to get the user first!
user, err := api.Database.GetUserByEmailOrUsername(dbauthz.AsSystemRestricted(ctx), database.GetUserByEmailOrUsernameParams{
Email: loginWithPassword.Email,
})
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
logger.Error(ctx, "unable to fetch user by email", slog.Error(err))
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error.",
})
return
}
user, roles, ok := api.loginRequest(ctx, rw, loginWithPassword)
// 'user.ID' will be empty, or will be an actual value. Either is correct
// here.
aReq.UserID = user.ID
// If the user doesn't exist, it will be a default struct.
equal, err := userpassword.Compare(string(user.HashedPassword), loginWithPassword.Password)
if err != nil {
logger.Error(ctx, "unable to compare passwords", slog.Error(err))
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error.",
})
return
}
if !equal {
// This message is the same as above to remove ease in detecting whether
// users are registered or not. Attackers still could with a timing attack.
httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{
Message: "Incorrect email or password.",
})
return
}
// If password authentication is disabled and the user does not have the
// owner role, block the request.
if api.DeploymentValues.DisablePasswordAuth {
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
Message: "Password authentication is disabled.",
})
return
}
if user.LoginType != database.LoginTypePassword {
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
Message: fmt.Sprintf("Incorrect login type, attempting to use %q but user is of login type %q", database.LoginTypePassword, user.LoginType),
})
return
}
//nolint:gocritic // System needs to fetch user roles in order to login user.
roles, err := api.Database.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), user.ID)
if err != nil {
logger.Error(ctx, "unable to fetch authorization user roles", slog.Error(err))
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error.",
})
return
}
// If the user logged into a suspended account, reject the login request.
if roles.Status != database.UserStatusActive {
httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{
Message: "Your account is suspended. Contact an admin to reactivate your account.",
})
if !ok {
// user failed to login
return
}
@ -163,6 +270,83 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) {
})
}
// loginRequest will process a LoginWithPasswordRequest and return the user if
// the credentials are correct. If 'false' is returned, the authentication failed
// and the appropriate error will be written to the ResponseWriter.
//
// The user struct is always returned, even if authentication failed. This is
// to support knowing what user attempted to login.
func (api *API) loginRequest(ctx context.Context, rw http.ResponseWriter, req codersdk.LoginWithPasswordRequest) (database.User, database.GetAuthorizationUserRolesRow, bool) {
logger := api.Logger.Named(userAuthLoggerName)
//nolint:gocritic // In order to login, we need to get the user first!
user, err := api.Database.GetUserByEmailOrUsername(dbauthz.AsSystemRestricted(ctx), database.GetUserByEmailOrUsernameParams{
Email: req.Email,
})
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
logger.Error(ctx, "unable to fetch user by email", slog.Error(err))
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error.",
})
return user, database.GetAuthorizationUserRolesRow{}, false
}
// If the user doesn't exist, it will be a default struct.
equal, err := userpassword.Compare(string(user.HashedPassword), req.Password)
if err != nil {
logger.Error(ctx, "unable to compare passwords", slog.Error(err))
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error.",
})
return user, database.GetAuthorizationUserRolesRow{}, false
}
if !equal {
// This message is the same as above to remove ease in detecting whether
// users are registered or not. Attackers still could with a timing attack.
httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{
Message: "Incorrect email or password.",
})
return user, database.GetAuthorizationUserRolesRow{}, false
}
// If password authentication is disabled and the user does not have the
// owner role, block the request.
if api.DeploymentValues.DisablePasswordAuth {
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
Message: "Password authentication is disabled.",
})
return user, database.GetAuthorizationUserRolesRow{}, false
}
if user.LoginType != database.LoginTypePassword {
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
Message: fmt.Sprintf("Incorrect login type, attempting to use %q but user is of login type %q", database.LoginTypePassword, user.LoginType),
})
return user, database.GetAuthorizationUserRolesRow{}, false
}
//nolint:gocritic // System needs to fetch user roles in order to login user.
roles, err := api.Database.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), user.ID)
if err != nil {
logger.Error(ctx, "unable to fetch authorization user roles", slog.Error(err))
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error.",
})
return user, database.GetAuthorizationUserRolesRow{}, false
}
// If the user logged into a suspended account, reject the login request.
if roles.Status != database.UserStatusActive {
httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{
Message: "Your account is suspended. Contact an admin to reactivate your account.",
})
return user, database.GetAuthorizationUserRolesRow{}, false
}
return user, roles, true
}
// Clear the user's session cookie.
//
// @Summary Log out user
@ -270,6 +454,7 @@ func (api *API) userAuthMethods(rw http.ResponseWriter, r *http.Request) {
}
httpapi.Write(r.Context(), rw, http.StatusOK, codersdk.AuthMethods{
ConvertToOIDCEnabled: api.Experiments.Enabled(codersdk.ExperimentConvertToOIDC),
Password: codersdk.AuthMethod{
Enabled: !api.DeploymentValues.DisablePasswordAuth.Value(),
},
@ -423,7 +608,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
aReq.Action = database.AuditActionRegister
}
cookie, key, err := api.oauthLogin(r, oauthLoginParams{
params := (&oauthLoginParams{
User: user,
Link: link,
State: state,
@ -433,7 +618,11 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
Email: verifiedEmail.GetEmail(),
Username: ghUser.GetLogin(),
AvatarURL: ghUser.GetAvatarURL(),
}).SetInitAuditRequest(func(params *audit.RequestParams) (*audit.Request[database.User], func()) {
return audit.InitRequest[database.User](rw, params)
})
cookies, key, err := api.oauthLogin(r, params)
defer params.CommitAuditLogs()
var httpErr httpError
if xerrors.As(err, &httpErr) {
httpapi.Write(ctx, rw, httpErr.code, codersdk.Response{
@ -453,7 +642,9 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
aReq.New = key
aReq.UserID = key.UserID
http.SetCookie(rw, cookie)
for _, cookie := range cookies {
http.SetCookie(rw, cookie)
}
redirect := state.Redirect
if redirect == "" {
@ -673,7 +864,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
// Convert the []interface{} we get to a []string.
groupsInterface, ok := groupsRaw.([]interface{})
if ok {
logger.Debug(ctx, "groups returned in oidc claims",
api.Logger.Debug(ctx, "groups returned in oidc claims",
slog.F("len", len(groupsInterface)),
slog.F("groups", groupsInterface),
)
@ -694,7 +885,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
groups = append(groups, group)
}
} else {
logger.Debug(ctx, "groups field was an unknown type",
api.Logger.Debug(ctx, "groups field was an unknown type",
slog.F("type", fmt.Sprintf("%T", groupsRaw)),
)
}
@ -759,7 +950,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
aReq.Action = database.AuditActionRegister
}
cookie, key, err := api.oauthLogin(r, oauthLoginParams{
params := (&oauthLoginParams{
User: user,
Link: link,
State: state,
@ -771,7 +962,11 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
AvatarURL: picture,
UsingGroups: usingGroups,
Groups: groups,
}).SetInitAuditRequest(func(params *audit.RequestParams) (*audit.Request[database.User], func()) {
return audit.InitRequest[database.User](rw, params)
})
cookies, key, err := api.oauthLogin(r, params)
defer params.CommitAuditLogs()
var httpErr httpError
if xerrors.As(err, &httpErr) {
httpapi.Write(ctx, rw, httpErr.code, codersdk.Response{
@ -791,7 +986,9 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
aReq.New = key
aReq.UserID = key.UserID
http.SetCookie(rw, cookie)
for i := range cookies {
http.SetCookie(rw, cookies[i])
}
redirect := state.Redirect
if redirect == "" {
@ -853,6 +1050,29 @@ type oauthLoginParams struct {
// to the Groups provided.
UsingGroups bool
Groups []string
commitLock sync.Mutex
initAuditRequest func(params *audit.RequestParams) *audit.Request[database.User]
commits []func()
}
func (p *oauthLoginParams) SetInitAuditRequest(f func(params *audit.RequestParams) (*audit.Request[database.User], func())) *oauthLoginParams {
p.initAuditRequest = func(params *audit.RequestParams) *audit.Request[database.User] {
p.commitLock.Lock()
defer p.commitLock.Unlock()
req, commit := f(params)
p.commits = append(p.commits, commit)
return req
}
return p
}
func (p *oauthLoginParams) CommitAuditLogs() {
p.commitLock.Lock()
defer p.commitLock.Unlock()
for _, f := range p.commits {
f()
}
}
type httpError struct {
@ -869,10 +1089,11 @@ func (e httpError) Error() string {
return e.msg
}
func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cookie, database.APIKey, error) {
func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.Cookie, database.APIKey, error) {
var (
ctx = r.Context()
user database.User
ctx = r.Context()
user database.User
cookies []*http.Cookie
)
err := api.Database.InTx(func(tx database.Store) error {
@ -884,6 +1105,19 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
user = params.User
link = params.Link
// If you do a convert to OIDC and your email does not match, we need to
// catch this and not make a new account.
if isMergeStateString(params.State.StateString) {
// Always clear this cookie. If it succeeds, we no longer need it.
// If it fails, we no longer care about it.
cookies = append(cookies, clearOAuthConvertCookie())
user, err = api.convertUserToOauth(ctx, r, tx, params)
if err != nil {
return err
}
params.User = user
}
if user.ID == uuid.Nil && !params.AllowSignups {
return httpError{
code: http.StatusForbidden,
@ -1061,7 +1295,118 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
return nil, database.APIKey{}, xerrors.Errorf("create API key: %w", err)
}
return cookie, *key, nil
return append(cookies, cookie), *key, nil
}
// convertUserToOauth will convert a user from password base loginType to
// an oauth login type. If it fails, it will return a httpError
func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db database.Store, params *oauthLoginParams) (database.User, error) {
user := params.User
// Trying to convert to OIDC, but the email does not match.
// So do not make a new user, just block the request.
if user.ID == uuid.Nil {
return database.User{}, httpError{
code: http.StatusBadRequest,
msg: fmt.Sprintf("The oidc account with the email %q does not match the email of the account you are trying to convert. Contact your administrator to resolve this issue.", params.Email),
}
}
jwtCookie, err := r.Cookie(OAuthConvertCookieValue)
if err != nil {
return database.User{}, httpError{
code: http.StatusBadRequest,
msg: fmt.Sprintf("Convert to oauth cookie not found. Missing signed jwt to authorize this action. " +
"Please try again."),
}
}
var claims OAuthConvertStateClaims
token, err := jwt.ParseWithClaims(jwtCookie.Value, &claims, func(token *jwt.Token) (interface{}, error) {
return api.OAuthSigningKey[:], nil
})
if xerrors.Is(err, jwt.ErrSignatureInvalid) || !token.Valid {
// These errors are probably because the user is mixing 2 coder deployments.
return database.User{}, httpError{
code: http.StatusBadRequest,
msg: "Using an invalid jwt to authorize this action. Ensure there is only 1 coder deployment and try again.",
}
}
if err != nil {
return database.User{}, httpError{
code: http.StatusInternalServerError,
msg: fmt.Sprintf("Error parsing jwt: %v", err),
}
}
// At this point, this request could be an attempt to convert from
// password auth to oauth auth. Always log these attempts.
var (
auditor = *api.Auditor.Load()
oauthConvertAudit = params.initAuditRequest(&audit.RequestParams{
Audit: auditor,
Log: api.Logger,
Request: r,
Action: database.AuditActionWrite,
})
)
oauthConvertAudit.UserID = claims.UserID
oauthConvertAudit.Old = user
// If we do not allow converting to oauth, return an error.
if !api.Experiments.Enabled(codersdk.ExperimentConvertToOIDC) {
return database.User{}, httpError{
code: http.StatusForbidden,
msg: fmt.Sprintf("Incorrect login type, attempting to use %q but user is of login type %q",
params.LoginType,
user.LoginType,
),
}
}
if claims.RegisteredClaims.Issuer != api.DeploymentID {
return database.User{}, httpError{
code: http.StatusForbidden,
msg: "Request to convert login type failed. Issuer mismatch. Found a cookie from another coder deployment, please try again.",
}
}
if params.State.StateString != claims.State {
return database.User{}, httpError{
code: http.StatusForbidden,
msg: "Request to convert login type failed. State mismatch.",
}
}
// Make sure the merge state generated matches this OIDC login request.
// It needs to have the correct login type information for this
// user.
if user.ID != claims.UserID ||
codersdk.LoginType(user.LoginType) != claims.FromLoginType ||
codersdk.LoginType(params.LoginType) != claims.ToLoginType {
return database.User{}, httpError{
code: http.StatusForbidden,
msg: fmt.Sprintf("Request to convert login type from %s to %s failed", user.LoginType, params.LoginType),
}
}
// Convert the user and default to the normal login flow.
// If the login succeeds, this transaction will commit and the user
// will be converted.
// nolint:gocritic // system query to update user login type. The user already
// provided their password to authenticate this request.
user, err = db.UpdateUserLoginType(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLoginTypeParams{
NewLoginType: params.LoginType,
UserID: user.ID,
})
if err != nil {
return database.User{}, httpError{
code: http.StatusInternalServerError,
msg: "Failed to convert user to new login type",
}
}
oauthConvertAudit.New = user
return user, nil
}
// githubLinkedID returns the unique ID for a GitHub user.
@ -1130,3 +1475,15 @@ func findLinkedUser(ctx context.Context, db database.Store, linkedID string, ema
return user, link, nil
}
func isMergeStateString(state string) bool {
return strings.HasPrefix(state, mergeStateStringPrefix)
}
func clearOAuthConvertCookie() *http.Cookie {
return &http.Cookie{
Name: OAuthConvertCookieValue,
Path: "/",
MaxAge: -1,
}
}