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

2018.3 auth #48179

Merged
merged 3 commits into from Jun 19, 2018

Conversation

Projects
None yet
4 participants
@ezh
Contributor

ezh commented Jun 18, 2018

What does this PR do?
Add more verbose debug messages for auth subsystem.
It is the updated version of #46807

What issues does this PR fix or reference?
#46806

Previous Behavior
The useless "Authorization failure occurred" message

New Behavior
New log messages at debug log level.

Tests written?
No (Updated)

Commits signed with GPG?
No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@ezh ezh requested a review from saltstack/team-core as a code owner Jun 18, 2018

@salt-jenkins salt-jenkins requested a review from saltstack/team-suse Jun 18, 2018

@ezh

This comment has been minimized.

Contributor

ezh commented Jun 18, 2018

@cachedout @rallytime I see no errors related to this PR.

@rallytime

This comment has been minimized.

Contributor

rallytime commented Jun 18, 2018

Hi @ezh - Yeah, that one failure is not related here. However, the 2018.3.2 branch is closed to fixes presently. Can you move this to the 2018.3 branch?

@isbm

isbm approved these changes Jun 18, 2018

@ezh ezh changed the base branch from 2018.3.2 to 2018.3 Jun 18, 2018

@gtmanfred

I remember hearing at some point that this was intentionally left vauge to make it more difficult to compromise the auth system if someone gets access to the logs.

@gtmanfred gtmanfred requested review from cachedout and thatch45 Jun 18, 2018

@ezh

This comment has been minimized.

Contributor

ezh commented Jun 19, 2018

Rebased on 2018.3.

There is no sensitive data like passwords in log messages at all.
There is only user name at debug log level if error occured in configuration, but debug log level is not for production use.

@gtmanfred

This comment has been minimized.

Contributor

gtmanfred commented Jun 19, 2018

Yes, I am glad they are debug logs, I will +1 this I just want to make sure it is OK with @cachedout

@rallytime

This comment has been minimized.

Contributor

rallytime commented Jun 19, 2018

This is a replacement of #46807, which I closed a couple weeks ago since we hadn't heard back yet on some test fixes.

@thatch45 and @cachedout signed off on the original there, so I am OK with this going in.

Thanks @gtmanfred and @ezh!

@rallytime rallytime merged commit 2a8e1c6 into saltstack:2018.3 Jun 19, 2018

6 of 10 checks passed

continuous-integration/jenkins/pr-merge This commit cannot be built
Details
jenkins/PR/salt-pr-linode-ubuntu16-py3 Pull Requests » Salt PR - Linode Ubuntu16.04 - PY3 #10788 — ABORTED
Details
jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #19871 — ABORTED
Details
default Build finished.
Details
WIP ready for review
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #26020 — SUCCESS
Details
jenkins/PR/salt-pr-docs-n Pull Requests » Salt PR - Docs #18075 — SUCCESS
Details
jenkins/PR/salt-pr-linode-cent7-py3 Pull Requests » Salt PR - Linode CentOS 7 - PY3 #5817 — SUCCESS
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt PR - Linode Ubuntu14.04 #23747 — SUCCESS
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #22710 — SUCCESS
Details
@gtmanfred

This comment has been minimized.

Contributor

gtmanfred commented Jun 19, 2018

perfect! thanks!

Daniel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment