Get user's SSH verified keys only using published info.
Use the GitHub public and unrestricted API for accessing
the user's SSH public keys.
There are a couple of benefits associated to this:
1. Get only the SSH keys that have been verified for
that user and made public to all the world.
2. Avoid requesting the "user" scope during OAuth
which grants unnecessary WRITE access to the user's
profile.
The latter raised concerns on people in doubt of
what Gerrit would have done to their GitHub user's
profile because of the involuntary WRITE access
requested before.
Drawbacks: we cannot get anymore the SSH label associated
to the published keys, as GitHub keeps that info
confidential. This is an acceptable price to pay in
order to lower the concerns on the GitHub OAuth scope.
NOTE: This commit uses a pending pull request on
Kohsuke's GitHub API on master
(see https://github.com/kohsuke/github-api/pull/61)
Change-Id: If53ddbbc90dd8e45de53caaf12a6e0fc69b8ee25
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/AccountController.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/AccountController.java
index 87da1d1..20fb53b 100644
--- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/AccountController.java
+++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/AccountController.java
@@ -23,8 +23,10 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import org.apache.commons.lang.StringUtils;
import org.kohsuke.github.GHKey;
import org.kohsuke.github.GHMyself;
+import org.kohsuke.github.GHVerifiedKey;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -58,20 +60,21 @@
HttpServletRequest req, HttpServletResponse resp, ControllerErrors errors)
throws ServletException, IOException {
GHMyself myself = hubLogin.getMyself();
- List<GHKey> githubKeys = myself.getPublicKeys();
- HashSet<String> gerritKeys = Sets.newHashSet(getSshKeys(user));
+ List<GHVerifiedKey> githubKeys = myself.getPublicVerifiedKeys();
+ HashSet<String> gerritKeys = Sets.newHashSet(getCurrentGerritSshKeys(user));
for (GHKey ghKey : githubKeys) {
String sshKeyCheckedParam = "key_check_" + ghKey.getId();
String sshKeyWithLabel = ghKey.getKey() + " " + ghKey.getTitle();
String checked = req.getParameter(sshKeyCheckedParam);
if (checked != null && checked.equalsIgnoreCase("on")
- && !gerritKeys.contains(sshKeyWithLabel)) {
+ && !gerritKeys.contains(ghKey.getKey())) {
addSshKey(user, sshKeyWithLabel);
+ gerritKeys.add(ghKey.getKey());
}
}
}
- private List<String> getSshKeys(final IdentifiedUser user) throws IOException {
+ private List<String> getCurrentGerritSshKeys(final IdentifiedUser user) throws IOException {
AccountResource res = new AccountResource(user);
try {
List<SshKeyInfo> keysInfo = restGetSshKeys.apply(res);
@@ -79,7 +82,7 @@
@Override
public String apply(SshKeyInfo keyInfo) {
- return keyInfo.sshPublicKey;
+ return StringUtils.substringBeforeLast(keyInfo.sshPublicKey, " ");
}
});
diff --git a/github-plugin/src/main/resources/static/account.html b/github-plugin/src/main/resources/static/account.html
index 47b318d..ed51218 100644
--- a/github-plugin/src/main/resources/static/account.html
+++ b/github-plugin/src/main/resources/static/account.html
@@ -56,7 +56,7 @@
<h5>Import GitHub SSH Public Keys</h5>
<ul>
- #foreach ( $key in $myself.publicKeys )
+ #foreach ( $key in $myself.publicVerifiedKeys )
#if ( $key.key.length() > 45 )
<li> <label for="$key.title" style="font-weight: bold;">$key.title :</label>
#set ( $trailStart = $key.key.length() - 10 )
