-
Notifications
You must be signed in to change notification settings - Fork 252
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
Tests: passkey: Add a ssh key as a passkey mapping #7207
Conversation
madhuriupadhye
commented
Feb 22, 2024
•
edited
Loading
edited
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a general question about the test and possibly a suggestion for additional tests:
Isn't the main point that SSH/SU should work with non-passkey mapping data stored in the provider when passkey feature is enabled?
Should we also cover the following scenarios?
- Mixed passkey and non-passkey data in the provider attribute
- SSH with passkey data in the provider attribute? (does this work properly now?)
- SSH with password if passkey data is in the provider attribute
- SSH with public key if ssh key is stored in the provider attribute (this one might be better in another test suite)?
8dbc367
to
e9fb6a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
e9fb6a8
to
63a476e
Compare
63a476e
to
8df7a22
Compare
b20c33d
to
f3788e1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 codeQL warning and a couple of suggestions for test case naming changes that might clarify test purposes.
eb70793
to
7d2e48c
Compare
Here, added two test cases: 1. Check log message when we add ssh key as passkey mapping. 2. Check log message when we add ssh key with passkey token. Signed-off-by: Madhuri Upadhye <[email protected]>
7d2e48c
to
d2316d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack, thank you.