Yes, it is a bad practice. Controller should be used only to control the flow of your application and pass data to the view (JSON/HTML/XML/etc). Repository/DAO should be used to retrieve/persist the data without knowing any business logic.

Consider following scenarios:

• Somehow you need to add/modify role access for the data. You have to add the logic in any controllers that call the repository, or you have to put it in the repository (will broker logic of other services that call this repo). If you call via a service, you just need to modify the service, or just add another service, no need to worry will broke other services that call that repo.
• Let's say you have two clients of your application (Website and REST API), in the future, if you have to modify role access or business logic, you have to modify on both website and REST API controllers.

Yes, it is a bad practice. Controller should be used only to control the flow of your application, get input from client apps, call service and pass data to the view (JSON/HTML/XML/etc). Repository/DAO should be used to retrieve/persist the data without knowing any business logic.

Consider following scenarios:

• Somehow you need to add/modify role access for the data. You have to add the logic in any controllers that call the repository, or you have to put it in the repository (will broke logic of other services that call this repo). If you call via a service, you just need to modify the service, or just add another service, no need to worry will broke other services that call that repo.
• Let's say you have two clients of your application (Website and REST API), in the future, if you have to modify role access or business logic, you have to modify on both website and REST API controllers.