Skip to content

Commit 1be6aed

Browse files
committed
fixup errors
1 parent 721ae79 commit 1be6aed

File tree

1 file changed

+63
-102
lines changed

1 file changed

+63
-102
lines changed

coderd/userauth.go

Lines changed: 63 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ import (
4141
"github.com/coder/coder/v2/coderd/userpassword"
4242
"github.com/coder/coder/v2/codersdk"
4343
"github.com/coder/coder/v2/cryptorand"
44-
"github.com/coder/coder/v2/site"
4544
)
4645

4746
const (
@@ -665,7 +664,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
665664
})
666665
cookies, user, key, err := api.oauthLogin(r, params)
667666
defer params.CommitAuditLogs()
668-
var httpErr httpError
667+
var httpErr idpsync.HttpError
669668
if xerrors.As(err, &httpErr) {
670669
httpErr.Write(rw, r)
671670
return
@@ -1065,7 +1064,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
10651064
})
10661065
cookies, user, key, err := api.oauthLogin(r, params)
10671066
defer params.CommitAuditLogs()
1068-
var httpErr httpError
1067+
var httpErr idpsync.HttpError
10691068
if xerrors.As(err, &httpErr) {
10701069
httpErr.Write(rw, r)
10711070
return
@@ -1093,7 +1092,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
10931092
}
10941093

10951094
// oidcGroups returns the groups for the user from the OIDC claims.
1096-
func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interface{}) (bool, []string, *httpError) {
1095+
func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interface{}) (bool, []string, *idpsync.HttpError) {
10971096
logger := api.Logger.Named(userAuthLoggerName)
10981097
usingGroups := false
10991098
var groups []string
@@ -1114,11 +1113,11 @@ func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interfac
11141113
slog.F("type", fmt.Sprintf("%T", groupsRaw)),
11151114
slog.Error(err),
11161115
)
1117-
return false, nil, &httpError{
1118-
code: http.StatusBadRequest,
1119-
msg: "Failed to sync groups from OIDC claims",
1120-
detail: err.Error(),
1121-
renderStaticPage: false,
1116+
return false, nil, &idpsync.HttpError{
1117+
Code: http.StatusBadRequest,
1118+
Msg: "Failed to sync groups from OIDC claims",
1119+
Detail: err.Error(),
1120+
RenderStaticPage: false,
11221121
}
11231122
}
11241123

