Skip to content

Commit f546cf4

Browse files
bitwisemanbitwiseman
committed
Use only credential providers internally to track credentials
Removes extra fields from GitHubClient.
1 parent 43efa78 commit f546cf4

11 files changed

Lines changed: 204 additions & 192 deletions

‎src/main/java/org/kohsuke/github/CredentialProvider.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,14 @@ public interface CredentialProvider {
3131
*/
3232
String getEncodedAuthorization() throws IOException;
3333

34+
/**
35+
* Binds this credential provider to a github instance.
36+
*
37+
* @param github
38+
*/
39+
default void bind(GitHub github) {
40+
}
41+
3442
/**
3543
* A {@link CredentialProvider} that ensures that no credentials are returned
3644
*/

‎src/main/java/org/kohsuke/github/GitHub.java‎

Lines changed: 38 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -93,32 +93,25 @@ public class GitHub {
9393
* "http://ghe.acme.com/api/v3". Note that GitHub Enterprise has <code>/api/v3</code> in the URL. For
9494
* historical reasons, this parameter still accepts the bare domain name, but that's considered
9595
* deprecated. Password is also considered deprecated as it is no longer required for api usage.
96-
* @param login
97-
* The user ID on GitHub that you are logging in as. Can be omitted if the OAuth token is provided or if
98-
* logging in anonymously. Specifying this would save one API call.
99-
* @param oauthAccessToken
100-
* Secret OAuth token.
101-
* @param password
102-
* User's password. Always used in conjunction with the {@code login} parameter
10396
* @param connector
97+
* a connector
98+
* @param rateLimitHandler
99+
* rateLimitHandler
100+
* @param abuseLimitHandler
101+
* abuseLimitHandler
102+
* @param rateLimitChecker
103+
* rateLimitChecker
104104
* @param credentialProvider
105-
* a credential provider, takes preference over all other auth-related parameters if it's not null
105+
* a credential provider
106106
*/
107107
GitHub(String apiUrl,
108-
String login,
109-
String oauthAccessToken,
110-
String jwtToken,
111-
String password,
112108
HttpConnector connector,
113109
RateLimitHandler rateLimitHandler,
114110
AbuseLimitHandler abuseLimitHandler,
115111
GitHubRateLimitChecker rateLimitChecker,
116112
CredentialProvider credentialProvider) throws IOException {
113+
credentialProvider.bind(this);
117114
this.client = new GitHubHttpUrlConnectionClient(apiUrl,
118-
login,
119-
oauthAccessToken,
120-
jwtToken,
121-
password,
122115
connector,
123116
rateLimitHandler,
124117
abuseLimitHandler,
@@ -129,6 +122,35 @@ public class GitHub {
129122
orgs = new ConcurrentHashMap<>();
130123
}
131124

125+
private GitHub(GitHubClient client) {
126+
this.client = client;
127+
users = new ConcurrentHashMap<>();
128+
orgs = new ConcurrentHashMap<>();
129+
}
130+
131+
static class CredentialRefreshGitHubWrapper extends GitHub {
132+
133+
CredentialProvider credentialProvider;
134+
135+
CredentialRefreshGitHubWrapper(GitHub github, CredentialProvider credentialProvider) {
136+
super(github.client);
137+
this.credentialProvider = credentialProvider;
138+
this.credentialProvider.bind(this);
139+
}
140+
141+
@Nonnull
142+
@Override
143+
Requester createRequest() {
144+
try {
145+
// Override
146+
return super.createRequest().setHeader("Authorization", credentialProvider.getEncodedAuthorization())
147+
.rateLimit(RateLimitTarget.NONE);
148+
} catch (IOException e) {
149+
throw new GHException("Failed to create requester to refresh credentials", e);
150+
}
151+
}
152+
}
153+
132154
/**
133155
* Obtains the credential from "~/.github" or from the System Environment Properties.
134156
*

‎src/main/java/org/kohsuke/github/GitHubBuilder.java‎

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,13 @@ public class GitHubBuilder implements Cloneable {
2424

2525
// default scoped so unit tests can read them.
2626
/* private */ String endpoint = GitHubClient.GITHUB_URL;
27-
/* private */ String user;
28-
/* private */ String password;
29-
/* private */ String oauthToken;
30-
/* private */ String jwtToken;
3127

3228
private HttpConnector connector;
3329

3430
private RateLimitHandler rateLimitHandler = RateLimitHandler.WAIT;
3531
private AbuseLimitHandler abuseLimitHandler = AbuseLimitHandler.WAIT;
3632
private GitHubRateLimitChecker rateLimitChecker = new GitHubRateLimitChecker();
37-
private CredentialProvider credentialProvider = null;
33+
private CredentialProvider credentialProvider = CredentialProvider.ANONYMOUS;
3834

3935
/**
4036
* Instantiates a new Git hub builder.
@@ -62,13 +58,13 @@ static GitHubBuilder fromCredentials() throws IOException {
6258

6359
builder = fromEnvironment();
6460

65-
if (builder.oauthToken != null || builder.user != null || builder.jwtToken != null)
61+
if (builder.credentialProvider != null)
6662
return builder;
6763

6864
try {
6965
builder = fromPropertyFile();
7066

71-
if (builder.oauthToken != null || builder.user != null || builder.jwtToken != null)
67+
if (builder.credentialProvider != null)
7268
return builder;
7369
} catch (FileNotFoundException e) {
7470
// fall through
@@ -248,9 +244,7 @@ public GitHubBuilder withEndpoint(String endpoint) {
248244
* @return the git hub builder
249245
*/
250246
public GitHubBuilder withPassword(String user, String password) {
251-
this.user = user;
252-
this.password = password;
253-
return this;
247+
return withCredentialProvider(ImmutableCredentialProvider.fromLoginAndPassword(user, password));
254248
}
255249

256250
/**
@@ -261,7 +255,7 @@ public GitHubBuilder withPassword(String user, String password) {
261255
* @return the git hub builder
262256
*/
263257
public GitHubBuilder withOAuthToken(String oauthToken) {
264-
return withOAuthToken(oauthToken, null);
258+
return withCredentialProvider(ImmutableCredentialProvider.fromOauthToken(oauthToken));
265259
}
266260

267261
/**
@@ -274,9 +268,7 @@ public GitHubBuilder withOAuthToken(String oauthToken) {
274268
* @return the git hub builder
275269
*/
276270
public GitHubBuilder withOAuthToken(String oauthToken, String user) {
277-
this.oauthToken = oauthToken;
278-
this.user = user;
279-
return this;
271+
return withCredentialProvider(ImmutableCredentialProvider.fromOauthToken(oauthToken, user));
280272
}
281273

282274
public GitHubBuilder withCredentialProvider(final CredentialProvider credentialProvider) {
@@ -293,7 +285,7 @@ public GitHubBuilder withCredentialProvider(final CredentialProvider credentialP
293285
* @see GHAppInstallation#createToken(java.util.Map) GHAppInstallation#createToken(java.util.Map)
294286
*/
295287
public GitHubBuilder withAppInstallationToken(String appInstallationToken) {
296-
return withOAuthToken(appInstallationToken, "");
288+
return withCredentialProvider(ImmutableCredentialProvider.fromAppInstallationToken(appInstallationToken));
297289
}
298290

299291
/**
@@ -304,8 +296,7 @@ public GitHubBuilder withAppInstallationToken(String appInstallationToken) {
304296
* @return the git hub builder
305297
*/
306298
public GitHubBuilder withJwtToken(String jwtToken) {
307-
this.jwtToken = jwtToken;
308-
return this;
299+
return withCredentialProvider(ImmutableCredentialProvider.fromJwtToken(jwtToken));
309300
}
310301

311302
/**
@@ -427,10 +418,6 @@ public GitHubBuilder withProxy(final Proxy p) {
427418
*/
428419
public GitHub build() throws IOException {
429420
return new GitHub(endpoint,
430-
user,
431-
oauthToken,
432-
jwtToken,
433-
password,
434421
connector,
435422
rateLimitHandler,
436423
abuseLimitHandler,

‎src/main/java/org/kohsuke/github/GitHubClient.java‎

Lines changed: 20 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import com.fasterxml.jackson.databind.*;
44
import com.fasterxml.jackson.databind.introspect.VisibilityChecker;
55
import org.apache.commons.io.IOUtils;
6-
import org.jetbrains.annotations.Nullable;
76

87
import java.io.FileNotFoundException;
98
import java.io.IOException;
@@ -72,10 +71,6 @@ abstract class GitHubClient {
7271
}
7372

7473
GitHubClient(String apiUrl,
75-
String login,
76-
String oauthAccessToken,
77-
String jwtToken,
78-
String password,
7974
HttpConnector connector,
8075
RateLimitHandler rateLimitHandler,
8176
AbuseLimitHandler abuseLimitHandler,
@@ -94,42 +89,35 @@ abstract class GitHubClient {
9489
this.connector = connector;
9590

9691
// Prefer credential configuration via provider
97-
if (credentialProvider != null) {
98-
this.credentialProvider = credentialProvider;
99-
} else {
100-
if (oauthAccessToken != null) {
101-
this.credentialProvider = ImmutableCredentialProvider.fromOauthToken(oauthAccessToken);
102-
} else {
103-
if (jwtToken != null) {
104-
this.credentialProvider = ImmutableCredentialProvider.fromJwtToken(jwtToken);
105-
} else if (password != null) {
106-
this.credentialProvider = ImmutableCredentialProvider.fromLoginAndPassword(login, password);
107-
} else {// anonymous access
108-
this.credentialProvider = CredentialProvider.ANONYMOUS;
109-
}
110-
}
111-
}
92+
this.credentialProvider = credentialProvider;
11293

11394
this.rateLimitHandler = rateLimitHandler;
11495
this.abuseLimitHandler = abuseLimitHandler;
11596
this.rateLimitChecker = rateLimitChecker;
11697

117-
this.login = getCurrentUser(login, jwtToken, myselfConsumer);
98+
this.login = getCurrentUser(myselfConsumer);
11899
}
119100

120-
@Nullable
121-
private String getCurrentUser(String login, String jwtToken, Consumer<GHMyself> myselfConsumer) throws IOException {
122-
if (login == null && this.credentialProvider.getEncodedAuthorization() != null && jwtToken == null) {
123-
try {
124-
GHMyself myself = fetch(GHMyself.class, "/user");
125-
if (myselfConsumer != null) {
126-
myselfConsumer.accept(myself);
101+
private String getCurrentUser(Consumer<GHMyself> myselfConsumer) throws IOException {
102+
String login = null;
103+
if (this.credentialProvider instanceof ImmutableCredentialProvider.UserCredentialProvider
104+
&& this.credentialProvider.getEncodedAuthorization() != null) {
105+
106+
ImmutableCredentialProvider.UserCredentialProvider userCredentialProvider = (ImmutableCredentialProvider.UserCredentialProvider) this.credentialProvider;
107+
108+
login = userCredentialProvider.getLogin();
109+
110+
if (login == null) {
111+
try {
112+
GHMyself myself = fetch(GHMyself.class, "/user");
113+
if (myselfConsumer != null) {
114+
myselfConsumer.accept(myself);
115+
}
116+
login = myself.getLogin();
117+
} catch (IOException e) {
118+
return null;
127119
}
128-
return myself.getLogin();
129-
} catch (IOException e) {
130-
return null;
131120
}
132-
133121
}
134122
return login;
135123
}
@@ -394,7 +382,6 @@ public <T> GitHubResponse<T> sendRequest(GitHubRequest request, @CheckForNull Gi
394382
"GitHub API request [" + (login == null ? "anonymous" : login) + "]: "
395383
+ request.method() + " " + request.url().toString());
396384
}
397-
398385
rateLimitChecker.checkRateLimit(this, request);
399386

400387
responseInfo = getResponseInfo(request);

‎src/main/java/org/kohsuke/github/GitHubHttpUrlConnectionClient.java‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,21 +33,13 @@
3333
class GitHubHttpUrlConnectionClient extends GitHubClient {
3434

3535
GitHubHttpUrlConnectionClient(String apiUrl,
36-
String login,
37-
String oauthAccessToken,
38-
String jwtToken,
39-
String password,
4036
HttpConnector connector,
4137
RateLimitHandler rateLimitHandler,
4238
AbuseLimitHandler abuseLimitHandler,
4339
GitHubRateLimitChecker rateLimitChecker,
4440
Consumer<GHMyself> myselfConsumer,
4541
CredentialProvider credentialProvider) throws IOException {
4642
super(apiUrl,
47-
login,
48-
oauthAccessToken,
49-
jwtToken,
50-
password,
5143
connector,
5244
rateLimitHandler,
5345
abuseLimitHandler,
@@ -116,8 +108,11 @@ static HttpURLConnection setupConnection(@Nonnull GitHubClient client, @Nonnull
116108

117109
// if the authentication is needed but no credential is given, try it anyway (so that some calls
118110
// that do work with anonymous access in the reduced form should still work.)
119-
if (client.credentialProvider.getEncodedAuthorization() != null) {
120-
connection.setRequestProperty("Authorization", client.credentialProvider.getEncodedAuthorization());
111+
if (!request.headers().containsKey("Authorization")) {
112+
String authorization = client.credentialProvider.getEncodedAuthorization();
113+
if (authorization != null) {
114+
connection.setRequestProperty("Authorization", client.credentialProvider.getEncodedAuthorization());
115+
}
121116
}
122117

123118
setRequestMethod(request.method(), connection);

0 commit comments

Comments
 (0)