Skip to content

chore: improve error message for incorrect login type #8349

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.

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 2 commits into from
Jul 7, 2023
Merged
Changes from 1 commit
Commits
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
Next Next commit
chore: add better error in wrong login type
  • Loading branch information
Emyrk committed Jul 6, 2023
commit e7ff51fa82e80311485f4575d12c9d846ca5b7c3
64 changes: 43 additions & 21 deletions coderd/userauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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),
}
}

Expand Down Expand Up @@ -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),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to dedupe this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably. I'll do it

}
}

Expand Down