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

[master, ipa-4-3] Tests: Fix integration sudo tests setup and checks #27

Closed
wants to merge 1 commit into from

Conversation

mirielka
Copy link
Contributor

@mirielka mirielka commented Aug 26, 2016

Adding 'defaults' sudorule to prevent requesting further user authentication.
Adding checks that if a user should be rejected access, a proper error message
is displayed.

https://fedorahosted.org/freeipa/ticket/6262

@mirielka mirielka changed the title Tests: Fix integration sudo tests setup and checks [master, ipa-4-3] Tests: Fix integration sudo tests setup and checks Aug 26, 2016
@lslebodn
Copy link
Contributor

The sudo test in master (ipatests/test_integration/test_sudo.py) is the same as in 4.3 branch.

And it passed for me on fedora 24 with latest ipa-4.3. I tested with sssd-1.13.4-4 (stable version in fedora) and sssd-1.14.1-1.fc24 (version in updates testing).

If there is bug then it can be either in freeIPA itself or it is a timing issue in sudo_test.py

-1

@mirielka
Copy link
Contributor Author

This PR is not intended to fix a failing test, but fix their execution and error checking. The negative tests for sudorule in master and ipa-4-3 (both with sssd-1.14.1-1.fc24, see jenkins jobs for master [1] and ipa-4-3 [2]) return following message when trying to use sudo as a user that is not allowed to use sudo by current sudorule:

We trust you have received the usual lecture from the local System Administrator. It usually boils down to these three things: 
     #1) Respect the privacy of others.
     #2) Think before you type.
     #3) With great power comes great responsibility.
sudo: no tty present and no askpass program specified

As commented by pbrezina in SSSD ticket [3], this message is caused by missing sudorule "defaults" (correct one is "Sorry, user is not allowed to use sudo" or similar). Addition of this sudorule is the purpose of this PR. Also I added some checks to the contents of the error message so tests do not pass invalid error message as correct just because it has the same return code.

[1] http://jenkins.idm.lab.eng.brq.redhat.com:8080/view/FreeIPA%20Integration%20-%20master%20Fedora%2024/job/freeipa-timed-integration-f24master-sudo-domlevel-0/44/consoleFull
[2] http://jenkins.idm.lab.eng.brq.redhat.com:8080/view/FreeIPA%20Integration%20-%20ipa-4-3%20Fedora%2024/job/freeipa-timed-integration-f24ipa43-sudo-domlevel-0/48/consoleFull
[3] https://fedorahosted.org/sssd/ticket/3152

P.S.: for me, one test in both master and ipa-4-3 fail both before and after application of this change, I would discuss it off-list with you later when I'm investigating it. Of course with Fedora 24 and sssd-1.14.1-1.fc24.

@lslebodn
Copy link
Contributor

This PR is not intended to fix a failing test, but fix their execution and error checking. The negative tests for sudorule in master and ipa-4-3 (both with sssd-1.14.1-1.fc24, see jenkins jobs for master [1] and ipa-4-3 [2]) return following message when trying to use sudo as a user that is not allowed to use sudo by current sudorule:

Then it would be good to check error error messages with enabled and with disabled sudorule "defaults". Both are valid use-cases

@lslebodn
Copy link
Contributor

I forgot to mention that it isn't necessary to it for all cases. maybe separate/new test might cover such test-case.

@mirielka
Copy link
Contributor Author

Yes, I also though about not running e.g. su -c "sudo -l" testuser but su -c "sudo -l -n" testuser - it would report error sudo: a password is required instead of the previously pasted message that doesn't say that much...

Adding 'defaults' sudorule to prevent requesting further user authentication.
Adding checks that if a user should be rejected access, a proper error message
is displayed.

https://fedorahosted.org/freeipa/ticket/6262
@lslebodn
Copy link
Contributor

Test passed on fedora 24 + freeipa-4_4.
ACK

@MartinBasti MartinBasti added the ack Pull Request approved, can be merged label Sep 14, 2016
@MartinBasti MartinBasti added the pushed Pull Request has already been pushed label Sep 14, 2016
@mirielka mirielka deleted the master_int_sudo branch September 14, 2016 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
3 participants