search

akeneo / pim-community-dev

[Community Development Repository] The open source Product Information Management (PIM)
http://www.akeneo.com
Other
954 stars 514 forks source link

Acl not applied to show job #5538

Closed mathewrapid closed 7 years ago

mathewrapid commented 7 years ago

I'm reporting a Bug

When the user does not have rights to export profiles, the export job can still be accessed through the path /job/show/{id} whereas the /spread/export_execution/{id} correctly gives an 403 response.

pierallard commented 7 years ago

Hi @mathewrapid !

You're true, I can reproduce this bug on my environment. A fix is welcome, if you have time to contribute, we'll glad to help you !

mathewrapid commented 7 years ago

Hi @pierallard !

I'm thinking that it could be patched by checking the job type (export, _quickexport, import) in the controller and then checking for user rights.

What would you suggest?

a2xchip commented 7 years ago

Hello @mathewrapid The problem is that acl ancestors were not applied to https://github.com/akeneo/pim-community-dev/blob/master/src/Pim/Bundle/EnrichBundle/Controller/JobTrackerController.php#L104 method (and it looks to all other controller methods) like in case of https://github.com/akeneo/pim-community-dev/blob/master/src/Pim/Bundle/ImportExportBundle/Controller/ExportProfileController.php#L50

If you have additional question, you are welcome :-)

mathewrapid commented 7 years ago

Hi @a2xchip

Yes, that is the problem. Now, I'm 99% sure (correct me if I'm wrong) that this cannot be fixed by adding the acl ancestors to the JobTrackerController as it is used to view jobs regardless their type (import, export, etc.).

So, I'm wondering that what would be the best way to solve this? In JobTrackerController load the job and check its type and then apply the isGranted logic to it?

a2xchip commented 7 years ago

@mathewrapid Sounds good :-) @pierallard what do you think?

pierallard commented 7 years ago

Sorry to my late answer ! Yes, you're right, ACL annotations only work for "simple" cases. In this case, I think you can not use annotation... You have to know if the job is "import" or "export" before throwing an "unauthorized" response.

a2xchip commented 7 years ago

Hello @mathewrapid any news on this issue?

mathewrapid commented 7 years ago

Hi @a2xchip,

I think this is up for grabs. Have not had time to make a pr.

a2xchip commented 7 years ago

@mathewrapid Hello! Thx for quick reply, gonna fix it before 1.7 release :-)

willy-ahva commented 7 years ago

Hi @a2xchip !

Any update about a PR ? :)

a2xchip commented 7 years ago

Last 7 hours i was sleeping but coming ;-)

willy-ahva commented 7 years ago

Sorry @a2xchip I didn't see the time on your last message. Hope you slept well ! :p Have a good day !

mickaelandrieu commented 7 years ago

Hello,

issue fixed thanks to @a2xchip contribution.

Thanks for the report @mathewrapid!