Rewrite OCS CSRF check to be readable

[provokateurin]

Summary

Previous code was unreadable and every time I come across it I had to read it multiple times to understand what it does. Should be much clearer now (at least it is to me).

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[provokateurin]

This PR was hell, the previous logic was really not readable and understandable and comment was misleading...

[blizzz]

Personally, you would increase readability by breaking long lines down into many =) (but with leading operators).

[christianlupus]

The changes I suggested are only cosmetic. As I used GitHub, I hope the indentations are correct and will satisfy any active cide style formatters.

One suggestion that might improve readability:
We could insert very small private methods that are just evaluating the if conditions. That would make it more readable, I guess. So, you would (for example) add a method

private function isInvalidCSRFRequired($reflectionMethod) {
	if ($this->hasAnnotationOrAttribute($reflectionMethod, 'NoCSRFRequired', NoCSRFRequired::class) {
		return false;
	}

	return ! $this->request->passesCSRFCheck();
}

This is for sure more verbose but I guess more readable, especially as the main beforeController method will offload significant parts of the logic. Adding comments to various parts of the condition methods will allow more human understanding as well.
What do you think about this?

Apart from that I did not find any flaws with the logic. The behavior should be 100% the same.

In general, I think this increases the readability and that should be done in any case. (Maybe even more)

[provokateurin]

@christianlupus I agree with you, it's a lot more readable when the logic is in a separate method. Then it's also easy to use early returns instead of all this mess.

[christianlupus]

This seems cleaner and I see no change in the implemented logic here.

[provokateurin]

How long do I have to rebase until this shit finally works 😅