Fix X-Robots-Tag header valid values check

[rizlas]

Description

Currently the only accepted value for the robots tag is none but:

• duplicate headers are acceptable https://stackoverflow.com/questions/4371328/are-duplicate-http-response-headers-acceptable
• the value none is only used by Google. Check this https://developer.mozilla.org/en-US/docs/Web/HTML/Element/meta/name and https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers.
• X-Robots-Tag': 'noindex nofollow', should be considered as valid

Related Issue

• Fixes #40713

Motivation and Context

Allow all the possible values of x-robots-tag header and handle duplicate headers if OC is behind a proxy

How Has This Been Tested?

• test environment: fully working environment with OC 10.12 (docker)
• test case 1: x-robots-tag header values: 'none, noindex, nofollow, nosnippet, noarchive'
• test case 2: x-robots-tag header values: 'none, nosnippet, noarchive'
• test case 3: x-robots-tag header values: 'none'
• test case 4: x-robots-tag header values: 'nofollow, nosnippet' [security warning raised]
• test case 5: x-robots-tag header values: 'nosnippet, noarchive' [security warning raised]
• test case 6: x-robots-tag header values: 'noindex, nofollow'
• test case 7: x-robots-tag header values: '' [security warning raised]

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:
• Changelog item, see TEMPLATE

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[CLAassistant]

All committers have signed the CLA.

[phil-davis]

Reviewers - after review is all done, merge this and include in 10.12.1 or wait?

[jvillafanez]

You might also want to create a function in order to move all the checks there. The idea would be to add support for arrays, so you could use 'X-Robots-Tag': 'none' or 'X-Robots-Tag': ['none', 'noindex', 'nofollow'],.

Note that I'm just commenting about the code itself. I don't know if the code is right or wrong.

[jnweiger]

Reviewers - after review is all done, merge this and include in 10.12.1 or wait?

I'd say wait.
a) because 10.12.1 is about to be shipped. and
b) this one touches security - hardening related topics. We need deeper understanding why we recommend 'none'.
Reference:
https://doc.owncloud.com/server/next/admin_manual/configuration/server/harden_server.html#use-of-security-related-headers-on-the-web-server

My understanding here:
Both alternatives: a) none + optional extra values and also b) noindex + nofollow + optional extra values effectively mean 'none'. So it seems to be safe to me. We need to take care, that in the case b) we need to assert that both are present, and not just one of them. (or one of them combined with none).

[rizlas]

@jvillafanez array is used only during validation. It is not a valid type input. Imho a separate function is not needed.

@jnweiger combinations of none, noindex, nofollow, nosnippet, noarchive are 32 (2^5) and they are:

''
none
noindex
nofollow
nosnippet
noarchive
none, noindex
none, nofollow
none, nosnippet
none, noarchive
noindex, nofollow
noindex, nosnippet
noindex, noarchive
nofollow, nosnippet
nofollow, noarchive
nosnippet, noarchive
none, noindex, nofollow
none, noindex, nosnippet
none, noindex, noarchive
none, nofollow, nosnippet
none, nofollow, noarchive
none, nosnippet, noarchive
noindex, nofollow, nosnippet
noindex, nofollow, noarchive
noindex, nosnippet, noarchive
nofollow, nosnippet, noarchive
none, noindex, nofollow, nosnippet
none, noindex, nofollow, noarchive
none, noindex, nosnippet, noarchive
none, nofollow, nosnippet, noarchive
noindex, nofollow, nosnippet, noarchive
none, noindex, nofollow, nosnippet, noarchive

With the current code these combinations are not valid:

''
noindex
nofollow
nosnippet
noarchive
noindex, nosnippet
noindex, noarchive
nofollow, nosnippet
nofollow, noarchive
nosnippet, noarchive
noindex, nosnippet, noarchive
nofollow, nosnippet, noarchive

On the contrary these are valid input:

none
none, noindex
none, nofollow
none, nosnippet
none, noarchive
noindex, nofollow
none, noindex, nofollow
none, noindex, nosnippet
none, noindex, noarchive
none, nofollow, nosnippet
none, nofollow, noarchive
none, nosnippet, noarchive
noindex, nofollow, nosnippet
noindex, nofollow, noarchive
none, noindex, nofollow, nosnippet
none, noindex, nofollow, noarchive
none, noindex, nosnippet, noarchive
none, nofollow, nosnippet, noarchive
noindex, nofollow, nosnippet, noarchive
none, noindex, nofollow, nosnippet, noarchive

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[jvillafanez]

This should be fine assuming there won't be changes in the checks. I mean, the "X-Robots-Tag" header will be the only exception to the exact match of key - values we're checking.

[phil-davis]

10.12.1 has been merged back to master, so we can merge new stuff like this.

[rizlas]

Thank you