implement generic file accessor class

[M4LuZ]

What is this PR doing?

WIP Implementation of a secure file accessor that makes working with files and folders easier while simultaneously preventing directory traversal both below LS root and any set intermediate path.
All current file accesses to be replaced with this

Which issue(s) this PR fixes:

no issue created
refs #710

Checklist

• CHANGELOG.md entry
• Documentation update

[M4LuZ]

@andygrunwald : Would appreciate your feedback on the general design and how that works based on the example file changed

[andygrunwald]

I am not 100% sure about the design of the class. Or lets say, purpose of the class.

Is the main purpose to represent one single file? Or is the purpose to enable Lansuite to initiate a File() instance once and handle multiple files?

From the name of the class, my first thought would be to handle one single file. If we loop over multiple files, for each file, a File() instance would be created.

In this case, I suggest

• to add the file path for the constructor as a first parameter
• to remove the argument from functions like getFullPath and exists, because the class only handles one file

In this case, it would also make sense to make a "delete" and "output" method.

LanSuite can still provide a helper function like "NewFile()" which sets the filesystem injection and root path.

Overall, I think we should do this, because we have multiple places where we deal with files.

If you think this is useful, I can also iterate over it a bit if you want :)

[M4LuZ]

HI @andygrunwald ,

thanks for the helfpul feedback.

Basically this was inteded as helper class for all file access operations.
Eplicitly not on a per-file basis, but more as per-file-access-context.
So that it can be used to define a subpart of the filesystem as area where access can/will hapen, potentially providing also further scoping options (e.g. filename filtering, extension / MIME-type filtering, both with a default set and the option to set additionals per object instance).

I am pondering whether addidng direct access methods like outputFileContent makes sense to it.
The intent was to ensure with that that the file path is OK before the file is read, thus that method .
This could be potentially be moved to a separate class for direct file operations?

I was close to naming the Class FileAccessor, based on your comments I think that helps to understand purpose and usage a lot better.

[andygrunwald]

What do you think about a pure Object Oriented approach here?
Creating two classes:

• File
• FileCollection

File

It is a single file and offers functions on a single file. Things like

• Exists
• Output
• Rename
• Delete
• ...

FileCollection

Represents a list/number of files. Can be initialized via

• An array of files
• By a folder

Internally, it has multiple File() objects.

Why?

This way we would have a clean and unit testable result. Responsibilities are clear and the security aspects (which triggered the PR) can also be taken into account.
The FileCollection can be used in modules like gallery, downloads, etc.

Thoughts?

[M4LuZ]

Thoughts?

#worksforme ;)

Was mentally already halfway there, your idea of using an array of files for initialisation for FileCollection makes a ton of sense.
For most of the proposed File class methods I would use functions that Symphony/Filesystem already provides, either as internal calls or maybe even by inheriting them as a child class?

[andygrunwald]

For most of the proposed File class methods I would use functions that Symphony/Filesystem already provides

This is true. Still, I would shim them away from the actual implementation and create a LanSuite File class as a Wrapper. This way we can exchange the lib as we want, without doing a big rewrite.

[sonarcloud]

Quality Gate failed

Failed conditions
1 Security Hotspot
3.8% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

[sonarcloud]

Quality Gate failed

Failed conditions
1 Security Hotspot
3.7% Duplication on New Code (required ≤ 3%)
E Security Rating on New Code (required ≥ A)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint