Skip to content

Commit 1c01704

Browse files
aeiseleSpace Team
authored andcommitted
TW-81291: GitHub App: check against token scope for commit status permission, display a more clear success message
1 parent 0d1196a commit 1c01704

File tree

5 files changed

+88
-7
lines changed

5 files changed

+88
-7
lines changed

commit-status-publisher-server/src/main/java/jetbrains/buildServer/commitPublisher/github/api/impl/GitHubApiFactoryImpl.java

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@
2323
import jetbrains.buildServer.commitPublisher.github.api.GitHubApiFactory;
2424
import jetbrains.buildServer.commitPublisher.github.api.impl.data.RepoInfo;
2525
import jetbrains.buildServer.http.SimpleCredentials;
26-
import jetbrains.buildServer.serverSide.oauth.OAuthToken;
27-
import jetbrains.buildServer.serverSide.oauth.OAuthTokensStorage;
26+
import jetbrains.buildServer.serverSide.ProjectManager;
27+
import jetbrains.buildServer.serverSide.oauth.*;
28+
import jetbrains.buildServer.users.SUser;
29+
import jetbrains.buildServer.vcs.SVcsRoot;
2830
import org.jetbrains.annotations.NotNull;
2931

3032
/**
@@ -37,10 +39,20 @@ public class GitHubApiFactoryImpl implements GitHubApiFactory {
3739
@NotNull
3840
protected final OAuthTokensStorage myOAuthTokensStorage;
3941

42+
@NotNull
43+
protected final OAuthConnectionsManager myConnectionsManager;
44+
45+
@NotNull
46+
protected final ProjectManager myProjectManager;
47+
4048
public GitHubApiFactoryImpl(@NotNull final HttpClientWrapper wrapper,
41-
@NotNull OAuthTokensStorage oAuthTokensStorage) {
49+
@NotNull OAuthTokensStorage oAuthTokensStorage,
50+
@NotNull OAuthConnectionsManager connectionsManager,
51+
@NotNull ProjectManager projectManager) {
4252
myWrapper = wrapper;
4353
myOAuthTokensStorage = oAuthTokensStorage;
54+
myConnectionsManager = connectionsManager;
55+
myProjectManager = projectManager;
4456
}
4557

4658

@@ -93,6 +105,45 @@ protected void checkPermissions(@NotNull Repository repo, @NotNull RepoInfo repo
93105
if (null == repoInfo.name || null == repoInfo.permissions) {
94106
throw new PublisherException(String.format("Repository \"%s\" is inaccessible", repo.url()));
95107
}
108+
109+
final SVcsRoot vcsRoot = myProjectManager.findVcsRootByExternalId(vcsRootId);
110+
if (vcsRoot == null) {
111+
throw new PublisherException("Unable to find VCS root by external id " + vcsRootId);
112+
}
113+
114+
final OAuthToken gitHubOAuthToken = myOAuthTokensStorage.getRefreshableToken(vcsRootId, tokenId, false);
115+
if (gitHubOAuthToken == null) {
116+
throw new PublisherException("Failed to retrieve configured token from storage (tokenId: " + tokenId + ")");
117+
}
118+
119+
if (SUser.UKNOWN_USER_ID == gitHubOAuthToken.getTeamCityUserId()) { // GitHub App Connection
120+
final OAuthConnectionDescriptor connection = getConnection(vcsRoot, tokenId);
121+
final TokenIntent intent = new TokenIntent(TokenIntentType.PUBLISH_STATUS, vcsRoot.getProperty("url"));
122+
if (!connection.getOauthProvider().isSuitableToken(gitHubOAuthToken, intent)) {
123+
throw new PublisherException(String.format("The stored token doesn't have push access to the repository \"%s\"", repo.url()));
124+
}
125+
126+
} else { // OAuth connection
127+
if (!repoInfo.permissions.push) {
128+
throw new PublisherException(String.format("There is no push access to the repository \"%s\"", repo.url()));
129+
}
130+
}
131+
}
132+
133+
134+
@NotNull
135+
private OAuthConnectionDescriptor getConnection(@NotNull SVcsRoot vcsRoot, @NotNull String tokenId) throws PublisherException {
136+
final TokenFullIdComponents components = OAuthTokensStorage.parseFullTokenId(tokenId);
137+
if (components == null) {
138+
throw new PublisherException("Unable to parse token id " + tokenId);
139+
}
140+
141+
final OAuthConnectionDescriptor connection = myConnectionsManager.findConnectionByTokenStorageId(vcsRoot.getProject(), components.getTokenStorageId());
142+
if (connection == null) {
143+
throw new PublisherException("Unable to find connection by token id " + tokenId);
144+
}
145+
146+
return connection;
96147
}
97148
};
98149
}

commit-status-publisher-server/src/main/resources/buildServerResources/github/githubSettings.jsp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,17 @@
146146
<c:if test="${testConnectionSupported}">
147147
<script>
148148
$j(document).ready(function() {
149-
PublisherFeature.showTestConnection();
149+
PublisherFeature.showTestConnection(() => {
150+
const selectedAuthType = $('${keys.authenticationTypeKey}').value;
151+
if (selectedAuthType === '${keys.authentificationTypeGitHubAppTokenValue}' || selectedAuthType === 'vcsRoot') {
152+
return {
153+
text: `This test confirms that TeamCity can <strong>pull/read</strong> data from the target repository under the provided credentials.<br/></br/>
154+
If builds fail to publish their statuses, check whether the current credentials have corresponding <strong>push/write</strong> permissions.`,
155+
preserveHtml: true
156+
};
157+
}
158+
return "";
159+
});
150160
});
151161
</script>
152162
</c:if>

commit-status-publisher-server/src/main/resources/buildServerResources/publisherFeatureHeader.jsp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,18 @@
7777
7878
onCompleteSave: function (form, responseXML, err) {
7979
BS.XMLResponse.processErrors(responseXML, that, null);
80-
BS.TestConnectionDialog.show(success, success ? (testConnectionSuccessInfo ? testConnectionSuccessInfo : "") : info, null);
80+
let successInfo = "";
81+
let preserveHtml = false;
82+
if (testConnectionSuccessInfo) {
83+
if (testConnectionSuccessInfo instanceof Function) {
84+
const info = testConnectionSuccessInfo.call();
85+
successInfo = info.text;
86+
preserveHtml = info.preserveHtml;
87+
} else {
88+
successInfo = testConnectionSuccessInfo;
89+
}
90+
}
91+
BS.TestConnectionDialog.show(success, success ? successInfo : info, null, preserveHtml);
8192
form.setSaving(false);
8293
form.enable();
8394
}

commit-status-publisher-server/src/main/resources/buildServerResources/stash/stashSettings.jsp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,13 @@
9797
<c:if test="${testConnectionSupported}">
9898
<script>
9999
$j(document).ready(function() {
100-
PublisherFeature.showTestConnection("This ensures that the repository is reachable under the provided credentials.\nIf status publishing still fails, it can be due to insufficient permissions of the corresponding BitBucket Server user.");
100+
PublisherFeature.showTestConnection(() => {
101+
return {
102+
text: `This test confirms that TeamCity can <strong>pull/read</strong> data from the target repository under the provided credentials.<br/></br/>
103+
If builds fail to publish their statuses, check whether the current Bitbucket server user has corresponding <strong>push/write</strong> permissions.`,
104+
preserveHtml: true
105+
};
106+
});
101107
});
102108
</script>
103109
</c:if>

commit-status-publisher-server/src/test/java/jetbrains/buildServer/commitPublisher/github/GitHubPublisherTest.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import jetbrains.buildServer.commitPublisher.github.api.impl.data.RepoInfo;
3333
import jetbrains.buildServer.messages.Status;
3434
import jetbrains.buildServer.serverSide.*;
35+
import jetbrains.buildServer.serverSide.oauth.OAuthConnectionsManager;
3536
import jetbrains.buildServer.serverSide.oauth.OAuthTokensStorage;
3637
import jetbrains.buildServer.util.HTTPRequestBuilder;
3738
import jetbrains.buildServer.util.TestFor;
@@ -200,7 +201,9 @@ protected void setUp() throws Exception {
200201
myBuildType.getProject().addParameter(new SimpleParameter("teamcity.commitStatusPublisher.publishQueuedBuildStatus", "true"));
201202

202203
myChangeStatusUpdater = new ChangeStatusUpdater(new GitHubApiFactoryImpl(new HttpClientWrapperImpl(new HTTPRequestBuilder.ApacheClient43RequestHandler(), () -> null),
203-
myFixture.getSingletonService(OAuthTokensStorage.class)), myFixture.getVcsHistory());
204+
myFixture.getSingletonService(OAuthTokensStorage.class),
205+
myFixture.getSingletonService(OAuthConnectionsManager.class),
206+
myFixture.getProjectManager()), myFixture.getVcsHistory());
204207

205208
myPublisherSettings = new GitHubSettings(myChangeStatusUpdater, new MockPluginDescriptor(), myWebLinks, myProblems,
206209
myOAuthConnectionsManager, myOAuthTokenStorage, myFixture.getSecurityContext(),

0 commit comments

Comments
 (0)