Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitdd9ae88

Browse files
committed
fix!: use client ip when creating connection logs for workspace proxied app accesses
1 parent6d39077 commitdd9ae88

File tree

12 files changed

+99
-11
lines changed

12 files changed

+99
-11
lines changed

‎coderd/database/dump.sql‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTERTABLE connection_logs ALTER COLUMN ipSETNOT NULL;
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
-- We can't guarantee that an IP will always be available, and omitting an IP
2+
-- is preferable to not creating a connection log at all.
3+
ALTERTABLE connection_logs ALTER COLUMN ip DROPNOT NULL;

‎coderd/workspaceapps/errors.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,18 @@ func WriteWorkspaceApp404(log slog.Logger, accessURL *url.URL, rw http.ResponseW
3939
})
4040
}
4141

42+
// WriteWorkspaceApp401 writes a generic authentication required HTML 401 error
43+
// page for a workspace app.
44+
funcWriteWorkspaceApp401(accessURL*url.URL,rw http.ResponseWriter,r*http.Request) {
45+
site.RenderStaticErrorPage(rw,r, site.ErrorPageData{
46+
Status:http.StatusUnauthorized,
47+
Title:"Unauthorized",
48+
Description:"Authentication required",
49+
RetryEnabled:false,
50+
DashboardURL:accessURL.String(),
51+
})
52+
}
53+
4254
// WriteWorkspaceApp500 writes a HTML 500 error page for a workspace app. If
4355
// appReq is not nil, it's fields will be added to the logged error message.
4456
funcWriteWorkspaceApp500(log slog.Logger,accessURL*url.URL,rw http.ResponseWriter,r*http.Request,appReq*Request,errerror,msgstring) {

‎coderd/workspaceapps/provider.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,15 @@ func ResolveRequest(rw http.ResponseWriter, r *http.Request, opts ResolveRequest
5454
returntoken,true
5555
}
5656

57+
ifappReq.AccessMethod==AccessMethodTerminal {
58+
// The client is expected to include the signed token in a query parameter
59+
// in the request. If that token doesn't exist or isn't valid then the
60+
// issuing code below won't work on proxies because there is no long
61+
// lived session token on the root proxy domain.
62+
WriteWorkspaceApp401(opts.DashboardURL,rw,r)
63+
returnnil,false
64+
}
65+
5766
issueReq:=IssueTokenRequest{
5867
AppRequest:appReq,
5968
PathAppBaseURL:opts.PathAppBaseURL.String(),

‎codersdk/connectionlog.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ type ConnectionLog struct {
2020
WorkspaceID uuid.UUID`json:"workspace_id" format:"uuid"`
2121
WorkspaceNamestring`json:"workspace_name"`
2222
AgentNamestring`json:"agent_name"`
23-
IP netip.Addr`json:"ip"`
23+
IP*netip.Addr`json:"ip,omitempty"`
2424
TypeConnectionType`json:"type"`
2525

2626
// WebInfo is only set when `type` is one of:

‎enterprise/coderd/connectionlog.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,13 @@ func convertConnectionLogs(dblogs []database.GetConnectionLogsOffsetRow) []coder
9393
}
9494

9595
funcconvertConnectionLog(dblog database.GetConnectionLogsOffsetRow) codersdk.ConnectionLog {
96-
ip,_:=netip.AddrFromSlice(dblog.ConnectionLog.Ip.IPNet.IP)
96+
varip*netip.Addr
97+
ifdblog.ConnectionLog.Ip.Valid {
98+
parsedIP,ok:=netip.AddrFromSlice(dblog.ConnectionLog.Ip.IPNet.IP)
99+
ifok {
100+
ip=&parsedIP
101+
}
102+
}
97103

98104
varuser*codersdk.User
99105
ifdblog.ConnectionLog.UserID.Valid {

‎enterprise/coderd/workspaceproxy.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,7 @@ func (api *API) workspaceProxyIssueSignedAppToken(rw http.ResponseWriter, r *htt
490490
return
491491
}
492492
userReq.Header.Set(codersdk.SessionTokenHeader,req.SessionToken)
493+
userReq.RemoteAddr=r.Header.Get(wsproxysdk.CoderWorkspaceProxyRealIPHeader)
493494

494495
// Exchange the token.
495496
token,tokenStr,ok:=api.AGPL.WorkspaceAppsProvider.Issue(ctx,rw,userReq,req)

‎enterprise/coderd/workspaceproxy_test.go‎

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package coderd_test
33
import (
44
"database/sql"
55
"fmt"
6+
"net"
67
"net/http"
78
"net/http/httptest"
89
"net/http/httputil"
@@ -12,13 +13,15 @@ import (
1213
"time"
1314

1415
"github.com/google/uuid"
16+
"github.com/sqlc-dev/pqtype"
1517
"github.com/stretchr/testify/assert"
1618
"github.com/stretchr/testify/require"
1719

1820
"cdr.dev/slog/sloggers/slogtest"
1921
"github.com/coder/coder/v2/agent/agenttest"
2022
"github.com/coder/coder/v2/buildinfo"
2123
"github.com/coder/coder/v2/coderd/coderdtest"
24+
"github.com/coder/coder/v2/coderd/connectionlog"
2225
"github.com/coder/coder/v2/coderd/database"
2326
"github.com/coder/coder/v2/coderd/database/db2sdk"
2427
"github.com/coder/coder/v2/coderd/database/dbgen"
@@ -610,13 +613,18 @@ func TestProxyRegisterDeregister(t *testing.T) {
610613
funcTestIssueSignedAppToken(t*testing.T) {
611614
t.Parallel()
612615

616+
connectionLogger:=connectionlog.NewFake()
617+
613618
client,user:=coderdenttest.New(t,&coderdenttest.Options{
619+
ConnectionLogging:true,
614620
Options:&coderdtest.Options{
615621
IncludeProvisionerDaemon:true,
622+
ConnectionLogger:connectionLogger,
616623
},
617624
LicenseOptions:&coderdenttest.LicenseOptions{
618625
Features: license.Features{
619626
codersdk.FeatureWorkspaceProxy:1,
627+
codersdk.FeatureConnectionLog:1,
620628
},
621629
},
622630
})
@@ -653,7 +661,7 @@ func TestIssueSignedAppToken(t *testing.T) {
653661
// Invalid request.
654662
AppRequest: workspaceapps.Request{},
655663
SessionToken:client.SessionToken(),
656-
})
664+
},"127.0.0.1")
657665
require.Error(t,err)
658666
})
659667

@@ -669,41 +677,70 @@ func TestIssueSignedAppToken(t *testing.T) {
669677
t.Parallel()
670678
proxyClient:=wsproxysdk.New(client.URL,proxyRes.ProxyToken)
671679

680+
fakeClientIP:="13.37.13.37"
681+
parsedFakeClientIP:= pqtype.Inet{
682+
Valid:true,IPNet: net.IPNet{
683+
IP:net.ParseIP(fakeClientIP),
684+
Mask:net.CIDRMask(32,32),
685+
},
686+
}
687+
672688
ctx:=testutil.Context(t,testutil.WaitLong)
673-
_,err:=proxyClient.IssueSignedAppToken(ctx,goodRequest)
689+
_,err:=proxyClient.IssueSignedAppToken(ctx,goodRequest,fakeClientIP)
674690
require.NoError(t,err)
691+
692+
require.True(t,connectionLogger.Contains(t, database.UpsertConnectionLogParams{
693+
Ip:parsedFakeClientIP,
694+
}))
675695
})
676696

677697
t.Run("OKHTML",func(t*testing.T) {
678698
t.Parallel()
679699
proxyClient:=wsproxysdk.New(client.URL,proxyRes.ProxyToken)
680700

701+
fakeClientIP:="192.168.1.100"
702+
parsedFakeClientIP:= pqtype.Inet{
703+
Valid:true,IPNet: net.IPNet{
704+
IP:net.ParseIP(fakeClientIP),
705+
Mask:net.CIDRMask(32,32),
706+
},
707+
}
708+
681709
rw:=httptest.NewRecorder()
682710
ctx:=testutil.Context(t,testutil.WaitLong)
683-
_,ok:=proxyClient.IssueSignedAppTokenHTML(ctx,rw,goodRequest)
711+
_,ok:=proxyClient.IssueSignedAppTokenHTML(ctx,rw,goodRequest,fakeClientIP)
684712
if!assert.True(t,ok,"expected true") {
685713
resp:=rw.Result()
686714
deferresp.Body.Close()
687715
dump,err:=httputil.DumpResponse(resp,true)
688716
require.NoError(t,err)
689717
t.Log(string(dump))
690718
}
719+
720+
require.True(t,connectionLogger.Contains(t, database.UpsertConnectionLogParams{
721+
Ip:parsedFakeClientIP,
722+
}))
691723
})
692724
}
693725

694726
funcTestReconnectingPTYSignedToken(t*testing.T) {
695727
t.Parallel()
696728

729+
connectionLogger:=connectionlog.NewFake()
730+
697731
db,pubsub:=dbtestutil.NewDB(t)
698732
client,closer,api,user:=coderdenttest.NewWithAPI(t,&coderdenttest.Options{
733+
ConnectionLogging:true,
699734
Options:&coderdtest.Options{
700735
Database:db,
701736
Pubsub:pubsub,
702737
IncludeProvisionerDaemon:true,
738+
ConnectionLogger:connectionLogger,
703739
},
704740
LicenseOptions:&coderdenttest.LicenseOptions{
705741
Features: license.Features{
706742
codersdk.FeatureWorkspaceProxy:1,
743+
codersdk.FeatureConnectionLog:1,
707744
},
708745
},
709746
})
@@ -887,6 +924,15 @@ func TestReconnectingPTYSignedToken(t *testing.T) {
887924

888925
// The token is validated in the apptest suite, so we don't need to
889926
// validate it here.
927+
928+
require.True(t,connectionLogger.Contains(t, database.UpsertConnectionLogParams{
929+
Ip: pqtype.Inet{
930+
Valid:true,IPNet: net.IPNet{
931+
IP:net.ParseIP("127.0.0.1"),
932+
Mask:net.CIDRMask(32,32),
933+
},
934+
},
935+
}))
890936
})
891937
}
892938

‎enterprise/wsproxy/tokenprovider.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func (p *TokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *ht
3939
}
4040
issueReq.AppRequest=appReq
4141

42-
resp,ok:=p.Client.IssueSignedAppTokenHTML(ctx,rw,issueReq)
42+
resp,ok:=p.Client.IssueSignedAppTokenHTML(ctx,rw,issueReq,r.RemoteAddr)
4343
if!ok {
4444
returnnil,"",false
4545
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp