8000 [JENKINS-55557] Add support for state parameter · ojacques/github-oauth-plugin@8d51832 · GitHub
[go: up one dir, main page]
Skip to content

Commit 8d51832

Browse files
jkakavasjkakavas
committed
[JENKINS-55557] Add support for state parameter
which mitigates CSRF attacks when using the authorization code grant flow of oAuth2
1 parent 57ea2d4 commit 8d51832

File tree

2 files changed

+37
-3
lines changed

2 files changed

+37
-3
lines changed

‎src/main/java/org/jenkinsci/plugins/GithubSecurityRealm.java

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,9 @@ of this software and associated documentation files (the "Software"), to deal
8888
import java.io.IOException;
8989
import java.net.InetSocketAddress;
9090
import java.net.Proxy;
91+
import java.security.SecureRandom;
9192
import java.util.Arrays;
93+
import java.util.Base64;
9294
import java.util.HashSet;
9395
import java.util.Set;
9496
import java.util.logging.Logger;
@@ -337,6 +339,7 @@ public String getOauthScopes() {
337339

338340
public HttpResponse doCommenceLogin(StaplerRequest request, @QueryParameter String from, @Header("Referer") final String referer)
339341
throws IOException {
342+
final String state = getSecureRandomString();
340343
String redirectOnFinish;
341344
if (from != null && Util.isSafeToRedirectTo(from)) {
342345
redirectOnFinish = from;
@@ -347,18 +350,19 @@ public HttpResponse doCommenceLogin(StaplerRequest request, @QueryParameter Stri
347350
}
348351

349352
request.getSession().setAttribute(REFERER_ATTRIBUTE, redirectOnFinish);
353+
request.getSession().setAttribute(STATE_ATTRIBUTE, state);
350354

351355
Set<String> scopes = new HashSet<>();
352356
for (GitHubOAuthScope s : getJenkins().getExtensionList(GitHubOAuthScope.class)) {
353357
scopes.addAll(s.getScopesToRequest());
354358
}
355359
String suffix="";
356360
if (!scopes.isEmpty()) {
357-
suffix = "&scope="+Util.join(scopes,",");
361+
suffix = "&scope="+Util.join(scopes,",")+"&state="+state;
358362
} else {
359363
// We need repo scope in order to access private repos
360364
// See https://developer.github.com/v3/oauth/#scopes
361-
suffix = "&scope=" + oauthScopes;
365+
suffix = "&scope=" + oauthScopes +"&state="+state;
362366
}
363367

364368
return new HttpRedirect(githubWebUri + "/login/oauth/authorize?client_id="
@@ -372,13 +376,27 @@ public HttpResponse doCommenceLogin(StaplerRequest request, @QueryParameter Stri
372376
public HttpResponse doFinishLogin(StaplerRequest request)
373377
throws IOException {
374378
String code = request.getParameter("code");
379+
String state = request.getParameter(STATE_ATTRIBUTE);
375380
String referer = (String)request.getSession().getAttribute(REFERER_ATTRIBUTE);
381+
String expectedState = (String)request.getSession().getAttribute(STATE_ATTRIBUTE);
376382

377383
if (code == null || code.trim().length() == 0) {
378384
Log.info("doFinishLogin: missing code.");
379385
return HttpResponses.redirectToContextRoot();
380386
}
381387

388+
if (state == null){
389+
Log.info("doFinishLogin: missing state parameter from Github response.");
390+
return HttpResponses.redirectToContextRoot();
391+
} else if (expectedState == null){
392+
Log.info("doFinishLogin: missing state parameter from user's session.");
393+
return HttpResponses.redirectToContextRoot();
394+
} else if (!state.equals(expectedState)){
395+
Log.info("state parameter value ["+state+"] does not match the expected one ["+expectedState+"]");
396+
return HttpResponses.redirectToContextRoot();
397+
}
398+
399+
382400
String accessToken = getAccessToken(code);
383401

384402
if (accessToken != null && accessToken.trim().length() > 0) {
@@ -460,6 +478,16 @@ private String getAccessToken(@Nonnull String code) throws IOException {
460478
return null;
461479
}
462480

481+
/**
482+
* Generates a random 20 byte String that conforms to the <a href="https://tools.ietf.org/html/rfc6749#section-10.10">specification</a>
483+
* requirements
484+
* @return a string that can be used as a state parameter
485+
*/
486+
private String getSecureRandomString() {
487+
final byte[] bytes = new byte[20];
488+
SECURE_RANDOM.nextBytes(bytes);
489+
return BASE64_ENCODER.encodeToString(bytes);
490+
}
463491
/**
464492
* Returns the proxy to be used when connecting to the given URI.
465493
*/
@@ -789,6 +817,11 @@ static Jenkins getJenkins() {
789817
private static final Logger LOGGER = Logger.getLogger(GithubSecurityRealm.class.getName());
790818

791819
private static final String REFERER_ATTRIBUTE = GithubSecurityRealm.class.getName()+".referer";
820+
private static final String STATE_ATTRIBUTE = "state";
821+
822+
private static final SecureRandom SECURE_RANDOM = new SecureRandom();
823+
824+
private static final Base64.Encoder BASE64_ENCODER = Base64.getUrlEncoder().withoutPadding();
792825

793826
/**
794827
* Asks for the password.

‎src/test/java/org/jenkinsci/plugins/GithubAccessTokenPropertyTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,8 @@ private void onUserTeams(HttpServletRequest req, HttpServletResponse resp) throw
245245

246246
private void onLoginOAuthAuthorize(HttpServletRequest req, HttpServletResponse resp) throws IOException {
247247
String code = "test";
248-
resp.sendRedirect(jenkinsRule.getURL() + "securityRealm/finishLogin?code=" + code);
248+
String state = req.getParameter("state");
249+
resp.sendRedirect(jenkinsRule.getURL() + "securityRealm/finishLogin?code=" + code+"&state="+state);
249250
}
250251

251252
private void onLoginOAuthAccessToken(HttpServletRequest req, HttpServletResponse resp) throws IOException {

0 commit comments

Comments
 (0)
0