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

Make Hardware Key unit test interactive #32235

Merged
merged 4 commits into from
Sep 27, 2023
Merged

Conversation

Joerger
Copy link
Contributor

@Joerger Joerger commented Sep 20, 2023

This PR updates some Hardware Key unit tests tests to also cover hardware_key_touch. This makes the test interactive as we don't currently have a way to mock PIV touch.

These tests aren't run by CI, but I use them when developing new PIV features like #31743. In the future we can adapt these tests to mock touch (if possible) or replace it with a tool similar to tsh bench to troubleshoot PIV issues.

Copy link
Contributor

@jakule jakule left a comment

Choose a reason for hiding this comment

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

I don't think that having an "interactive test' as a part of our test suite helps testing and I think it will be more confusing when you run the test suite, and something fails without any explanation.
IMO the benefitial would be:

  1. Fully automate the test by simulating the touch (if possible)
  2. create something similar to tsh bench that is part of tsh and allows you to test this feature manually. We could even ask our users to run this test when they have some issues with PIV.

@Joerger Joerger force-pushed the joerger/interactive-piv-tests branch from 54e5ac1 to 4af9782 Compare September 25, 2023 18:39
@Joerger Joerger changed the title Make Hardware Key unit test interactive and add it to testplan Make Hardware Key unit test interactive Sep 25, 2023
@Joerger
Copy link
Contributor Author

Joerger commented Sep 25, 2023

@jakule Fair enough, I've removed it from the test plan and updated the PR description. Right now I just to merge this PR as a prerequisite for #31743 and #31732

@Joerger Joerger requested a review from jakule September 25, 2023 19:11
Copy link
Contributor

@jakule jakule left a comment

Choose a reason for hiding this comment

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

LGTM 🙈

@Joerger Joerger force-pushed the joerger/interactive-piv-tests branch from 4af9782 to a12afe7 Compare September 27, 2023 17:43
@Joerger
Copy link
Contributor Author

Joerger commented Sep 27, 2023

@rudream should be a quick review when you get a chance, thanks!

@Joerger Joerger added this pull request to the merge queue Sep 27, 2023
Merged via the queue into master with commit f87c46a Sep 27, 2023
27 checks passed
@Joerger Joerger deleted the joerger/interactive-piv-tests branch September 27, 2023 19:14
Joerger added a commit that referenced this pull request Oct 12, 2023
* Make yubikey unit test interactive and add to test plan.

* Move yubikey hardware signer method tests to interactive yubikey test.

* Remove hardware key interactive unit test from testplan
@Joerger Joerger mentioned this pull request Oct 12, 2023
github-merge-queue bot pushed a commit that referenced this pull request Oct 12, 2023
* Refactor PIV metadata certificate logic (#32250)

* Change PIV metadata cert to be self signed by an ephemeral key so it can be signed without touch/pin.

* Refactor touch prompt logic.

* Prompt user before overwriting non-teleport certificates instead of just returning an error.

* Update RFD and Docs.

* Address CR.

* Export some methods and variables for use in tests.

* Address comments.

* Make Hardware Key unit test interactive (#32235)

* Make yubikey unit test interactive and add to test plan.

* Move yubikey hardware signer method tests to interactive yubikey test.

* Remove hardware key interactive unit test from testplan

* Client check for YubiKey private key touch/pin policies (#32264)

* Preemptively check yubikey private key touch/pin policies on the client side.

* Replace hardware key signer functions with private key methods.

* Refactor overwrite/generate key logic.

* Have hardware_key_touch count towards MFA verification instead of replacing the normal MFA verification flow. (#31663)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants