Remove sensitive shared_secret data from occ config:list output

[settermjd]

Description

This PR applies a patch from @phil-davis, that ensures that log.condition shared_secret sensitive data is removed from calls to occ config:list output.

Motivation and Context

'log.condition' => [
        [
            'shared_secret' => '57b58edb6637fe3059b3595cf9c41b9',
            'users' => ['user1'],
            'apps' => ['files_texteditor'],
            'logfile' => '/tmp/test.log'
        ],
    ],

If you had the above configuration in config/config.php, then the shared_secret information would not be stripped, from the output of a call to occ config:list. However, with the PR applied, the output would appear as follows:

"log.condition": [
      {
          "shared_secret": "***REMOVED SENSITIVE VALUE***",
           "users": [
               "user1"
           ],
           "apps": [
               "files_texteditor"
           ],
           "logfile": "\/tmp\/test.log"
      },
]

How Has This Been Tested?

Currently, it's only been manually tested.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:

Open tasks:

• Backport (if applicable set "backport-request" label and remove when the backport was done)

[phil-davis]

My patch had a code-style fail. I pushed a 2nd commit that has the little code-style fix and unit tests.

[phil-davis]

It would be nice to have acceptance tests for occ commands like this. @patrickjahns raised a general issue about that: owncloud/QA#570
It won't happen overnight, but it will happen. (Separate to this PR)

[settermjd]

@phil-davis, in this case, would unit tests not be more suitable?

[phil-davis]

Yes, unit tests are good - they check that the underlying classes/methods do the right thing.

Acceptance tests check that the actual occ commands have actually been hooked up through the code to the underlying classes/methods that they should be. e.g. in this case if the occ config:list command is not actually written to end up making use of the class with getFilteredValue() then it won't do the job - no matter how many unit tests of getFilteredValue() are passing.

[phil-davis]

@settermjd this PR has been started from a very old master - I will rebase it. drone CI is complaining about apiSharing suite not found. That was renamed a long time ago.

You should pull master branch to your local copy of core repo to get it up-to-date.

[codecov]

Codecov Report

Merging #31984 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master   #31984      +/-   ##
============================================
+ Coverage     63.49%   63.51%   +0.02%     
- Complexity    18545    18549       +4     
============================================
  Files          1167     1167              
  Lines         69544    69548       +4     
  Branches       1264     1264              
============================================
+ Hits          44154    44173      +19     
+ Misses        25021    25006      -15     
  Partials        369      369

Flag	Coverage Δ	Complexity Δ
#javascript	52.59% <ø> (ø)	0 <ø> (ø)	⬇️
#phpunit	64.76% <100%> (+0.02%)	18549 <0> (+4)	⬆️

Impacted Files	Coverage Δ	Complexity Δ
lib/private/SystemConfig.php	100% <100%> (+55.55%)	18 <0> (+4)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 539389c...2607fce. Read the comment docs.

[PVince81]

see comment

[PVince81]

I have the feeling that this was already in place but was missing the "s", see line 52.

AFAIK there is no setting called singular "log.condition", see config.sample.php

[phil-davis]

log.condition is still backward-compatible supported. See lib/private/Log.php function log - the first few lines check log.conditions andd then fall back to log.condition

[PVince81]

Please backport

[phil-davis]

Backport stable10 #31997

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.