diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 208ff53d8f..b097766f0d 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -891,7 +891,7 @@ CREATE TABLE connection_logs ( workspace_name text NOT NULL, agent_name text NOT NULL, type connection_type NOT NULL, - ip inet NOT NULL, + ip inet, code integer, user_agent text, user_id uuid, diff --git a/coderd/database/migrations/000369_nullable_conn_log_ip.down.sql b/coderd/database/migrations/000369_nullable_conn_log_ip.down.sql new file mode 100644 index 0000000000..caa9fce72e --- /dev/null +++ b/coderd/database/migrations/000369_nullable_conn_log_ip.down.sql @@ -0,0 +1 @@ +ALTER TABLE connection_logs ALTER COLUMN ip SET NOT NULL; diff --git a/coderd/database/migrations/000369_nullable_conn_log_ip.up.sql b/coderd/database/migrations/000369_nullable_conn_log_ip.up.sql new file mode 100644 index 0000000000..5e6b95f9f2 --- /dev/null +++ b/coderd/database/migrations/000369_nullable_conn_log_ip.up.sql @@ -0,0 +1,3 @@ +-- We can't guarantee that an IP will always be available, and omitting an IP +-- is preferable to not creating a connection log at all. +ALTER TABLE connection_logs ALTER COLUMN ip DROP NOT NULL; diff --git a/codersdk/connectionlog.go b/codersdk/connectionlog.go index 9dd78694b4..3e2acec6df 100644 --- a/codersdk/connectionlog.go +++ b/codersdk/connectionlog.go @@ -20,7 +20,7 @@ type ConnectionLog struct { WorkspaceID uuid.UUID `json:"workspace_id" format:"uuid"` WorkspaceName string `json:"workspace_name"` AgentName string `json:"agent_name"` - IP netip.Addr `json:"ip"` + IP *netip.Addr `json:"ip,omitempty"` Type ConnectionType `json:"type"` // WebInfo is only set when `type` is one of: diff --git a/enterprise/coderd/connectionlog.go b/enterprise/coderd/connectionlog.go index 21f0420f06..05e3a40b2d 100644 --- a/enterprise/coderd/connectionlog.go +++ b/enterprise/coderd/connectionlog.go @@ -93,7 +93,13 @@ func convertConnectionLogs(dblogs []database.GetConnectionLogsOffsetRow) []coder } func convertConnectionLog(dblog database.GetConnectionLogsOffsetRow) codersdk.ConnectionLog { - ip, _ := netip.AddrFromSlice(dblog.ConnectionLog.Ip.IPNet.IP) + var ip *netip.Addr + if dblog.ConnectionLog.Ip.Valid { + parsedIP, ok := netip.AddrFromSlice(dblog.ConnectionLog.Ip.IPNet.IP) + if ok { + ip = &parsedIP + } + } var user *codersdk.User if dblog.ConnectionLog.UserID.Valid { diff --git a/enterprise/coderd/workspaceproxy.go b/enterprise/coderd/workspaceproxy.go index 16fe079d20..2ebee9d93f 100644 --- a/enterprise/coderd/workspaceproxy.go +++ b/enterprise/coderd/workspaceproxy.go @@ -490,6 +490,7 @@ func (api *API) workspaceProxyIssueSignedAppToken(rw http.ResponseWriter, r *htt return } userReq.Header.Set(codersdk.SessionTokenHeader, req.SessionToken) + userReq.RemoteAddr = r.Header.Get(wsproxysdk.CoderWorkspaceProxyRealIPHeader) // Exchange the token. token, tokenStr, ok := api.AGPL.WorkspaceAppsProvider.Issue(ctx, rw, userReq, req) diff --git a/enterprise/coderd/workspaceproxy_test.go b/enterprise/coderd/workspaceproxy_test.go index 28d46c0137..d4be30d822 100644 --- a/enterprise/coderd/workspaceproxy_test.go +++ b/enterprise/coderd/workspaceproxy_test.go @@ -3,6 +3,7 @@ package coderd_test import ( "database/sql" "fmt" + "net" "net/http" "net/http/httptest" "net/http/httputil" @@ -12,6 +13,7 @@ import ( "time" "github.com/google/uuid" + "github.com/sqlc-dev/pqtype" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -19,6 +21,7 @@ import ( "github.com/coder/coder/v2/agent/agenttest" "github.com/coder/coder/v2/buildinfo" "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/connectionlog" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbgen" @@ -610,13 +613,18 @@ func TestProxyRegisterDeregister(t *testing.T) { func TestIssueSignedAppToken(t *testing.T) { t.Parallel() + connectionLogger := connectionlog.NewFake() + client, user := coderdenttest.New(t, &coderdenttest.Options{ + ConnectionLogging: true, Options: &coderdtest.Options{ IncludeProvisionerDaemon: true, + ConnectionLogger: connectionLogger, }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{ codersdk.FeatureWorkspaceProxy: 1, + codersdk.FeatureConnectionLog: 1, }, }, }) @@ -653,7 +661,7 @@ func TestIssueSignedAppToken(t *testing.T) { // Invalid request. AppRequest: workspaceapps.Request{}, SessionToken: client.SessionToken(), - }) + }, "127.0.0.1") require.Error(t, err) }) @@ -669,18 +677,38 @@ func TestIssueSignedAppToken(t *testing.T) { t.Parallel() proxyClient := wsproxysdk.New(client.URL, proxyRes.ProxyToken) + fakeClientIP := "13.37.13.37" + parsedFakeClientIP := pqtype.Inet{ + Valid: true, IPNet: net.IPNet{ + IP: net.ParseIP(fakeClientIP), + Mask: net.CIDRMask(32, 32), + }, + } + ctx := testutil.Context(t, testutil.WaitLong) - _, err := proxyClient.IssueSignedAppToken(ctx, goodRequest) + _, err := proxyClient.IssueSignedAppToken(ctx, goodRequest, fakeClientIP) require.NoError(t, err) + + require.True(t, connectionLogger.Contains(t, database.UpsertConnectionLogParams{ + Ip: parsedFakeClientIP, + })) }) t.Run("OKHTML", func(t *testing.T) { t.Parallel() proxyClient := wsproxysdk.New(client.URL, proxyRes.ProxyToken) + fakeClientIP := "192.168.1.100" + parsedFakeClientIP := pqtype.Inet{ + Valid: true, IPNet: net.IPNet{ + IP: net.ParseIP(fakeClientIP), + Mask: net.CIDRMask(32, 32), + }, + } + rw := httptest.NewRecorder() ctx := testutil.Context(t, testutil.WaitLong) - _, ok := proxyClient.IssueSignedAppTokenHTML(ctx, rw, goodRequest) + _, ok := proxyClient.IssueSignedAppTokenHTML(ctx, rw, goodRequest, fakeClientIP) if !assert.True(t, ok, "expected true") { resp := rw.Result() defer resp.Body.Close() @@ -688,22 +716,31 @@ func TestIssueSignedAppToken(t *testing.T) { require.NoError(t, err) t.Log(string(dump)) } + + require.True(t, connectionLogger.Contains(t, database.UpsertConnectionLogParams{ + Ip: parsedFakeClientIP, + })) }) } func TestReconnectingPTYSignedToken(t *testing.T) { t.Parallel() + connectionLogger := connectionlog.NewFake() + db, pubsub := dbtestutil.NewDB(t) client, closer, api, user := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ + ConnectionLogging: true, Options: &coderdtest.Options{ Database: db, Pubsub: pubsub, IncludeProvisionerDaemon: true, + ConnectionLogger: connectionLogger, }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{ codersdk.FeatureWorkspaceProxy: 1, + codersdk.FeatureConnectionLog: 1, }, }, }) @@ -887,6 +924,15 @@ func TestReconnectingPTYSignedToken(t *testing.T) { // The token is validated in the apptest suite, so we don't need to // validate it here. + + require.True(t, connectionLogger.Contains(t, database.UpsertConnectionLogParams{ + Ip: pqtype.Inet{ + Valid: true, IPNet: net.IPNet{ + IP: net.ParseIP("127.0.0.1"), + Mask: net.CIDRMask(32, 32), + }, + }, + })) }) } diff --git a/enterprise/wsproxy/tokenprovider.go b/enterprise/wsproxy/tokenprovider.go index 5093c60157..0f263157a5 100644 --- a/enterprise/wsproxy/tokenprovider.go +++ b/enterprise/wsproxy/tokenprovider.go @@ -39,7 +39,7 @@ func (p *TokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *ht } issueReq.AppRequest = appReq - resp, ok := p.Client.IssueSignedAppTokenHTML(ctx, rw, issueReq) + resp, ok := p.Client.IssueSignedAppTokenHTML(ctx, rw, issueReq, r.RemoteAddr) if !ok { return nil, "", false } diff --git a/enterprise/wsproxy/wsproxysdk/wsproxysdk.go b/enterprise/wsproxy/wsproxysdk/wsproxysdk.go index 15400a2d33..d768a919f4 100644 --- a/enterprise/wsproxy/wsproxysdk/wsproxysdk.go +++ b/enterprise/wsproxy/wsproxysdk/wsproxysdk.go @@ -21,6 +21,12 @@ import ( "github.com/coder/websocket" ) +const ( + // CoderWorkspaceProxyAuthTokenHeader is the header that contains the + // resolved real IP address of the client that made the request to the proxy. + CoderWorkspaceProxyRealIPHeader = "Coder-Workspace-Proxy-Real-IP" +) + // Client is a HTTP client for a subset of Coder API routes that external // proxies need. type Client struct { @@ -84,10 +90,11 @@ type IssueSignedAppTokenResponse struct { // IssueSignedAppToken issues a new signed app token for the provided app // request. The error page will be returned as JSON. For use in external // proxies, use IssueSignedAppTokenHTML instead. -func (c *Client) IssueSignedAppToken(ctx context.Context, req workspaceapps.IssueTokenRequest) (IssueSignedAppTokenResponse, error) { +func (c *Client) IssueSignedAppToken(ctx context.Context, req workspaceapps.IssueTokenRequest, clientIP string) (IssueSignedAppTokenResponse, error) { resp, err := c.RequestIgnoreRedirects(ctx, http.MethodPost, "/api/v2/workspaceproxies/me/issue-signed-app-token", req, func(r *http.Request) { // This forces any HTML error pages to be returned as JSON instead. r.Header.Set("Accept", "application/json") + r.Header.Set(CoderWorkspaceProxyRealIPHeader, clientIP) }) if err != nil { return IssueSignedAppTokenResponse{}, xerrors.Errorf("make request: %w", err) @@ -105,7 +112,7 @@ func (c *Client) IssueSignedAppToken(ctx context.Context, req workspaceapps.Issu // IssueSignedAppTokenHTML issues a new signed app token for the provided app // request. The error page will be returned as HTML in most cases, and will be // written directly to the provided http.ResponseWriter. -func (c *Client) IssueSignedAppTokenHTML(ctx context.Context, rw http.ResponseWriter, req workspaceapps.IssueTokenRequest) (IssueSignedAppTokenResponse, bool) { +func (c *Client) IssueSignedAppTokenHTML(ctx context.Context, rw http.ResponseWriter, req workspaceapps.IssueTokenRequest, clientIP string) (IssueSignedAppTokenResponse, bool) { writeError := func(rw http.ResponseWriter, err error) { res := codersdk.Response{ Message: "Internal server error", @@ -117,6 +124,7 @@ func (c *Client) IssueSignedAppTokenHTML(ctx context.Context, rw http.ResponseWr resp, err := c.RequestIgnoreRedirects(ctx, http.MethodPost, "/api/v2/workspaceproxies/me/issue-signed-app-token", req, func(r *http.Request) { r.Header.Set("Accept", "text/html") + r.Header.Set(CoderWorkspaceProxyRealIPHeader, clientIP) }) if err != nil { writeError(rw, xerrors.Errorf("perform issue signed app token request: %w", err)) diff --git a/enterprise/wsproxy/wsproxysdk/wsproxysdk_test.go b/enterprise/wsproxy/wsproxysdk/wsproxysdk_test.go index 6b4da6831c..ba6562d45c 100644 --- a/enterprise/wsproxy/wsproxysdk/wsproxysdk_test.go +++ b/enterprise/wsproxy/wsproxysdk/wsproxysdk_test.go @@ -22,6 +22,8 @@ import ( func Test_IssueSignedAppTokenHTML(t *testing.T) { t.Parallel() + fakeClientIP := "127.0.0.1" + t.Run("OK", func(t *testing.T) { t.Parallel() @@ -68,7 +70,7 @@ func Test_IssueSignedAppTokenHTML(t *testing.T) { tokenRes, ok := client.IssueSignedAppTokenHTML(ctx, rw, workspaceapps.IssueTokenRequest{ AppRequest: expectedAppReq, SessionToken: expectedSessionToken, - }) + }, fakeClientIP) if !assert.True(t, ok) { t.Log("issue request failed when it should've succeeded") t.Log("response dump:") @@ -118,7 +120,7 @@ func Test_IssueSignedAppTokenHTML(t *testing.T) { tokenRes, ok := client.IssueSignedAppTokenHTML(ctx, rw, workspaceapps.IssueTokenRequest{ AppRequest: workspaceapps.Request{}, SessionToken: "user-session-token", - }) + }, fakeClientIP) require.False(t, ok) require.Empty(t, tokenRes) require.True(t, rw.WasWritten()) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 4314d00ef4..8943e69289 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -351,7 +351,7 @@ export interface ConnectionLog { readonly workspace_id: string; readonly workspace_name: string; readonly agent_name: string; - readonly ip: string; + readonly ip?: string; readonly type: ConnectionType; readonly web_info?: ConnectionLogWebInfo; readonly ssh_info?: ConnectionLogSSHInfo;