🐛 Attachments to host objects without org_id can not be downloaded from the portal

[jbostoen]

Use case: there's a (custom) class that's not tied to an organization (so no org_id). In the iTop configuration file, it's made possible to add attachments (itop-attachments).

Now, when this info (e.g. a class "Release" containing a description of release notes + one or more attachments) is published in the web portal, the error does not have sufficient rights.

This happens because it tries to query attachments which are published in the organizations the user has access to.

However, since in this case the attachment's "item_org_id" is 0, the user doesn't have access.

To address this, the PR adds a simple condition which will bypass the security check (allowing all data) when the attachment's item_org_id is 0.

[Hipska]

I was looking into it, but are you sure the user profile has rights to read that class?

It seems like now you are bypassing a lot of security: As soon as an class with attachment has no org_id, all users can download the attachments even if they don't have rights to the class.. To me, this is even worse than the scenario you described before as it can be seen as a security leak..

[jbostoen]

@Hipska you're right, thanks for pointing this out! I was under the impression the security of the host object would still be checked, but this would also be bypassed with this boolean, which is not the goal at all.

I'll adjust the condition in this PR to include this check as well.

[piRGoif]

I had teh same impression, thanks Hipska ! I'm changing the PR status, Jeffrey tell us when you'll be ready for review !

[Hipska]

@jbostoen I still don't know if this is the case:

I was looking into it, but are you sure the user profile has rights to read that class?

[jbostoen]

I've updated the condition to check for rights on the host object of the attachment. I think it's fine now?

[Hipska]

That doesn't answer my question. I believe this whole PR isn't needed..

[jbostoen]

I'm sure the user has the rights to view the host object, he can clearly see it in the portal :)

[Hipska]

You know the portal bypasses that with the scopes, right?

[jbostoen]

Yes. The user can properly see the host object in the portal. AFAIK no separate rights for "attachments" linked to the host object are supposedly needed?

When trying to set this up, the user could see the host object and the attachments were listed; but downloading failed due to lack of rights.

[Hipska]

That's why I specifically ask if the class is in the allowed matrix for any of the profiles this user has.

[jbostoen]

It's in the allowed matrix, both in the back-end/console "portal user" profile as well as in the specific classes scope for the front-end user portal

[piRGoif]

Ok thanks One dev should have a close look at your proposal (I will set the status accordingly). I don't think it can be reviewed this month sorry :/.

[jbostoen]

@piRGoif any update please?

[piRGoif]

Hello, Before presenting a PR to the reviews a Combodo employee has to take a close look and be able to present the proposition internally. For now your PR wasn't handled by anyone at Combodo (no one assigned). We have a backlog of such PR unfortunately... I can't give any date as there are no official resources planned to handled PR (just good will and an amount of time)

[Molkobain]

Technical review: We must discuss this with the product team first as it is a functional change (next week). Then will check if the fix is the right one.

[Molkobain]

Discussed during functional review:

• In the backoffice, are you able to download the attachments of the said custom class? I understand that your fix is only on the portal, but we are curious.
• Can you test it again in both the backoffice and the portal (with your fix) with a user that has some allowed organizations? Can you download the attachments?

[jbostoen]

Just tested it: as a regular agent (non-admin) with a specified organization, you can upload an attachment (for example if you enable it for FAQ). The upload gets stored correctly, but not retrieved.

Shall I modify that too, or do you guys prefer a different approach?

[Molkobain]

Are you talking in backoffice or the portal? Can you test the 4 cases and give the individual result? Thanks

[jbostoen]

• Back-office - agent just has a "support agent" role and one giving access to FAQ (something with 'problem' in the name):
  • Restricted to specific allowed organization: with and without my fix: agent is unable to see the attachment. Agent can upload an attachment, it gets stored in DB, but is never rendered since item_org_id is 0. It gets saved with a "temp" ID, with item ID set to 0. Upon saving the FAQ, it never gets updated.
  • Not restricted: attachment gets saved properly.
• Portal: without fix: user is unable to download an attachment linked to a class with no org_id (~ FAQ)
• Portal: with fix: user is able to download the attachment - only if they are allowed to view the "host" object.

Use cases could be attachments for FAQs, or in my example a modified class "release" which contains release info + an attached ZIP file.

[Molkobain]

The test cases are with or without an allowed org for the user (see "Allowed organizations" tab on the user object), sorry for the confusion.

[jbostoen]

I explicitly set an allowed org for the user object in this case. I've updated my previous comment to reflect something else: initially it's created as a temp attachment with a temp ID and key set to 0. This never gets updated either, and the attachment ultimately gets removed from the DB altogether.

This is not the case when the agent is not restricted to organizations.

[Molkobain]

Thanks Jeffrey, we will discuss with the product team about this.

[jbostoen]

@Molkobain Any idea when this will be discussed, please? :)

[piRGoif]

Hello Jeffrey, reviews occurs every month, first week the technical one, second week the functional one.

[piRGoif]

Functional review : OK for iTop 3.1 !