Skip to content

Commit a5853f6

Browse files
committed
fix(api): filter sessions by connector, as the request already claimed
ListAuthSessionsReq grew a connector_id field with the session rework and nothing ever read it, so a caller asking for one connector's sessions got every one the user had. It filters now, the example app's session view passes the connector it is already scoped to, and the RPC comment says what the message does. Also: several tests set a client state's AuthenticatedAt to a day in the future, left over from mapping the old per-client ExpiresAt onto it. Nothing reads the field today, which is exactly why it should not describe a user who authenticated tomorrow. Signed-off-by: maksim.nabokikh <max.nabokih@gmail.com>
1 parent 254312f commit a5853f6

6 files changed

Lines changed: 35 additions & 13 deletions

File tree

api/v2/api.proto

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,7 @@ service Dex {
574574
rpc VerifyPassword(VerifyPasswordReq) returns (VerifyPasswordResp) {};
575575
// GetAuthSession returns an auth session by its ID.
576576
rpc GetAuthSession(GetAuthSessionReq) returns (GetAuthSessionResp) {};
577-
// ListAuthSessions lists auth sessions, optionally filtered by user_id.
577+
// ListAuthSessions lists auth sessions, optionally filtered by user and connector.
578578
rpc ListAuthSessions(ListAuthSessionsReq) returns (ListAuthSessionsResp) {};
579579
// DeleteAuthSession deletes an auth session and revokes associated refresh tokens.
580580
rpc DeleteAuthSession(DeleteAuthSessionReq) returns (DeleteAuthSessionResp) {};

api/v2/api_grpc.pb.go

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

examples/example-app/server/admin.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,8 @@ func (s *Server) handleAdmin(w http.ResponseWriter, r *http.Request) {
230230
// connector gave the user, refresh tokens under the encoded sub claim
231231
// that ends up in tokens.
232232
if data.UserID != "" {
233-
if resp, err := s.admin.api.ListAuthSessions(ctx, &api.ListAuthSessionsReq{UserId: data.UserID}); err == nil {
233+
req := &api.ListAuthSessionsReq{UserId: data.UserID, ConnectorId: data.ConnectorID}
234+
if resp, err := s.admin.api.ListAuthSessions(ctx, req); err == nil {
234235
for _, sess := range resp.Sessions {
235236
data.Sessions = append(data.Sessions, AdminSession{
236237
ID: sess.Id,

server/apiserver/api_test.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -891,7 +891,7 @@ func TestGetAuthSession(t *testing.T) {
891891
ID: "nonce123", Secret: "nonce123",
892892
ClientStates: map[string]*storage.ClientAuthState{
893893
"client-a": {
894-
AuthenticatedAt: now.Add(24 * time.Hour),
894+
AuthenticatedAt: now,
895895
LastActivity: now,
896896
LastTokenIssuedAt: now,
897897
},
@@ -979,6 +979,24 @@ func TestListAuthSessions(t *testing.T) {
979979
if len(resp.Sessions) != 2 {
980980
t.Fatalf("expected 2 sessions for user1, got %d", len(resp.Sessions))
981981
}
982+
983+
// Filter by connector_id, and by both: one user signed in through two
984+
// connectors is two sessions, and a caller may want either or one of them.
985+
resp, err = client.ListAuthSessions(ctx, &api.ListAuthSessionsReq{ConnectorId: "conn1"})
986+
if err != nil {
987+
t.Fatalf("list auth sessions by connector: %v", err)
988+
}
989+
if len(resp.Sessions) != 2 {
990+
t.Fatalf("expected 2 sessions on conn1, got %d", len(resp.Sessions))
991+
}
992+
993+
resp, err = client.ListAuthSessions(ctx, &api.ListAuthSessionsReq{UserId: "user1", ConnectorId: "conn2"})
994+
if err != nil {
995+
t.Fatalf("list auth sessions by user and connector: %v", err)
996+
}
997+
if len(resp.Sessions) != 1 {
998+
t.Fatalf("expected 1 session for user1 on conn2, got %d", len(resp.Sessions))
999+
}
9821000
}
9831001

9841002
func TestDeleteAuthSession(t *testing.T) {

server/apiserver/sessions.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@ func (d dexAPI) ListAuthSessions(ctx context.Context, req *api.ListAuthSessionsR
7878
if req.UserId != "" && s.UserID != req.UserId {
7979
continue
8080
}
81+
if req.ConnectorId != "" && s.ConnectorID != req.ConnectorId {
82+
continue
83+
}
8184
sessions = append(sessions, storageAuthSessionToAPI(s))
8285
}
8386

server/authflow/sessionlogin_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ func TestCreateOrUpdateAuthSession(t *testing.T) {
380380
ID: nonce, Secret: nonce,
381381
ClientStates: map[string]*storage.ClientAuthState{
382382
"client-1": {
383-
AuthenticatedAt: now.Add(24 * time.Hour),
383+
AuthenticatedAt: now.Add(-10 * time.Minute),
384384
LastActivity: now.Add(-10 * time.Minute),
385385
},
386386
},
@@ -497,7 +497,7 @@ func setupSessionLoginFixture(t *testing.T, s *sessionTestServer) storage.AuthRe
497497
ID: "test-nonce", Secret: "test-nonce",
498498
ClientStates: map[string]*storage.ClientAuthState{
499499
"client-1": {
500-
AuthenticatedAt: now.Add(24 * time.Hour),
500+
AuthenticatedAt: now.Add(-1 * time.Minute),
501501
LastActivity: now.Add(-1 * time.Minute),
502502
},
503503
},
@@ -662,7 +662,7 @@ func setupSessionWithIdentity(t *testing.T, s *sessionTestServer, now time.Time,
662662
ID: nonce, Secret: nonce,
663663
ClientStates: map[string]*storage.ClientAuthState{
664664
"client-1": {
665-
AuthenticatedAt: now.Add(24 * time.Hour),
665+
AuthenticatedAt: now.Add(-1 * time.Minute),
666666
LastActivity: now.Add(-1 * time.Minute),
667667
},
668668
},
@@ -986,7 +986,7 @@ func TestFindSSOSession(t *testing.T) {
986986
ConnectorID: "mock",
987987
ClientStates: map[string]*storage.ClientAuthState{
988988
"client-a": {
989-
AuthenticatedAt: now.Add(24 * time.Hour),
989+
AuthenticatedAt: now.Add(-5 * time.Minute),
990990
LastActivity: now.Add(-5 * time.Minute),
991991
},
992992
},
@@ -1011,7 +1011,7 @@ func TestFindSSOSession(t *testing.T) {
10111011
ConnectorID: "mock",
10121012
ClientStates: map[string]*storage.ClientAuthState{
10131013
"client-a": {
1014-
AuthenticatedAt: now.Add(24 * time.Hour),
1014+
AuthenticatedAt: now.Add(-5 * time.Minute),
10151015
LastActivity: now.Add(-5 * time.Minute),
10161016
},
10171017
},
@@ -1038,7 +1038,7 @@ func TestFindSSOSession(t *testing.T) {
10381038
ConnectorID: "mock",
10391039
ClientStates: map[string]*storage.ClientAuthState{
10401040
"client-a": {
1041-
AuthenticatedAt: now.Add(24 * time.Hour),
1041+
AuthenticatedAt: now.Add(-5 * time.Minute),
10421042
LastActivity: now.Add(-5 * time.Minute),
10431043
},
10441044
},
@@ -1071,7 +1071,7 @@ func TestTrySessionLogin_SSO(t *testing.T) {
10711071
ID: "test-nonce", Secret: "test-nonce",
10721072
ClientStates: map[string]*storage.ClientAuthState{
10731073
"client-a": {
1074-
AuthenticatedAt: now.Add(24 * time.Hour),
1074+
AuthenticatedAt: now.Add(-1 * time.Minute),
10751075
LastActivity: now.Add(-1 * time.Minute),
10761076
},
10771077
},
@@ -1217,7 +1217,7 @@ func TestTrySessionLogin_SSO(t *testing.T) {
12171217
ID: "test-nonce", Secret: "test-nonce",
12181218
ClientStates: map[string]*storage.ClientAuthState{
12191219
"client-a": {
1220-
AuthenticatedAt: now.Add(24 * time.Hour),
1220+
AuthenticatedAt: now.Add(-1 * time.Minute),
12211221
LastActivity: now.Add(-1 * time.Minute),
12221222
},
12231223
},

0 commit comments

Comments
 (0)