Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE REQUEST] Don't redact auth type HTTP request logging #4257

Merged
merged 5 commits into from
Dec 15, 2023

Conversation

JuancaG05
Copy link
Collaborator

@JuancaG05 JuancaG05 commented Dec 13, 2023

Related Issues

App: #4249

  • Added changelog files for the fixed issues in folder changelog/unreleased. More info here

QA

@JuancaG05 JuancaG05 self-assigned this Dec 13, 2023
@JuancaG05 JuancaG05 linked an issue Dec 13, 2023 that may be closed by this pull request
10 tasks
@JuancaG05
Copy link
Collaborator Author

When using the MDM parameter, if logs are already enabled, we need to disable and re-enable them again for this option to work properly. This is because we cannot perform an active listening to changes in MDM parameters, we just read them when necessary, and in this case, we decide if we show sensitive information or not (by reading the MDM parameter) every time we enable logs.

@michaelstingl
Copy link
Contributor

This is because we cannot perform an active listening to changes in MDM parameters, we just read them when necessary, and in this case, we decide if we show sensitive information or not (by reading the MDM parameter) every time we enable logs.

Okay for me, that MDM config changes require a restart of the app. iOS app for example triggers a notification to tell the user to close and re-open the app.

Copy link
Contributor

@Aitorbp Aitorbp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, I leave you some doubts here:

Copy link
Contributor

@Aitorbp Aitorbp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jesmrec
Copy link
Collaborator

jesmrec commented Dec 14, 2023

QA checks

  • Check that authentication method is separated from [redacted] in Authorization header
  • Check redact_auth_header_logs = true does not reveal secrets in logs
  • Check redact_auth_header_logs = false reveals secrets in logs
  • Check both values using MDM build (need disable and enable feature)

OK from my side

@Aitorbp Aitorbp force-pushed the feature/reveal_redacted_info_logs branch from 0aa1512 to 6d5584c Compare December 14, 2023 13:41
@JuancaG05 JuancaG05 merged commit c3c6fd5 into master Dec 15, 2023
5 checks passed
@JuancaG05 JuancaG05 deleted the feature/reveal_redacted_info_logs branch December 15, 2023 09:17
Aitorbp pushed a commit that referenced this pull request Feb 5, 2024
[FEATURE REQUEST] Don't redact auth type HTTP request logging
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE REQUEST] Don't redact auth type HTTP request logging
4 participants