-
Notifications
You must be signed in to change notification settings - Fork 0
Ability to log in with HarvardKey from discovery page. #16
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
base: main
Are you sure you want to change the base?
Conversation
| Kaixa.click('.btn-primary'); | ||
| boolean isHarvardKey = obj.has('isHarvardKey') && obj.getBoolean('isHarvardKey'); | ||
| boolean needs2FA = isHarvardKey && obj.has('needs2FA') && obj.getBoolean('needs2FA'); | ||
|
|
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.
In GlobalCredentials, the user will be something like:
public static String admin ='''
{
'username': '[email protected]',
'password': '',
'isHarvardKey': true,
'needs2FA': true,
}
I'm not attached to specific names so please suggest better ones.
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.
Love this. The only thing I'm thinking is that the name isHarvardKey is confusing to me because isn't Harvard Guest part of HarvardKey? Maybe clearer would be to flip this and just call it isHarvardGuest? Or something like isOfficialHarvardKey or something
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.
Flipping it to isHarvardGuest is good with me.
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.
So now:
public static String admin ='''
{
'username': '[email protected]',
'password': 'blah',
'isHarvardGuest': true,
}
a2105b6 to
040c686
Compare
040c686 to
880ae59
Compare
src/Kaixa.groovy
Outdated
| * page for the user to choose between HarvardKey and Harvard Guest credentials | ||
| */ | ||
| public static void handleHarvardKey(name) { | ||
| public static void handleHarvardKey(String name, boolean expectDiscoveryPage = true) { |
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.
Why not detect whether there is a discovery page?
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.
Yeah... When I implemented this, I was thinking that it's part of the test: if the application is expecting the discovery page and there isn't one, the test would fail. But: we are not testing the Harvard login, right? So I guess it's fine. It certainly will make things simpler.
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.
Done!
src/Kaixa.groovy
Outdated
| Kaixa.waitForAtLeastOneElementPresent([ | ||
| '#idp_1001962798_button', // HarvardGuest | ||
| '#idp_1824601020_button', // HarvardKey | ||
| ]); |
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.
This would be the feature you could use to check if there's a discovery page. This function returns which one appeared. You could wait for either the discovery page or the login page and handle it appropriately
gabeabrams
left a comment
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.
Looks pretty good! Just a couple thoughts, but nothing blocking
No description provided.