mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(coderd/workspaceapps): verify workspace owner matches app username (#26085)
fix(coderd/workspaceapps): verify workspace owner matches app username When resolving a workspace app by workspace UUID, the URL's username segment was never reconciled against the resolved workspace's owner. A user could serve their own workspace app from a hostname embedding another user's username, so the parsed origin username belonged to the victim. Combined with the username-equality CORS check, this allowed credentialed cross-origin reads of the victim's app responses. Reject the request with a 404 when the resolved workspace's owner does not match the user named in the request. Refs: https://linear.app/codercom/issue/PLAT-260
This commit is contained in:
@@ -1247,6 +1247,34 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
|
||||
assertWorkspaceLastUsedAtNotUpdated(t, appDetails, testutil.WaitLong)
|
||||
})
|
||||
|
||||
// Security (PLAT-260): must 404 when the URL username segment
|
||||
// names a different owner than the resolved workspace.
|
||||
t.Run("WorkspaceUUIDOwnerMismatchShould404", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
appDetails := setupProxyTest(t, nil)
|
||||
otherUserClient, otherUser := coderdtest.CreateAnotherUser(t, appDetails.SDKClient, appDetails.FirstUser.OrganizationID, rbac.RoleMember())
|
||||
appClient := appDetails.AppClient(t)
|
||||
appClient.SetSessionToken(otherUserClient.SessionToken())
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
forgedApp := appDetails.Apps.Public
|
||||
forgedApp.Username = otherUser.Username
|
||||
forgedApp.WorkspaceName = appDetails.Workspace.ID.String()
|
||||
|
||||
resp, err := requestWithRetries(ctx, t, appClient, http.MethodGet, appDetails.SubdomainAppURL(forgedApp).String(), nil)
|
||||
require.NoError(t, err)
|
||||
defer resp.Body.Close()
|
||||
require.Equal(t, http.StatusNotFound, resp.StatusCode)
|
||||
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
require.NoError(t, err)
|
||||
require.Contains(t, string(body), "404 - Application Not Found")
|
||||
assertWorkspaceLastUsedAtNotUpdated(t, appDetails, testutil.WaitLong)
|
||||
})
|
||||
|
||||
t.Run("RedirectsWithSlash", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -908,6 +908,50 @@ func Test_ResolveRequest(t *testing.T) {
|
||||
require.Len(t, connLogger.ConnectionLogs(), 0)
|
||||
})
|
||||
|
||||
// Security (PLAT-260): a UUID workspace lookup must reject when
|
||||
// the URL's username segment names a different owner. Otherwise a
|
||||
// same-owner origin can be spoofed for credentialed cross-origin
|
||||
// reads.
|
||||
t.Run("WorkspaceUUIDOwnerMismatch", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
req := (workspaceapps.Request{
|
||||
AccessMethod: workspaceapps.AccessMethodPath,
|
||||
BasePath: "/app",
|
||||
UsernameOrID: secondUser.Username,
|
||||
WorkspaceNameOrID: workspace.ID.String(),
|
||||
AgentNameOrID: agentName,
|
||||
AppSlugOrPort: appNamePublic,
|
||||
}).Normalize()
|
||||
|
||||
connLogger := connectionlog.NewFake()
|
||||
auditableIP := testutil.RandomIPv6(t)
|
||||
|
||||
rw := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("GET", "/app", nil)
|
||||
r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken())
|
||||
r.RemoteAddr = auditableIP
|
||||
|
||||
token, ok := workspaceappsResolveRequest(t, connLogger, rw, r, workspaceapps.ResolveRequestOptions{
|
||||
Logger: api.Logger,
|
||||
SignedTokenProvider: api.WorkspaceAppsProvider,
|
||||
DashboardURL: api.AccessURL,
|
||||
PathAppBaseURL: api.AccessURL,
|
||||
AppHostname: api.AppHostname,
|
||||
AppRequest: req,
|
||||
})
|
||||
require.False(t, ok)
|
||||
require.Nil(t, token)
|
||||
|
||||
w := rw.Result()
|
||||
defer w.Body.Close()
|
||||
b, err := io.ReadAll(w.Body)
|
||||
require.NoError(t, err)
|
||||
require.Contains(t, string(b), "404 - Application Not Found")
|
||||
require.Equal(t, http.StatusNotFound, w.StatusCode)
|
||||
require.Len(t, connLogger.ConnectionLogs(), 0)
|
||||
})
|
||||
|
||||
t.Run("RedirectSubdomainAuth", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -248,6 +248,9 @@ func (r Request) getDatabase(ctx context.Context, db database.Store) (*databaseR
|
||||
)
|
||||
if workspaceID, uuidErr := uuid.Parse(r.WorkspaceNameOrID); uuidErr == nil {
|
||||
workspace, workspaceErr = db.GetWorkspaceByID(ctx, workspaceID)
|
||||
if workspaceErr == nil && workspace.OwnerID != user.ID {
|
||||
workspaceErr = sql.ErrNoRows
|
||||
}
|
||||
} else {
|
||||
workspace, workspaceErr = db.GetWorkspaceByOwnerIDAndName(ctx, database.GetWorkspaceByOwnerIDAndNameParams{
|
||||
OwnerID: user.ID,
|
||||
|
||||
Reference in New Issue
Block a user