Skip to content

chore: use httpError to allow better error elevation #11065

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.

Sign up for GitHub

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 1 commit into from
Dec 6, 2023
Merged

chore: use httpError to allow better error elevation #11065

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 22 additions & 23 deletions coderd/userauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,18 +925,15 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
picture, _ = pictureRaw.(string)
}

usingGroups, groups, err := api.oidcGroups(ctx, mergedClaims)
if err != nil {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Failed to sync groups from OIDC claims",
Detail: err.Error(),
})
usingGroups, groups, groupErr := api.oidcGroups(ctx, mergedClaims)
if groupErr != nil {
groupErr.Write(rw, r)
return
}

roles, ok := api.oidcRoles(ctx, rw, r, mergedClaims)
if !ok {
// oidcRoles writes the error to the response writer for us.
roles, roleErr := api.oidcRoles(ctx, mergedClaims)
if roleErr != nil {
roleErr.Write(rw, r)
return
}

Expand Down Expand Up @@ -1009,7 +1006,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
}

// oidcGroups returns the groups for the user from the OIDC claims.
func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interface{}) (bool, []string, error) {
func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interface{}) (bool, []string, *httpError) {
logger := api.Logger.Named(userAuthLoggerName)
usingGroups := false
var groups []string
Expand All @@ -1026,7 +1023,12 @@ func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interfac
slog.F("type", fmt.Sprintf("%T", groupsRaw)),
slog.Error(err),
)
return false, nil, err
return false, nil, &httpError{
code: http.StatusBadRequest,
msg: "Failed to sync groups from OIDC claims",
detail: err.Error(),
renderStaticPage: false,
}
}

api.Logger.Debug(ctx, "groups returned in oidc claims",
Expand Down Expand Up @@ -1058,10 +1060,10 @@ func (api *API) oidcGroups(ctx context.Context, mergedClaims map[string]interfac
// It would be preferred to just return an error, however this function
// decorates returned errors with the appropriate HTTP status codes and details
// that are hard to carry in a standard `error` without more work.
func (api *API) oidcRoles(ctx context.Context, rw http.ResponseWriter, r *http.Request, mergedClaims map[string]interface{}) ([]string, bool) {
func (api *API) oidcRoles(ctx context.Context, mergedClaims map[string]interface{}) ([]string, *httpError) {
roles := api.OIDCConfig.UserRolesDefault
if !api.OIDCConfig.RoleSyncEnabled() {
return roles, true
return roles, nil
}

rolesRow, ok := mergedClaims[api.OIDCConfig.UserRoleField]
Expand All @@ -1080,15 +1082,12 @@ func (api *API) oidcRoles(ctx context.Context, rw http.ResponseWriter, r *http.R
slog.F("type", fmt.Sprintf("%T", rolesRow)),
slog.Error(err),
)
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
Status: http.StatusInternalServerError,
HideStatus: true,
Title: "Login disabled until OIDC config is fixed",
Description: fmt.Sprintf("Roles claim must be an array of strings, type found: %T. Disabling role sync will allow login to proceed.", rolesRow),
RetryEnabled: false,
DashboardURL: "/login",
})
return nil, false
return nil, &httpError{
code: http.StatusInternalServerError,
msg: "Login disabled until OIDC config is fixed",
detail: fmt.Sprintf("Roles claim must be an array of strings, type found: %T. Disabling role sync will allow login to proceed.", rolesRow),
renderStaticPage: false,
}
}

api.Logger.Debug(ctx, "roles returned in oidc claims",
Expand All @@ -1107,7 +1106,7 @@ func (api *API) oidcRoles(ctx context.Context, rw http.ResponseWriter, r *http.R

roles = append(roles, role)
}
return roles, true
return roles, nil
}

// claimFields returns the sorted list of fields in the claims map.
Expand Down