From e7ff51fa82e80311485f4575d12c9d846ca5b7c3 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 6 Jul 2023 14:06:00 -0400 Subject: [PATCH 1/2] chore: add better error in wrong login type --- coderd/userauth.go | 64 +++++++++++++++++++++++++++++++--------------- 1 file changed, 43 insertions(+), 21 deletions(-) diff --git a/coderd/userauth.go b/coderd/userauth.go index 6ecf19d1f6053..cf6a47b2a169e 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -32,6 +32,7 @@ import ( "github.com/coder/coder/coderd/userpassword" "github.com/coder/coder/codersdk" "github.com/coder/coder/cryptorand" + "github.com/coder/coder/site" ) const ( @@ -625,10 +626,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) { defer params.CommitAuditLogs() var httpErr httpError if xerrors.As(err, &httpErr) { - httpapi.Write(ctx, rw, httpErr.code, codersdk.Response{ - Message: httpErr.msg, - Detail: httpErr.detail, - }) + httpErr.Write(rw, r) return } if err != nil { @@ -969,10 +967,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { defer params.CommitAuditLogs() var httpErr httpError if xerrors.As(err, &httpErr) { - httpapi.Write(ctx, rw, httpErr.code, codersdk.Response{ - Message: httpErr.msg, - Detail: httpErr.detail, - }) + httpErr.Write(rw, r) return } if err != nil { @@ -1076,9 +1071,28 @@ func (p *oauthLoginParams) CommitAuditLogs() { } type httpError struct { - code int - msg string - detail string + code int + msg string + detail string + renderStaticPage bool +} + +func (e httpError) Write(rw http.ResponseWriter, r *http.Request) { + if e.renderStaticPage { + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: e.code, + HideStatus: true, + Title: e.msg, + Description: e.detail, + RetryEnabled: false, + DashboardURL: "/login", + }) + return + } + httpapi.Write(r.Context(), rw, e.code, codersdk.Response{ + Message: e.msg, + Detail: e.detail, + }) } func (e httpError) Error() string { @@ -1126,12 +1140,16 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C } if user.ID != uuid.Nil && user.LoginType != params.LoginType { + addedMsg := "" + if user.LoginType == database.LoginTypePassword { + addedMsg = " You can convert your account to use this login type by visiting your account settings." + } return 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, - ), + code: http.StatusForbidden, + renderStaticPage: true, + msg: "Incorrect login type", + detail: fmt.Sprintf("Attempting to use login type %q, but the user has the login type %q.%s", + params.LoginType, user.LoginType, addedMsg), } } @@ -1355,12 +1373,16 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data // If we do not allow converting to oauth, return an error. if !api.Experiments.Enabled(codersdk.ExperimentConvertToOIDC) { + addedMsg := "" + if user.LoginType == database.LoginTypePassword { + addedMsg = " You can convert your account to use this login type by visiting your account settings." + } 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, - ), + code: http.StatusForbidden, + renderStaticPage: true, + msg: "Incorrect login type", + detail: fmt.Sprintf("Attempting to use login type %q, but the user has the login type %q.%s", + params.LoginType, user.LoginType, addedMsg), } } From 019cb0059bc2ceb3c12251b606070eee5eca77b0 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 7 Jul 2023 10:55:19 -0400 Subject: [PATCH 2/2] dedup code --- coderd/userauth.go | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/coderd/userauth.go b/coderd/userauth.go index cf6a47b2a169e..be083757f2d41 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -1140,17 +1140,7 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C } if user.ID != uuid.Nil && user.LoginType != params.LoginType { - addedMsg := "" - if user.LoginType == database.LoginTypePassword { - addedMsg = " You can convert your account to use this login type by visiting your account settings." - } - return httpError{ - code: http.StatusForbidden, - renderStaticPage: true, - msg: "Incorrect login type", - detail: fmt.Sprintf("Attempting to use login type %q, but the user has the login type %q.%s", - params.LoginType, user.LoginType, addedMsg), - } + return wrongLoginTypeHTTPError(user.LoginType, params.LoginType) } // This can happen if a user is a built-in user but is signing in @@ -1373,17 +1363,7 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data // If we do not allow converting to oauth, return an error. if !api.Experiments.Enabled(codersdk.ExperimentConvertToOIDC) { - addedMsg := "" - if user.LoginType == database.LoginTypePassword { - addedMsg = " You can convert your account to use this login type by visiting your account settings." - } - return database.User{}, httpError{ - code: http.StatusForbidden, - renderStaticPage: true, - msg: "Incorrect login type", - detail: fmt.Sprintf("Attempting to use login type %q, but the user has the login type %q.%s", - params.LoginType, user.LoginType, addedMsg), - } + return database.User{}, wrongLoginTypeHTTPError(user.LoginType, params.LoginType) } if claims.RegisteredClaims.Issuer != api.DeploymentID { @@ -1509,3 +1489,17 @@ func clearOAuthConvertCookie() *http.Cookie { MaxAge: -1, } } + +func wrongLoginTypeHTTPError(user database.LoginType, params database.LoginType) httpError { + addedMsg := "" + if user == database.LoginTypePassword { + addedMsg = " You can convert your account to use this login type by visiting your account settings." + } + return httpError{ + code: http.StatusForbidden, + renderStaticPage: true, + msg: "Incorrect login type", + detail: fmt.Sprintf("Attempting to use login type %q, but the user has the login type %q.%s", + params, user, addedMsg), + } +}