@@ -1147,11 +1146,11 @@ func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interfac
11471146
if len(groups) == 0 {
11481147
detail = "You are currently not a member of any groups! Ask an administrator to add you to an authorized group to login."
11491148
}
1150-
return usingGroups, groups, &httpError{
1151-
code: http.StatusForbidden,
1152-
msg: "Not a member of an allowed group",
1153-
detail: detail,
1154-
renderStaticPage: true,
1149+
return usingGroups, groups, &idpsync.HttpError{
1150+
Code: http.StatusForbidden,
1151+
Msg: "Not a member of an allowed group",
1152+
Detail: detail,
1153+
RenderStaticPage: true,
11551154
}
11561155
}
11571156
}
@@ -1171,7 +1170,7 @@ func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interfac
11711170
// It would be preferred to just return an error, however this function
11721171
// decorates returned errors with the appropriate HTTP status codes and details
11731172
// that are hard to carry in a standard `error` without more work.
1174-
func (api *API) oidcRoles(ctx context.Context, mergedClaims map[string]interface{}) ([]string, *httpError) {
1173+
func (api *API) oidcRoles(ctx context.Context, mergedClaims map[string]interface{}) ([]string, *idpsync.HttpError) {
11751174
roles := api.OIDCConfig.UserRolesDefault
11761175
if !api.OIDCConfig.RoleSyncEnabled() {
11771176
return roles, nil
@@ -1193,11 +1192,11 @@ func (api *API) oidcRoles(ctx context.Context, mergedClaims map[string]interface
11931192
slog.F("type", fmt.Sprintf("%T", rolesRow)),
11941193
slog.Error(err),
11951194
)
1196-
return nil, &httpError{
1197-
code: http.StatusInternalServerError,
1198-
msg: "Login disabled until OIDC config is fixed",
1199-
detail: fmt.Sprintf("Roles claim must be an array of strings, type found: %T. Disabling role sync will allow login to proceed.", rolesRow),
1200-
renderStaticPage: false,
1195+
return nil, &idpsync.HttpError{
1196+
Code: http.StatusInternalServerError,
1197+
Msg: "Login disabled until OIDC config is fixed",
1198+
Detail: fmt.Sprintf("Roles claim must be an array of strings, type found: %T. Disabling role sync will allow login to proceed.", rolesRow),
1199+
RenderStaticPage: false,
12011200
}
12021201
}
12031202

@@ -1318,43 +1317,6 @@ func (p *oauthLoginParams) CommitAuditLogs() {
13181317
}
13191318
}
13201319

1321-
type httpError struct {
1322-
code int
1323-
msg string
1324-
detail string
1325-
renderStaticPage bool
1326-
1327-
renderDetailMarkdown bool
1328-
}
1329-
1330-
func (e httpError) Write(rw http.ResponseWriter, r *http.Request) {
1331-
if e.renderStaticPage {
1332-
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
1333-
Status: e.code,
1334-
HideStatus: true,
1335-
Title: e.msg,
1336-
Description: e.detail,
1337-
RetryEnabled: false,
1338-
DashboardURL: "/login",
1339-
1340-
RenderDescriptionMarkdown: e.renderDetailMarkdown,
1341-
})
1342-
return
1343-
}
1344-
httpapi.Write(r.Context(), rw, e.code, codersdk.Response{
1345-
Message: e.msg,
1346-
Detail: e.detail,
1347-
})
1348-
}
1349-
1350-
func (e httpError) Error() string {
1351-
if e.detail != "" {
1352-
return e.detail
1353-
}
1354-
1355-
return e.msg
1356-
}
1357-
13581320
func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.Cookie, database.User, database.APIKey, error) {
13591321
var (
13601322
ctx = r.Context()
@@ -1391,13 +1353,12 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
13911353
if api.OIDCConfig != nil && api.OIDCConfig.SignupsDisabledText != "" {
13921354
signupsDisabledText = render.HTMLFromMarkdown(api.OIDCConfig.SignupsDisabledText)
13931355
}
1394-
return httpError{
1395-
code: http.StatusForbidden,
1396-
msg: "Signups are disabled",
1397-
detail: signupsDisabledText,
1398-
renderStaticPage: true,
1399-
1400-
renderDetailMarkdown: true,
1356+
return &idpsync.HttpError{
1357+
Code: http.StatusForbidden,
1358+
Msg: "Signups are disabled",
1359+
Detail: signupsDisabledText,
1360+
RenderStaticPage: true,
1361+
RenderDetailMarkdown: true,
14011362
}
14021363
}
14031364

@@ -1443,9 +1404,9 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
14431404
}
14441405
}
14451406
if !validUsername {
1446-
return httpError{
1447-
code: http.StatusConflict,
1448-
msg: fmt.Sprintf("exhausted alternatives for taken username %q", original),
1407+
return &idpsync.HttpError{
1408+
Code: http.StatusConflict,
1409+
Msg: fmt.Sprintf("exhausted alternatives for taken username %q", original),
14491410
}
14501411
}
14511412
}
@@ -1586,11 +1547,11 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
15861547
//nolint:gocritic
15871548
err := api.Options.SetUserSiteRoles(dbauthz.AsSystemRestricted(ctx), logger, tx, user.ID, filtered)
15881549
if err != nil {
1589-
return httpError{
1590-
code: http.StatusBadRequest,
1591-
msg: "Invalid roles through OIDC claims",
1592-
detail: fmt.Sprintf("Error from role assignment attempt: %s", err.Error()),
1593-
renderStaticPage: true,
1550+
return &idpsync.HttpError{
1551+
Code: http.StatusBadRequest,
1552+
Msg: "Invalid roles through OIDC claims",
1553+
Detail: fmt.Sprintf("Error from role assignment attempt: %s", err.Error()),
1554+
RenderStaticPage: true,
15941555
}
15951556
}
15961557
if len(ignored) > 0 {
@@ -1701,17 +1662,17 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data
17011662
// Trying to convert to OIDC, but the email does not match.
17021663
// So do not make a new user, just block the request.
17031664
if user.ID == uuid.Nil {
1704-
return database.User{}, httpError{
1705-
code: http.StatusBadRequest,
1706-
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),
1665+
return database.User{}, idpsync.HttpError{
1666+
Code: http.StatusBadRequest,
1667+
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),
17071668
}
17081669
}
17091670

17101671
jwtCookie, err := r.Cookie(OAuthConvertCookieValue)
17111672
if err != nil {
1712-
return database.User{}, httpError{
1713-
code: http.StatusBadRequest,
1714-
msg: fmt.Sprintf("Convert to oauth cookie not found. Missing signed jwt to authorize this action. " +
1673+
return database.User{}, idpsync.HttpError{
1674+
Code: http.StatusBadRequest,
1675+
Msg: fmt.Sprintf("Convert to oauth cookie not found. Missing signed jwt to authorize this action. " +
17151676
"Please try again."),
17161677
}
17171678
}
@@ -1721,15 +1682,15 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data
17211682
})
17221683
if xerrors.Is(err, jwt.ErrSignatureInvalid) || !token.Valid {
17231684
// These errors are probably because the user is mixing 2 coder deployments.
1724-
return database.User{}, httpError{
1725-
code: http.StatusBadRequest,
1726-
msg: "Using an invalid jwt to authorize this action. Ensure there is only 1 coder deployment and try again.",
1685+
return database.User{}, idpsync.HttpError{
1686+
Code: http.StatusBadRequest,
1687+
Msg: "Using an invalid jwt to authorize this action. Ensure there is only 1 coder deployment and try again.",
17271688
}
17281689
}
17291690
if err != nil {
1730-
return database.User{}, httpError{
1731-
code: http.StatusInternalServerError,
1732-
msg: fmt.Sprintf("Error parsing jwt: %v", err),
1691+
return database.User{}, idpsync.HttpError{
1692+
Code: http.StatusInternalServerError,
1693+
Msg: fmt.Sprintf("Error parsing jwt: %v", err),
17331694
}
17341695
}
17351696

@@ -1749,16 +1710,16 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data
17491710
oauthConvertAudit.Old = user
17501711

17511712
if claims.RegisteredClaims.Issuer != api.DeploymentID {
1752-
return database.User{}, httpError{
1753-
code: http.StatusForbidden,
1754-
msg: "Request to convert login type failed. Issuer mismatch. Found a cookie from another coder deployment, please try again.",
1713+
return database.User{}, idpsync.HttpError{
1714+
Code: http.StatusForbidden,
1715+
Msg: "Request to convert login type failed. Issuer mismatch. Found a cookie from another coder deployment, please try again.",
17551716
}
17561717
}
17571718

17581719
if params.State.StateString != claims.State {
1759-
return database.User{}, httpError{
1760-
code: http.StatusForbidden,
1761-
msg: "Request to convert login type failed. State mismatch.",
1720+
return database.User{}, idpsync.HttpError{
1721+
Code: http.StatusForbidden,
1722+
Msg: "Request to convert login type failed. State mismatch.",
17621723
}
17631724
}
17641725

@@ -1768,9 +1729,9 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data
17681729
if user.ID != claims.UserID ||
17691730
codersdk.LoginType(user.LoginType) != claims.FromLoginType ||
17701731
codersdk.LoginType(params.LoginType) != claims.ToLoginType {
1771-
return database.User{}, httpError{
1772-
code: http.StatusForbidden,
1773-
msg: fmt.Sprintf("Request to convert login type from %s to %s failed", user.LoginType, params.LoginType),
1732+
return database.User{}, idpsync.HttpError{
1733+
Code: http.StatusForbidden,
1734+
Msg: fmt.Sprintf("Request to convert login type from %s to %s failed", user.LoginType, params.LoginType),
17741735
}
17751736
}
17761737

@@ -1784,9 +1745,9 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data
17841745
UserID: user.ID,
17851746
})
17861747
if err != nil {
1787-
return database.User{}, httpError{
1788-
code: http.StatusInternalServerError,
1789-
msg: "Failed to convert user to new login type",
1748+
return database.User{}, idpsync.HttpError{
1749+
Code: http.StatusInternalServerError,
1750+
Msg: "Failed to convert user to new login type",
17901751
}
17911752
}
17921753
oauthConvertAudit.New = user
@@ -1872,16 +1833,16 @@ func clearOAuthConvertCookie() *http.Cookie {
18721833
}
18731834
}
18741835

1875-
func wrongLoginTypeHTTPError(user database.LoginType, params database.LoginType) httpError {
1836+
func wrongLoginTypeHTTPError(user database.LoginType, params database.LoginType) idpsync.HttpError {
18761837
addedMsg := ""
18771838
if user == database.LoginTypePassword {
18781839
addedMsg = " You can convert your account to use this login type by visiting your account settings."
18791840
}
1880-
return httpError{
1881-
code: http.StatusForbidden,
1882-
renderStaticPage: true,
1883-
msg: "Incorrect login type",
1884-
detail: fmt.Sprintf("Attempting to use login type %q, but the user has the login type %q.%s",
1841+
return idpsync.HttpError{
1842+
Code: http.StatusForbidden,
1843+
RenderStaticPage: true,
1844+
Msg: "Incorrect login type",
1845+
Detail: fmt.Sprintf("Attempting to use login type %q, but the user has the login type %q.%s",
18851846
params, user, addedMsg),
18861847
}
18871848
}

0 commit comments

Comments
 (0)