chore: display warnings on app share failure (#9783)

* chore: add warnings to app share failure
Warnings only appear if the app is misconfigured to the deployment
This commit is contained in:
Steven Masley
2023-09-19 16:54:51 -05:00
committed by GitHub
parent 1fd1c654a9
commit a18bf73131
4 changed files with 101 additions and 15 deletions

View File

@ -10,6 +10,7 @@ import (
"strings" "strings"
"time" "time"
"golang.org/x/exp/slices"
"golang.org/x/xerrors" "golang.org/x/xerrors"
"cdr.dev/slog" "cdr.dev/slog"
@ -100,7 +101,7 @@ func (p *DBTokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *
// Lookup workspace app details from DB. // Lookup workspace app details from DB.
dbReq, err := appReq.getDatabase(dangerousSystemCtx, p.Database) dbReq, err := appReq.getDatabase(dangerousSystemCtx, p.Database)
if xerrors.Is(err, sql.ErrNoRows) { if xerrors.Is(err, sql.ErrNoRows) {
WriteWorkspaceApp404(p.Logger, p.DashboardURL, rw, r, &appReq, err.Error()) WriteWorkspaceApp404(p.Logger, p.DashboardURL, rw, r, &appReq, nil, err.Error())
return nil, "", false return nil, "", false
} else if err != nil { } else if err != nil {
WriteWorkspaceApp500(p.Logger, p.DashboardURL, rw, r, &appReq, err, "get app details from database") WriteWorkspaceApp500(p.Logger, p.DashboardURL, rw, r, &appReq, err, "get app details from database")
@ -114,7 +115,7 @@ func (p *DBTokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *
} }
// Verify the user has access to the app. // Verify the user has access to the app.
authed, err := p.authorizeRequest(r.Context(), authz, dbReq) authed, warnings, err := p.authorizeRequest(r.Context(), authz, dbReq)
if err != nil { if err != nil {
WriteWorkspaceApp500(p.Logger, p.DashboardURL, rw, r, &appReq, err, "verify authz") WriteWorkspaceApp500(p.Logger, p.DashboardURL, rw, r, &appReq, err, "verify authz")
return nil, "", false return nil, "", false
@ -122,7 +123,7 @@ func (p *DBTokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *
if !authed { if !authed {
if apiKey != nil { if apiKey != nil {
// The request has a valid API key but insufficient permissions. // The request has a valid API key but insufficient permissions.
WriteWorkspaceApp404(p.Logger, p.DashboardURL, rw, r, &appReq, "insufficient permissions") WriteWorkspaceApp404(p.Logger, p.DashboardURL, rw, r, &appReq, warnings, "insufficient permissions")
return nil, "", false return nil, "", false
} }
@ -218,7 +219,12 @@ func (p *DBTokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *
return &token, tokenStr, true return &token, tokenStr, true
} }
func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *httpmw.Authorization, dbReq *databaseRequest) (bool, error) { // authorizeRequest returns true/false if the request is authorized. The returned []string
// are warnings that aid in debugging. These messages do not prevent authorization,
// but may indicate that the request is not configured correctly.
// If an error is returned, the request should be aborted with a 500 error.
func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *httpmw.Authorization, dbReq *databaseRequest) (bool, []string, error) {
var warnings []string
accessMethod := dbReq.AccessMethod accessMethod := dbReq.AccessMethod
if accessMethod == "" { if accessMethod == "" {
accessMethod = AccessMethodPath accessMethod = AccessMethodPath
@ -233,6 +239,14 @@ func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *httpmw.Au
// Dangerous.AllowPathAppSiteOwnerAccess flag is enabled in the check below. // Dangerous.AllowPathAppSiteOwnerAccess flag is enabled in the check below.
sharingLevel := dbReq.AppSharingLevel sharingLevel := dbReq.AppSharingLevel
if isPathApp && !p.DeploymentValues.Dangerous.AllowPathAppSharing.Value() { if isPathApp && !p.DeploymentValues.Dangerous.AllowPathAppSharing.Value() {
if dbReq.AppSharingLevel != database.AppSharingLevelOwner {
// This is helpful for debugging, and ok to leak to the user.
// This is because the app has the sharing level set to something that
// should be shared, but we are disabling it from a deployment wide
// flag. So the template should be fixed to set the sharing level to
// "owner" instead and this will not appear.
warnings = append(warnings, fmt.Sprintf("unable to use configured sharing level %q because path-based app sharing is disabled (see --dangerous-allow-path-app-sharing), using sharing level \"owner\" instead", sharingLevel))
}
sharingLevel = database.AppSharingLevelOwner sharingLevel = database.AppSharingLevelOwner
} }
@ -240,7 +254,7 @@ func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *httpmw.Au
if roles == nil { if roles == nil {
// The user is not authenticated, so they can only access the app if it // The user is not authenticated, so they can only access the app if it
// is public. // is public.
return sharingLevel == database.AppSharingLevelPublic, nil return sharingLevel == database.AppSharingLevelPublic, warnings, nil
} }
// Block anyone from accessing workspaces they don't own in path-based apps // Block anyone from accessing workspaces they don't own in path-based apps
@ -254,7 +268,13 @@ func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *httpmw.Au
sharingLevel == database.AppSharingLevelOwner && sharingLevel == database.AppSharingLevelOwner &&
dbReq.Workspace.OwnerID.String() != roles.Actor.ID && dbReq.Workspace.OwnerID.String() != roles.Actor.ID &&
!p.DeploymentValues.Dangerous.AllowPathAppSiteOwnerAccess.Value() { !p.DeploymentValues.Dangerous.AllowPathAppSiteOwnerAccess.Value() {
return false, nil // This is not ideal to check for the 'owner' role, but we are only checking
// to determine whether to show a warning for debugging reasons. This does
// not do any authz checks, so it is ok.
if roles != nil && slices.Contains(roles.Actor.Roles.Names(), rbac.RoleOwner()) {
warnings = append(warnings, "path-based apps with \"owner\" share level are only accessible by the workspace owner (see --dangerous-allow-path-app-site-owner-access)")
}
return false, warnings, nil
} }
// Figure out which RBAC resource to check. For terminals we use execution // Figure out which RBAC resource to check. For terminals we use execution
@ -280,7 +300,7 @@ func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *httpmw.Au
// scope allows it). // scope allows it).
err := p.Authorizer.Authorize(ctx, roles.Actor, rbacAction, rbacResource) err := p.Authorizer.Authorize(ctx, roles.Actor, rbacAction, rbacResource)
if err == nil { if err == nil {
return true, nil return true, []string{}, nil
} }
switch sharingLevel { switch sharingLevel {
@ -293,15 +313,15 @@ func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *httpmw.Au
// to connect to the actor's own workspace. This enforces scopes. // to connect to the actor's own workspace. This enforces scopes.
err := p.Authorizer.Authorize(ctx, roles.Actor, rbacAction, rbacResourceOwned) err := p.Authorizer.Authorize(ctx, roles.Actor, rbacAction, rbacResourceOwned)
if err == nil { if err == nil {
return true, nil return true, []string{}, nil
} }
case database.AppSharingLevelPublic: case database.AppSharingLevelPublic:
// We don't really care about scopes and stuff if it's public anyways. // We don't really care about scopes and stuff if it's public anyways.
// Someone with a restricted-scope API key could just not submit the API // Someone with a restricted-scope API key could just not submit the API
// key cookie in the request and access the page. // key cookie in the request and access the page.
return true, nil return true, []string{}, nil
} }
// No checks were successful. // No checks were successful.
return false, nil return false, warnings, nil
} }

View File

@ -10,16 +10,19 @@ import (
// WriteWorkspaceApp404 writes a HTML 404 error page for a workspace app. If // WriteWorkspaceApp404 writes a HTML 404 error page for a workspace app. If
// appReq is not nil, it will be used to log the request details at debug level. // appReq is not nil, it will be used to log the request details at debug level.
func WriteWorkspaceApp404(log slog.Logger, accessURL *url.URL, rw http.ResponseWriter, r *http.Request, appReq *Request, msg string) { //
// The 'warnings' parameter is sent to the user, 'details' is only shown in the logs.
func WriteWorkspaceApp404(log slog.Logger, accessURL *url.URL, rw http.ResponseWriter, r *http.Request, appReq *Request, warnings []string, details string) {
if appReq != nil { if appReq != nil {
slog.Helper() slog.Helper()
log.Debug(r.Context(), log.Debug(r.Context(),
"workspace app 404: "+msg, "workspace app 404: "+details,
slog.F("username_or_id", appReq.UsernameOrID), slog.F("username_or_id", appReq.UsernameOrID),
slog.F("workspace_and_agent", appReq.WorkspaceAndAgent), slog.F("workspace_and_agent", appReq.WorkspaceAndAgent),
slog.F("workspace_name_or_id", appReq.WorkspaceNameOrID), slog.F("workspace_name_or_id", appReq.WorkspaceNameOrID),
slog.F("agent_name_or_id", appReq.AgentNameOrID), slog.F("agent_name_or_id", appReq.AgentNameOrID),
slog.F("app_slug_or_port", appReq.AppSlugOrPort), slog.F("app_slug_or_port", appReq.AppSlugOrPort),
slog.F("warnings", warnings),
) )
} }
@ -29,6 +32,7 @@ func WriteWorkspaceApp404(log slog.Logger, accessURL *url.URL, rw http.ResponseW
Description: "The application or workspace you are trying to access does not exist or you do not have permission to access it.", Description: "The application or workspace you are trying to access does not exist or you do not have permission to access it.",
RetryEnabled: false, RetryEnabled: false,
DashboardURL: accessURL.String(), DashboardURL: accessURL.String(),
Warnings: warnings,
}) })
} }

View File

@ -754,6 +754,7 @@ type ErrorPageData struct {
Description string Description string
RetryEnabled bool RetryEnabled bool
DashboardURL string DashboardURL string
Warnings []string
} }
// RenderStaticErrorPage renders the static error page. This is used by app // RenderStaticErrorPage renders the static error page. This is used by app

View File

@ -38,7 +38,7 @@ running). */}}
text-align: center; text-align: center;
} }
svg { .coder-svg {
width: 80px; width: 80px;
margin-bottom: 24px; margin-bottom: 24px;
} }
@ -49,11 +49,18 @@ running). */}}
margin-bottom: 8px; margin-bottom: 8px;
} }
p { p,
li {
color: #b2bfd7; color: #b2bfd7;
line-height: 140%; line-height: 140%;
} }
.warning li {
text-align: left;
padding-top: 10px;
margin-left: 30px;
}
.button-group { .button-group {
display: flex; display: flex;
align-items: center; align-items: center;
@ -83,11 +90,41 @@ running). */}}
.button-group button:hover { .button-group button:hover {
border-color: hsl(222, 31%, 40%); border-color: hsl(222, 31%, 40%);
} }
.warning {
margin-top: 24px;
border: 1px solid rgb(243, 140, 89);
background: rgb(13, 19, 33);
width: calc(520px + var(--side-padding) * 2);
/* Recenter */
margin-left: calc(-1 * (100px + var(--side-padding)));
padding: 10px;
}
.warning-title {
display: inline-flex;
align-self: center;
align-items: center;
}
.svg-icon svg {
height: 1em;
width: 1em;
}
.warning-title h3 {
margin-left: 10px;
}
</style> </style>
</head> </head>
<body> <body>
<div class="container"> <div class="container">
<svg viewBox="0 0 36 36" fill="none" xmlns="http://www.w3.org/2000/svg"> <svg
class="coder-svg"
viewBox="0 0 36 36"
fill="none"
xmlns="http://www.w3.org/2000/svg"
>
<g clip-path="url(#clip0_1094_2915)"> <g clip-path="url(#clip0_1094_2915)">
<path <path
d="M32.9812 15.9039C32.326 15.9039 31.8894 15.5197 31.8894 14.7311V10.202C31.8894 7.31059 30.6982 5.71326 27.6211 5.71326H26.1917V8.76638H26.6285C27.8394 8.76638 28.4152 9.43363 28.4152 10.6266V14.63C28.4152 16.3689 28.9313 17.0766 30.0629 17.4405C28.9313 17.7843 28.4152 18.5122 28.4152 20.251C28.4152 21.2418 28.4152 22.2325 28.4152 23.2233C28.4152 24.0523 28.4152 24.8611 28.1968 25.69C27.9784 26.4584 27.6211 27.1863 27.1248 27.8131C26.8468 28.1771 26.5292 28.4803 26.1719 28.7635V29.1678H27.6012C30.6784 29.1678 31.8696 27.5705 31.8696 24.6791V20.1499C31.8696 19.3411 32.2863 18.9772 32.9614 18.9772H33.7754V15.924H32.9812V15.9039Z" d="M32.9812 15.9039C32.326 15.9039 31.8894 15.5197 31.8894 14.7311V10.202C31.8894 7.31059 30.6982 5.71326 27.6211 5.71326H26.1917V8.76638H26.6285C27.8394 8.76638 28.4152 9.43363 28.4152 10.6266V14.63C28.4152 16.3689 28.9313 17.0766 30.0629 17.4405C28.9313 17.7843 28.4152 18.5122 28.4152 20.251C28.4152 21.2418 28.4152 22.2325 28.4152 23.2233C28.4152 24.0523 28.4152 24.8611 28.1968 25.69C27.9784 26.4584 27.6211 27.1863 27.1248 27.8131C26.8468 28.1771 26.5292 28.4803 26.1719 28.7635V29.1678H27.6012C30.6784 29.1678 31.8696 27.5705 31.8696 24.6791V20.1499C31.8696 19.3411 32.2863 18.9772 32.9614 18.9772H33.7754V15.924H32.9812V15.9039Z"
@ -131,6 +168,30 @@ running). */}}
.Error.Title }} .Error.Title }}
</h1> </h1>
<p>{{ .Error.Description }}</p> <p>{{ .Error.Description }}</p>
{{- if .Error.Warnings }}
<div class="warning">
<div class="warning-title">
<svg
height="1em"
width="auto"
focusable="false"
aria-hidden="true"
viewBox="0 0 24 24"
>
<path
fill="#e66828"
d="M12 5.99L19.53 19H4.47L12 5.99M12 2L1 21h22L12 2zm1 14h-2v2h2v-2zm0-6h-2v4h2v-4z"
></path>
</svg>
<h3>Warnings</h3>
</div>
<ul>
{{ range $i, $v := .Error.Warnings }}
<li>{{ $v }}</li>
{{ end }}
</ul>
</div>
{{ end }}
<div class="button-group"> <div class="button-group">
{{- if .Error.RetryEnabled }} {{- if .Error.RetryEnabled }}
<button onclick="window.location.reload()">Retry</button> <button onclick="window.location.reload()">Retry</button>