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

Add Functionality of OTP to support user 2fa #603

Merged
merged 28 commits into from Nov 13, 2019
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
62bf691
Add Functionality of OTP to github
madhephaestus Nov 9, 2019
331084f
comments
madhephaestus Nov 9, 2019
8567c5e
Update src/main/java/org/kohsuke/github/GitHub.java
madhephaestus Nov 10, 2019
cf55d67
Added test to comply with CONTRIBUTING.md
madhephaestus Nov 10, 2019
efc4d0f
Merge branch 'master' of git@github.com:madhephaestus/github-api.git
madhephaestus Nov 10, 2019
c1f999d
Disabling test that can't run on CI
madhephaestus Nov 10, 2019
d8cc1f8
Properly disabling the test
madhephaestus Nov 10, 2019
91d67fb
Tuning the test to pass
madhephaestus Nov 10, 2019
a3fe95d
clear junk data
Nov 12, 2019
5da2caa
Adding the assume only valid when live
Nov 12, 2019
df6c951
Unit test passing with OTP data
Nov 12, 2019
ad0a24a
Make sure test is only run with stored mock data
Nov 12, 2019
8cb7210
set the timestamp in the token name to match the test data
Nov 12, 2019
2c50255
renaming
madhephaestus Nov 12, 2019
2f118a9
Adding suggested exception from @PauloMigAlmeida
madhephaestus Nov 12, 2019
6ae586a
Creating a receiver lambda for the PTM functionality
madhephaestus Nov 12, 2019
421c0a1
Implement @PauloMigAlmeida suggested lambda style OTP workflow
madhephaestus Nov 12, 2019
5ae0627
re-run the data capture with the overloaded function
madhephaestus Nov 12, 2019
ff78885
added the filter to the test to only run with mock data
madhephaestus Nov 12, 2019
3dcaf5a
Unit test passing with Mock data
madhephaestus Nov 12, 2019
4a78690
Remove old function prototype and replace it with the lambda model.
madhephaestus Nov 12, 2019
1fe820d
instructions for reproducing the test data
madhephaestus Nov 12, 2019
9f49854
Comments for the path of OTP exception
madhephaestus Nov 13, 2019
3d7d780
only run test with moc date
madhephaestus Nov 13, 2019
d32299f
add testing of scopes in assigned token
madhephaestus Nov 13, 2019
e138c6f
Switch interface to existing Supplier class
madhephaestus Nov 13, 2019
feabd07
adding javadoc
madhephaestus Nov 13, 2019
e325bf7
fixed for the sensitive javadoc parser
madhephaestus Nov 13, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 20 additions & 1 deletion src/main/java/org/kohsuke/github/GitHub.java
Expand Up @@ -671,7 +671,26 @@ public GHAuthorization createToken(Collection<String> scope, String note, String

return requester.method("POST").to("/authorizations", GHAuthorization.class).wrap(this);
}

/**
* Creates a new authorization using an OTP.
*
* Start by running createToken, if exception is thrown, prompt for OTP from user
madhephaestus marked this conversation as resolved.
Show resolved Hide resolved
*
* Once OTP is received, call this token request
*
* The token created can be then used for {@link GitHub#connectUsingOAuth(String)} in the future.
*
* @see <a href="http://developer.github.com/v3/oauth/#create-a-new-authorization">Documentation</a>
*/
public GHAuthorization createTokenOtp(Collection<String> scope, String note, String noteUrl, String OTP) throws IOException{
Requester requester = new Requester(this)
.with("scopes", scope)
.with("note", note)
.with("note_url", noteUrl);
// Add the OTP from the user
requester.setHeader("x-github-otp", OTP);
return requester.method("POST").to("/authorizations", GHAuthorization.class).wrap(this);
}
/**
* @see <a href="https://developer.github.com/v3/oauth_authorizations/#get-or-create-an-authorization-for-a-specific-app">docs</a>
*/
Expand Down
52 changes: 52 additions & 0 deletions src/test/java/org/kohsuke/github/Github2faTest.java
@@ -0,0 +1,52 @@
package org.kohsuke.github;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.util.Arrays;
import java.util.Date;
import java.util.List;

import org.junit.Test;
public class Github2faTest extends AbstractGitHubWireMockTest {
public String twoFactorAuthCodePrompt() {
System.out.print("Github 2 factor temp key: ");
// create a scanner so we can read the command-line input
BufferedReader buf = new BufferedReader(new InputStreamReader(System.in));
// TODO Auto-generated method stub
try {
return buf.readLine().trim();
} catch (IOException e) {
return null;
}
}
@Test
public void test2faToken() {
DateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd_HH-mm-ss");
String timestamp = dateFormat.format(new Date());
List<String> asList = Arrays.asList("repo", "gist", "write:packages", "read:packages", "delete:packages",
"user", "delete_repo");
String string = "Test2faTokenCreate-" + timestamp;// use time stamp to ensure the token creations do not collide with older tokens
GHAuthorization token=null;
try {
token = gitHub.createToken(asList, string, "this is a test token created by a unit test");
}catch (IOException ex) {
// under 2fa mode this exception is expected, and is necessary
// as the exception is called, GitHub will generate and send an OTP to the users SMS
// we will prompt at the command line for the users 2fa code
try {
//token = gitHub.createTokenOtp(asList, string, "", twoFactorAuthCodePrompt());// prompt at command line for 2fa OTP code
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wrote this test but can't actually run it because the test framework does not have user interactions. How should i test something that is dependant on user interaction, server time sensitive date, then running the test?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. Yeah, it's a little harder than I first thought.

Here's my suggestion:

  • Add checks that right information is in the GHAuthorization that came back from the server.
  • Add a thread sleep of 30 seconds after your call to createTokenOtp().
  • Add actions and checks that verify things are as they should as though the user responded to the SMS
  • Run the test with takeSnapshot with yourself as the user.
  • Respond to the SMS while the test is waiting.
  • Let the test finish successfully
  • Comment out the thread sleep with info explaining this is where a person would get an SMS message and respond
  • Add assumeFalse("Test only valid when not proxying", mockGitHub.isUseProxy()); to the top of your method.
  • Clean up the permissions your account and checkin all the files.

This will help people understand what this method is for and will ensure if someone goes to change it they have some idea of what needs to be done.

Cool?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, cool, so the takeSnapshot will record the user interaction. I would then bake in the OTP code during the capture that I get, into the unit test, to match the header data. That way it can be run in mock. Is that right?

} catch (Exception e) {
e.printStackTrace();
fail();
}
}
assert token!=null;

String p = token.getToken();
bitwiseman marked this conversation as resolved.
Show resolved Hide resolved

assert p!=null;
}
}