fix(federation): remove plaintext auth token from error log in getSharedSecret

[DeepDiver1975]

Summary

• getSharedSecret (@publicpage) logged both the submitted and expected tokens in plaintext on failure: got "X" but expected "Y"
• Any unauthenticated attacker who knows a trusted server URL could trigger this log entry at will, harvesting the valid token from logs
• Fix removes the getToken() DB call that existed only to populate the log, replacing the message with a generic non-disclosing string

Security Impact

Medium — requires secondary log-read access to exploit; unauthenticated trigger

Note

This PR touches the same file as security/fix-federation-token-oracle — merge that one first to avoid conflicts.

Test plan

• testGetSharedSecretDoesNotLogTokenValues asserts neither attacker nor stored token appears in the logged string; fails without fix
• Run make test TEST_PHP_SUITE=apps/federation

🤖 Generated with Claude Code

[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.

[DeepDiver1975]

Code Review — fix(federation): remove plaintext auth token from error log in getSharedSecret

Overview: getSharedSecret (@PublicPage) logged the valid stored token in plaintext on authentication failure: got "X" but expected "Y". Since any unauthenticated caller can trigger this log entry for any trusted server URL, anyone with log-read access could harvest the valid federation token and replay it. The fix removes the getToken() call and replaces the log message with a non-disclosing string.

Correctness

The change is minimal: one line removed (the $expectedToken DB fetch), one string literal updated. The isValidToken() check and the 403 response path are unchanged. ✅

Does dbHandler->getToken() get called twice now? No — isValidToken() internally calls getToken() and does the comparison itself. The additional getToken() call that was removed existed solely to populate the log message. Removing it is correct. ✅

Test

testGetSharedSecretDoesNotLogTokenValues asserts that the logger->error message contains neither the attacker-supplied token nor the stored token:

$this->logicalAnd(
    $this->logicalNot($this->stringContains($attackerToken)),
    $this->logicalNot($this->stringContains($storedToken))
)

This directly tests the security property, not just that a log call happened. The test would fail against the old code (which embeds both values) and pass against the new code. ✅

Note in PR body: "merge that one first to avoid conflicts" (referring to #41579). Both PRs touch OCSAuthAPIController.php but at different line ranges, so a clean merge order matters. Review team should coordinate.

Summary

Aspect	Assessment
Token removed from log	✅ Both attacker-supplied and stored token scrubbed
Extra DB call removed	✅ No longer calls getToken() redundantly
Test	✅ Directly asserts neither value appears in log message
Merge dependency	⚠️ Merge #41579 first to avoid conflict

Verdict: Ready to merge (after #41579).