Skip to content

Commit 4a8df47

Browse files
AlexTugarevAlexTugarev
committed
[gitlab] scopes refinement
Signed-off-by: Alex Tugarev <alex@gitpod.io>
1 parent 55564b6 commit 4a8df47

20 files changed

+114
-134
lines changed

‎components/dashboard/src/components/access-control/access-control.tsx

+17-38
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export class AccessControl extends React.Component<AccessControlProps, AccessCon
8787
}
8888
}
8989
}
90-
90+
9191
protected async redirectToLogin() {
9292
window.location.href = new GitpodHostUrl(window.location.toString()).with({
9393
pathname: '/login/',
@@ -127,7 +127,7 @@ export class AccessControl extends React.Component<AccessControlProps, AccessCon
127127
return undefined;
128128
}
129129
const scopes = authProvider.scopes || [];
130-
const updatedScopes = updatedScopesString.split(',').filter(s => !!s && scopes.some(scope => scope === s));
130+
const updatedScopes = updatedScopesString.split(',').filter(s => !!s && scopes.all.some(scope => scope === s));
131131
if (updatedScopes.length > 0 && hosts.some(h => h === updatedHost)) {
132132
return { updatedHost, updatedScopes };
133133
}
@@ -154,7 +154,7 @@ export class AccessControl extends React.Component<AccessControlProps, AccessCon
154154
const authProvider = authProviders.find(p => p.host === host)!;
155155
let newScopes = new Set<string>(value.split(','));
156156
let oldScopes = new Set<string>(scopes.get(host) || []);
157-
const merged = new Set<string>((authProvider.scopes || []).filter(scope => oldScopes.has(scope) || newScopes.has(scope)));
157+
const merged = new Set<string>((authProvider.scopes.all || []).filter(scope => oldScopes.has(scope) || newScopes.has(scope)));
158158
scopes.set(host, merged);
159159
}
160160
});
@@ -289,12 +289,11 @@ export class AccessControl extends React.Component<AccessControlProps, AccessCon
289289
this.setState({ newScopes });
290290
}
291291

