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

Reduce initial authentication setup complexity #46807

Closed
wants to merge 2 commits into
base: 2018.3
from

Conversation

Projects
None yet
4 participants
@ezh
Contributor

ezh commented Apr 1, 2018

What does this PR do?

Add more verbose debug messages for auth subsystem.

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

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 Apr 1, 2018

@cachedout

I am OK with this but would also like @thatch45 to look at this.

@cachedout cachedout requested a review from thatch45 Apr 2, 2018

@thatch45

None of these specifications expose auth data to an attacker

@rallytime

This comment has been minimized.

Contributor

rallytime commented Apr 3, 2018

@ezh The follow tests failures look related here:

  • integration.shell.test_key.KeyTest.test_list_acc_wrong_eauth
  • integration.shell.test_runner.RunTest.test_salt_run_with_wrong_eauth

https://jenkins.saltstack.com/job/PR/job/salt-pr-linode-cent7-py3/3704/

Can you take a look?

@ezh

This comment has been minimized.

Contributor

ezh commented Apr 3, 2018

Yes. I'll check it.

@cachedout

This comment has been minimized.

Contributor

cachedout commented Apr 9, 2018

@ezh Any updates?

@ezh

This comment has been minimized.

Contributor

ezh commented Apr 10, 2018

I could check it on holiday. Sorry.

@ezh

This comment has been minimized.

Contributor

ezh commented Apr 14, 2018

Go Go Jenkins!

@ezh

This comment has been minimized.

Contributor

ezh commented Apr 14, 2018

@cachedout @rallytime It looks like eauth tests fixed.

  • integration.shell.test_key.KeyTest.test_list_acc_wrong_eauth
  • integration.shell.test_runner.RunTest.test_salt_run_with_wrong_eauth
@rallytime

This comment has been minimized.

Contributor

rallytime commented Apr 17, 2018

@rallytime

This comment has been minimized.

Contributor

rallytime commented May 15, 2018

I am going to close this PR since we haven't heard back here for a while. We'd still love to keep the conversation going. We would be happy to open this again when you're ready or you can open a new PR. Whichever is easiest for you. 🙂 Thank you!

@rallytime rallytime closed this May 15, 2018

@ezh ezh referenced this pull request Jun 18, 2018

Merged

2018.3 auth #48179

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