fix: check user user is active in aibridge auth (#26173) (#26264)

jdomeracki-coder · pawbana · f0ssel · web-flow · commit 943b04f66320 · 2026-06-11T14:02:17.000-04:00
Cherry-pick backport to `release/2.34`.

Co-authored-by: Paweł Banaszewski &lt;pawel@coder.com&gt;
Co-authored-by: Garrett Delfosse &lt;delfossegarrett@gmail.com&gt;
diff --git a/coderd/aibridgedserver/aibridgedserver.go b/coderd/aibridgedserver/aibridgedserver.go
@@ -43,6 +43,7 @@ var (
 	ErrExpired       = xerrors.New("expired")
 	ErrUnknownUser   = xerrors.New("unknown user")
 	ErrDeletedUser   = xerrors.New("deleted user")
+	ErrInactiveUser  = xerrors.New("inactive user")
 	ErrSystemUser    = xerrors.New("system user")
 	ErrAmbiguousAuth = xerrors.New("both key and key_id set; exactly one required")
 
@@ -623,10 +624,13 @@ func (s *Server) IsAuthorized(ctx context.Context, in *proto.IsAuthorizedRequest
 		return nil, ErrUnknownUser
 	}
 
-	// User is not deleted or a system user.
+	// User is active, not deleted, and not a system user.
 	if user.Deleted {
 		return nil, ErrDeletedUser
 	}
+	if user.Status != database.UserStatusActive {
+		return nil, ErrInactiveUser
+	}
 	if user.IsSystem {
 		return nil, ErrSystemUser
 	}
diff --git a/coderd/aibridgedserver/aibridgedserver_test.go b/coderd/aibridgedserver/aibridgedserver_test.go
@@ -102,16 +102,36 @@ func TestAuthorization(t *testing.T) {
 			name:        "deleted user",
 			expectedErr: aibridgedserver.ErrDeletedUser,
 			mocksFn: func(db *dbmock.MockStore, apiKey database.APIKey, user database.User) {
+				user.Deleted = true
 				db.EXPECT().GetAPIKeyByID(gomock.Any(), apiKey.ID).Times(1).Return(apiKey, nil)
-				db.EXPECT().GetUserByID(gomock.Any(), user.ID).Times(1).Return(database.User{ID: user.ID, Deleted: true}, nil)
+				db.EXPECT().GetUserByID(gomock.Any(), user.ID).Times(1).Return(user, nil)
+			},
+		},
+		{
+			name:        "suspended user",
+			expectedErr: aibridgedserver.ErrInactiveUser,
+			mocksFn: func(db *dbmock.MockStore, apiKey database.APIKey, user database.User) {
+				user.Status = database.UserStatusSuspended
+				db.EXPECT().GetAPIKeyByID(gomock.Any(), apiKey.ID).Times(1).Return(apiKey, nil)
+				db.EXPECT().GetUserByID(gomock.Any(), user.ID).Times(1).Return(user, nil)
+			},
+		},
+		{
+			name:        "dormant user",
+			expectedErr: aibridgedserver.ErrInactiveUser,
+			mocksFn: func(db *dbmock.MockStore, apiKey database.APIKey, user database.User) {
+				user.Status = database.UserStatusDormant
+				db.EXPECT().GetAPIKeyByID(gomock.Any(), apiKey.ID).Times(1).Return(apiKey, nil)
+				db.EXPECT().GetUserByID(gomock.Any(), user.ID).Times(1).Return(user, nil)
 			},
 		},
 		{
 			name:        "system user",
 			expectedErr: aibridgedserver.ErrSystemUser,
 			mocksFn: func(db *dbmock.MockStore, apiKey database.APIKey, user database.User) {
+				user.IsSystem = true
 				db.EXPECT().GetAPIKeyByID(gomock.Any(), apiKey.ID).Times(1).Return(apiKey, nil)
-				db.EXPECT().GetUserByID(gomock.Any(), user.ID).Times(1).Return(database.User{ID: user.ID, IsSystem: true}, nil)
+				db.EXPECT().GetUserByID(gomock.Any(), user.ID).Times(1).Return(user, nil)
 			},
 		},
 		{
@@ -201,7 +221,7 @@ func TestAuthorization(t *testing.T) {
 
 // When IsAuthorizedRequest carries KeyId instead of Key, the server skips
 // the secret check and validates only that the key exists, is unexpired, and
-// belongs to a non-deleted non-system user. This is the path used by
+// belongs to an active, non-deleted, non-system user. This is the path used by
 // in-process delegated callers (e.g., chatd) that hold only the key ID.
 func TestAuthorization_Delegated(t *testing.T) {
 	t.Parallel()
@@ -260,8 +280,31 @@ func TestAuthorization_Delegated(t *testing.T) {
 			name:        "deleted user",
 			expectedErr: aibridgedserver.ErrDeletedUser,
 			mocksFn: func(db *dbmock.MockStore, apiKey database.APIKey, user database.User) {
+				user.Deleted = true
 				db.EXPECT().GetAPIKeyByID(gomock.Any(), apiKey.ID).Times(1).Return(apiKey, nil)
-				db.EXPECT().GetUserByID(gomock.Any(), user.ID).Times(1).Return(database.User{ID: user.ID, Deleted: true}, nil)
+				db.EXPECT().GetUserByID(gomock.Any(), user.ID).Times(1).Return(user, nil)
+			},
+		},
+		{
+			// The delegated path must reject inactive users; transport
+			// trust does not override account suspension.
+			name:        "suspended user",
+			expectedErr: aibridgedserver.ErrInactiveUser,
+			mocksFn: func(db *dbmock.MockStore, apiKey database.APIKey, user database.User) {
+				user.Status = database.UserStatusSuspended
+				db.EXPECT().GetAPIKeyByID(gomock.Any(), apiKey.ID).Times(1).Return(apiKey, nil)
+				db.EXPECT().GetUserByID(gomock.Any(), user.ID).Times(1).Return(user, nil)
+			},
+		},
+		{
+			// Dormant users are inactive unless they are explicitly
+			// reactivated through the HTTP middleware path.
+			name:        "dormant user",
+			expectedErr: aibridgedserver.ErrInactiveUser,
+			mocksFn: func(db *dbmock.MockStore, apiKey database.APIKey, user database.User) {
+				user.Status = database.UserStatusDormant
+				db.EXPECT().GetAPIKeyByID(gomock.Any(), apiKey.ID).Times(1).Return(apiKey, nil)
+				db.EXPECT().GetUserByID(gomock.Any(), user.ID).Times(1).Return(user, nil)
 			},
 		},
 		{
@@ -270,8 +313,9 @@ func TestAuthorization_Delegated(t *testing.T) {
 			name:        "system user",
 			expectedErr: aibridgedserver.ErrSystemUser,
 			mocksFn: func(db *dbmock.MockStore, apiKey database.APIKey, user database.User) {
+				user.IsSystem = true
 				db.EXPECT().GetAPIKeyByID(gomock.Any(), apiKey.ID).Times(1).Return(apiKey, nil)
-				db.EXPECT().GetUserByID(gomock.Any(), user.ID).Times(1).Return(database.User{ID: user.ID, IsSystem: true}, nil)
+				db.EXPECT().GetUserByID(gomock.Any(), user.ID).Times(1).Return(user, nil)
 			},
 		},
 	}