292-
protected renderScopeTooltip(scope: string) {
293-
const text = this.getTooltipForScope(scope);
294-
if (!text) {
292+
protected renderScopeTooltip(description: string | undefined) {
293+
if (!description) {
295294
return undefined;
296295
}
297-
return (<Tooltip title={text} placement="right" interactive style={{ maxWidth: 200, padding: '12px' }}>
296+
return (<Tooltip title={description} placement="right" interactive style={{ maxWidth: 200, padding: '12px' }}>
298297
<span>
299298
<InfoIcon fontSize="small" color="disabled" style={{ verticalAlign: 'middle' }} />
300299
</span>
@@ -355,9 +354,9 @@ export class AccessControl extends React.Component<AccessControlProps, AccessCon
355354

356355
<Grid item style={{ flexGrow: 2 }}>
357356
<CardContent style={{ display: 'block', float: 'left', textAlign: 'left', paddingBottom: '8px' }}>
358-
{(provider.scopes || []).map((scope, index) => {
359-
const disabled = provider.requirements && provider.requirements.default.includes(scope);
360-
const defaultChecked = newScopes.has(scope) || (provider.requirements && provider.requirements.default.includes(scope));
357+
{(provider.scopes.all || []).map((scope, index) => {
358+
const disabled = provider.scopes.default.includes(scope);
359+
const defaultChecked = newScopes.has(scope) || (provider.scopes.default.includes(scope));
361360
const color = newScopes.has(scope) && !oldScopes.has(scope) ? 'secondary' : 'primary';
362361
return (<p key={host + "-scope-" + index} style={{ display: 'table', marginTop: 0 }}>
363362
<label style={{ display: 'flex', alignItems: 'center' }}>
@@ -368,7 +367,7 @@ export class AccessControl extends React.Component<AccessControlProps, AccessCon
368367
defaultChecked={defaultChecked}
369368
/>
370369
{this.getLabelForScope(scope)}
371-
{this.renderScopeTooltip(scope)}
370+
{this.renderScopeTooltip(provider.scopes.descriptions[scope])}
372371
</label>
373372
</p>);
374373
})}
@@ -391,7 +390,7 @@ export class AccessControl extends React.Component<AccessControlProps, AccessCon
391390
this.renderUpdateButton(dirty, () => this.updateToken(provider.host, Array.from(newScopes)), 'Update') :
392391
this.renderUpdateButton(true, () => this.updateToken(provider.host,
393392
// for authorization we set the required (if any) plus the new scopes
394-
[...(provider.requirements && provider.requirements.default || []), ...Array.from(newScopes)]), 'Connect'))
393+
[...(provider.scopes.default || []), ...Array.from(newScopes)]), 'Connect'))
395394
}
396395
</CardActions>
397396
</Grid>
@@ -414,11 +413,13 @@ export class AccessControl extends React.Component<AccessControlProps, AccessCon
414413
case "read:org": return "read organizations";
415414
case "workflow": return "update workflows";
416415
// GitLab
417-
case "read_user": return "read user";
418-
case "api": return "allow api calls";
419-
case "read_repository": return "repository access";
416+
case "read_user": return "read user info";
417+
case "api": return "full API access";
418+
case "read_api": return "read API access";
419+
case "read_repository": return "read repositories";
420+
case "write_repository": return "write repository";
420421
// Bitbucket
421-
case "account": return "read account";
422+
case "account": return "read account info";
422423
case "repository": return "read repositories";
423424
case "repository:write": return "write repositories";
424425
case "pullrequest": return "read pull requests";
@@ -428,28 +429,6 @@ export class AccessControl extends React.Component<AccessControlProps, AccessCon
428429
}
429430
}
430431

431-
protected getTooltipForScope(scope: string): string | undefined {
432-
switch (scope) {
433-
case "user:email": return "Read-only access to your email addresses";
434-
case "public_repo": return "Write access to code in public repositories and organizations";
435-
case "repo": return "Read/write access to code in private repositories and organizations";
436-
case "read:org": return "Read-only access to organizations (used to suggest organizations when forking a repository)";
437-
case "workflow": return "Allow updating GitHub Actions workflow files";
438-
// GitLab
439-
case "read_user": return "Read-only access to your email addresses";
440-
case "api": return "Allow making API calls (used to set up a webhook when enabling prebuilds for a repository)";
441-
case "read_repository": return "Read/write access to your repositories";
442-
// Bitbucket
443-
case "account": return "Read-only access to your account information";
444-
case "repository": return "Read-only access to your repositories (note: Bitbucket doesn't support revoking scopes)";
445-
case "repository:write": return "Read/write access to your repositories (note: Bitbucket doesn't support revoking scopes)";
446-
case "pullrequest": return "Read access to pull requests and ability to collaborate via comments, tasks, and approvals (note: Bitbucket doesn't support revoking scopes)";
447-
case "pullrequest:write": return "Allow creating, merging and declining pull requests (note: Bitbucket doesn't support revoking scopes)";
448-
case "webhook": return "Allow installing webhooks (used when enabling prebuilds for a repository, note: Bitbucket doesn't support revoking scopes)";
449-
default: return undefined;
450-
}
451-
}
452-
453432
protected updateToken(provider: string, scopes: string[]) {
454433
if (scopes.length === 0) {
455434
return;

‎components/gitpod-protocol/src/protocol.ts

+7-6
Original file line numberDiff line numberDiff line change
@@ -1025,12 +1025,13 @@ export interface AuthProviderInfo {
10251025
readonly description?: string;
10261026

10271027
readonly settingsUrl?: string;
1028-
readonly scopes?: string[];
1029-
readonly requirements?: {
1030-
readonly default: string[];
1031-
readonly publicRepo: string[];
1032-
readonly privateRepo: string[];
1033-
}
1028+
readonly scopes: AuthProviderScopes;
1029+
}
1030+
1031+
export interface AuthProviderScopes {
1032+
readonly default: string[];
1033+
readonly all: string[];
1034+
readonly descriptions: { [key: string]: string };
10341035
}
10351036

10361037
export interface AuthProviderEntry {

‎components/server/src/auth/authenticator.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,8 @@ export class Authenticator {
201201
await AuthFlow.attach(req.session, { host, returnTo, overrideScopes: override });
202202
let wantedScopes = scopes.split(',');
203203
if (wantedScopes.length === 0) {
204-
if (authProvider.info.requirements) {
205-
wantedScopes = authProvider.info.requirements.default;
204+
if (authProvider.info.scopes) {
205+
wantedScopes = authProvider.info.scopes.default;
206206
}
207207
}
208208
// compute merged scopes
@@ -211,8 +211,8 @@ export class Authenticator {
211211
wantedScopes = this.mergeScopes(currentScopes, wantedScopes);
212212
// in case user signed in with another identity, we need to ensure the merged scopes contain
213213
// all default needed to for proper authentication
214-
if (currentScopes.length === 0 && authProvider.info.requirements) {
215-
wantedScopes = this.mergeScopes(authProvider.info.requirements.default, wantedScopes);
214+
if (currentScopes.length === 0 && authProvider.info.scopes) {
215+
wantedScopes = this.mergeScopes(authProvider.info.scopes.default, wantedScopes);
216216
}
217217
}
218218
// authorize Gitpod

‎components/server/src/auth/generic-auth-provider.ts

+7-8
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ export class GenericAuthProvider implements AuthProvider {
8080
}
8181

8282
protected defaultInfo(): AuthProviderInfo {
83-
const scopes = this.oauthScopes;
83+
const { oauthScopes } = this;
8484
const { id, type, icon, host, ownerId, verified, hiddenOnDashboard, disallowLogin, description, loginContextMatcher } = this.config;
8585
return {
8686
authProviderId: id,
@@ -93,13 +93,12 @@ export class GenericAuthProvider implements AuthProvider {
9393
loginContextMatcher,
9494
disallowLogin,
9595
description,
96-
scopes,
96+
scopes: {
97+
all: oauthScopes,
98+
default: oauthScopes,
99+
descriptions: {}
100+
},
97101
settingsUrl: this.oauthConfig.settingsUrl,
98-
requirements: {
99-
default: scopes,
100-
publicRepo: scopes,
101-
privateRepo: scopes
102-
}
103102
}
104103
}
105104

@@ -119,7 +118,7 @@ export class GenericAuthProvider implements AuthProvider {
119118
protected get oauthConfig() {
120119
return this.config.oauth!;
121120
}
122-
protected get oauthScopes() {
121+
protected get oauthScopes(): string[] {
123122
if (!this.oauthConfig.scope) {
124123
return [];
125124
}

‎components/server/src/bitbucket/bitbucket-auth-provider.ts

+4-9
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,7 @@ export class BitbucketAuthProvider extends GenericAuthProvider {
1919
get info(): AuthProviderInfo {
2020
return {
2121
...this.defaultInfo(),
22-
scopes: BitbucketOAuthScopes.ALL,
23-
requirements: {
24-
default: BitbucketOAuthScopes.Requirements.DEFAULT,
25-
publicRepo: BitbucketOAuthScopes.Requirements.DEFAULT,
26-
privateRepo: BitbucketOAuthScopes.Requirements.DEFAULT,
27-
},
22+
scopes: BitbucketOAuthScopes.definitions,
2823
}
2924
}
3025

@@ -39,7 +34,7 @@ export class BitbucketAuthProvider extends GenericAuthProvider {
3934
authorizationUrl: oauth.authorizationUrl || `https://${this.config.host}/site/oauth2/authorize`,
4035
tokenUrl: oauth.tokenUrl || `https://${this.config.host}/site/oauth2/access_token`,
4136
settingsUrl: oauth.settingsUrl || `https://${this.config.host}/account/settings/app-authorizations/`,
42-
scope: BitbucketOAuthScopes.ALL.join(scopeSeparator),
37+
scope: BitbucketOAuthScopes.definitions.default.join(scopeSeparator),
4338
scopeSeparator
4439
};
4540
}
@@ -49,7 +44,7 @@ export class BitbucketAuthProvider extends GenericAuthProvider {
4944
}
5045

5146
authorize(req: express.Request, res: express.Response, next: express.NextFunction, scope?: string[]): void {
52-
super.authorize(req, res, next, scope ? scope : BitbucketOAuthScopes.Requirements.DEFAULT);
47+
super.authorize(req, res, next, scope ? scope : BitbucketOAuthScopes.definitions.default);
5348
}
5449

5550
protected get baseURL() {
@@ -105,7 +100,7 @@ export class BitbucketAuthProvider extends GenericAuthProvider {
105100
set.add('pullrequest');
106101
}
107102
for (const item of set.values()) {
108-
if (!(BitbucketOAuthScopes.Requirements.DEFAULT.includes(item))) {
103+
if (!(BitbucketOAuthScopes.definitions.default.includes(item))) {
109104
set.delete(item);
110105
}
111106
}

‎components/server/src/bitbucket/bitbucket-oauth-scopes.ts

+14-8
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
* See License-AGPL.txt in the project root for license information.
55
*/
66

7+
import { AuthProviderScopes } from "@gitpod/gitpod-protocol";
8+
79
// https://confluence.atlassian.com/bitbucket/oauth-on-bitbucket-cloud-238027431.html
810

911
export namespace BitbucketOAuthScopes {
@@ -20,12 +22,16 @@ export namespace BitbucketOAuthScopes {
2022
/** Create, list web hooks */
2123
export const WEBHOOK = "webhook"
2224

23-
export const ALL = [ACCOUNT_READ, REPOSITORY_READ, REPOSITORY_WRITE, PULL_REQUEST_READ, PULL_REQUEST_WRITE, WEBHOOK];
24-
25-
export const Requirements = {
26-
/**
27-
* Minimal required permission.
28-
*/
29-
DEFAULT: ALL
30-
}
25+
export const definitions: AuthProviderScopes = {
26+
default: [ACCOUNT_READ, REPOSITORY_READ, REPOSITORY_WRITE, PULL_REQUEST_READ, PULL_REQUEST_WRITE, WEBHOOK],
27+
all: [ACCOUNT_READ, REPOSITORY_READ, REPOSITORY_WRITE, PULL_REQUEST_READ, PULL_REQUEST_WRITE, WEBHOOK],
28+
descriptions: {
29+
ACCOUNT_READ: "Grants read-only access to your account information.",
30+
REPOSITORY_READ: "Read-only access to your repositories (note: Bitbucket doesn't support revoking scopes)",
31+
REPOSITORY_WRITE: "Grants read/write access to your repositories (note: Bitbucket doesn't support revoking scopes)",
32+
PULL_REQUEST_READ: "Grants read-only access to pull requests and ability to collaborate via comments, tasks, and approvals (note: Bitbucket doesn't support revoking scopes)",
33+
PULL_REQUEST_WRITE: "Allows creating, merging and declining pull requests (note: Bitbucket doesn't support revoking scopes)",
34+
WEBHOOK: "Allows installing webhooks (used when enabling prebuilds for a repository, note: Bitbucket doesn't support revoking scopes)",
35+
}
36+
};
3137
}

‎components/server/src/bitbucket/bitbucket-token-handler.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export class BitbucketTokenHelper {
3636
// no token
3737
}
3838
if (requiredScopes.length === 0) {
39-
requiredScopes = BitbucketOAuthScopes.Requirements.DEFAULT
39+
requiredScopes = BitbucketOAuthScopes.definitions.default
4040
}
4141
throw UnauthorizedError.create(host, requiredScopes, "missing-identity");
4242
}

‎components/server/src/dev/authenticator-dev.ts

+1
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ class DummyAuthProvider implements AuthProvider {
3535
host: "github.com",
3636
icon: this.config.icon,
3737
description: this.config.description,
38+
scopes: ({} as any)
3839
}
3940
}
4041
readonly host = "github.com";

‎components/server/src/github/api.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ export class GitHubRestApi {
124124
if (typeof userOrToken === 'string') {
125125
token = userOrToken;
126126
} else {
127-
const githubToken = await this.tokenHelper.getTokenWithScopes(userOrToken, GitHubScope.Requirements.DEFAULT);
127+
const githubToken = await this.tokenHelper.getTokenWithScopes(userOrToken, GitHubScope.definitions.default);
128128
token = githubToken.value;
129129
}
130130
const api = new GitHub(this.getGitHubOptions(token));

‎components/server/src/github/github-auth-provider.ts

+3-8
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,7 @@ export class GitHubAuthProvider extends GenericAuthProvider {
2121
get info(): AuthProviderInfo {
2222
return {
2323
...this.defaultInfo(),
24-
scopes: GitHubScope.All,
25-
requirements: {
26-
default: GitHubScope.Requirements.DEFAULT,
27-
publicRepo: GitHubScope.Requirements.PUBLIC_REPO,
28-
privateRepo: GitHubScope.Requirements.PRIVATE_REPO,
29-
},
24+
scopes: GitHubScope.definitions,
3025
}
3126
}
3227

@@ -41,13 +36,13 @@ export class GitHubAuthProvider extends GenericAuthProvider {
4136
...oauth!,
4237
authorizationUrl: oauth.authorizationUrl || defaultUrls.authorizationUrl,
4338
tokenUrl: oauth.tokenUrl || defaultUrls.tokenUrl,
44-
scope: GitHubScope.All.join(scopeSeparator),
39+
scope: GitHubScope.definitions.default.join(scopeSeparator),
4540
scopeSeparator
4641
};
4742
}
4843

4944
authorize(req: express.Request, res: express.Response, next: express.NextFunction, scope?: string[]): void {
50-
super.authorize(req, res, next, scope ? scope : GitHubScope.Requirements.DEFAULT);
45+
super.authorize(req, res, next, scope ? scope : GitHubScope.definitions.default);
5146
}
5247

5348
/**

‎components/server/src/github/github-context-parser.ts

+2-3
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,8 @@ export class GithubContextParser extends AbstractContextParser implements IConte
5757
// most likely the token needs to be updated after revoking by user.
5858
throw UnauthorizedError.create(this.config.host, scopes, "http-unauthorized");
5959
}
60-
// todo@alex: this is very unlikely. is coercing it into a valid case helpful?
61-
// here, GH API responded with a 401 code, and we are missing a token. OTOH, a missing token would not lead to a request.
62-
throw UnauthorizedError.create(this.config.host, GitHubScope.Requirements.PUBLIC_REPO, "missing-identity");
60+
// this isn't expected to be reached, as a missing token is supposed to be handled already.
61+
throw UnauthorizedError.create(this.config.host, GitHubScope.definitions.default, "missing-identity");
6362
}
6463
throw error;
6564
} finally {

‎components/server/src/github/github-token-helper.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export class GitHubTokenHelper {
3535
// no token
3636
}
3737
if (requiredScopes.length === 0) {
38-
requiredScopes = GitHubScope.Requirements.DEFAULT
38+
requiredScopes = GitHubScope.definitions.default
3939
}
4040
throw UnauthorizedError.create(host, requiredScopes, "missing-identity");
4141
}

‎components/server/src/github/scopes.ts

+12-11
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* See License-AGPL.txt in the project root for license information.
55
*/
66

7+
import { AuthProviderScopes } from "@gitpod/gitpod-protocol";
78

89
export namespace GitHubScope {
910
export const EMAIL = "user:email";
@@ -12,15 +13,15 @@ export namespace GitHubScope {
1213
export const ORGS = "read:org";
1314
export const WORKFLOW = "workflow";
1415

15-
export const All = [EMAIL, PUBLIC, PRIVATE, ORGS, WORKFLOW];
16-
export const Requirements = {
17-
/**
18-
* Minimal required permission.
19-
* GitHub's API is not restricted any further.
20-
*/
21-
DEFAULT: [EMAIL],
22-
23-
PUBLIC_REPO: [PUBLIC],
24-
PRIVATE_REPO: [PRIVATE],
25-
}
16+
export const definitions: AuthProviderScopes = {
17+
default: [EMAIL],
18+
all: [EMAIL, PUBLIC, PRIVATE, ORGS, WORKFLOW],
19+
descriptions: {
20+
EMAIL: "Grants read-only access to the authenticated user's profile.",
21+
PUBLIC: "Grants read-write access to public repositories using Git-over-HTTP.",
22+
PRIVATE: "Grants read-write access to private repositories using Git-over-HTTP.",
23+
ORGS: "Grants read-only access to organization membership.",
24+
WORKFLOW: "Grants the ability to add and update GitHub Actions workflow files.",
25+
}
26+
};
2627
}

‎components/server/src/gitlab/api.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export class GitLabApi {
2727
if (typeof userOrToken === 'string') {
2828
oauthToken = userOrToken;
2929
} else {
30-
const gitlabToken = await this.tokenHelper.getTokenWithScopes(userOrToken, GitLabScope.Requirements.DEFAULT);
30+
const gitlabToken = await this.tokenHelper.getTokenWithScopes(userOrToken, GitLabScope.definitions.default);
3131
oauthToken = gitlabToken.value;
3232
}
3333
return GitLab.create({

0 commit comments

Comments
 (0)