diff --git a/coderd/aibridgedserver/aibridgedserver.go b/coderd/aibridgedserver/aibridgedserver.go index 8dbaa10bfa..94411de16d 100644 --- 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 index 9aeb082069..5930260e05 100644 --- 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) }, }, }