Skip to content

Commit dcf0404

Browse files
jeanp413jeanp413
committed
Address feedback
1 parent 64a3604 commit dcf0404

File tree

3 files changed

+120
-37
lines changed

3 files changed

+120
-37
lines changed

‎components/server/src/user/gitpod-token-service.spec.db.ts

Lines changed: 76 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ describe("GitpodTokenService", async () => {
2828
let stranger: User;
2929
let org: Organization;
3030

31+
const resourceAccessGuard = {
32+
async canAccess() {
33+
return true;
34+
},
35+
};
36+
3137
beforeEach(async () => {
3238
container = createTestContainer();
3339
Experiments.configureTestingClient({
@@ -66,69 +72,110 @@ describe("GitpodTokenService", async () => {
6672
});
6773

6874
it("should generate a new gitpod token", async () => {
69-
const resp1 = await gs.getGitpodTokens(member.id, member.id);
75+
const resp1 = await gs.getGitpodTokens(member.id, member.id, resourceAccessGuard);
7076
expect(resp1.length).to.equal(0);
7177

72-
await gs.generateNewGitpodToken(member.id, member.id, { name: "token1", type: GitpodTokenType.API_AUTH_TOKEN });
78+
await gs.generateNewGitpodToken(
79+
member.id,
80+
member.id,
81+
{ name: "token1", type: GitpodTokenType.API_AUTH_TOKEN },
82+
resourceAccessGuard,
83+
);
7384

74-
const resp2 = await gs.getGitpodTokens(member.id, member.id);
85+
const resp2 = await gs.getGitpodTokens(member.id, member.id, resourceAccessGuard);
7586
expect(resp2.length).to.equal(1);
7687

77-
await expectError(ErrorCodes.NOT_FOUND, gs.getGitpodTokens(stranger.id, member.id));
88+
await expectError(ErrorCodes.NOT_FOUND, gs.getGitpodTokens(stranger.id, member.id, resourceAccessGuard));
7889
await expectError(
7990
ErrorCodes.NOT_FOUND,
80-
gs.generateNewGitpodToken(stranger.id, member.id, { name: "token2", type: GitpodTokenType.API_AUTH_TOKEN }),
91+
gs.generateNewGitpodToken(
92+
stranger.id,
93+
member.id,
94+
{ name: "token2", type: GitpodTokenType.API_AUTH_TOKEN },
95+
resourceAccessGuard,
96+
),
8197
);
8298
});
8399

84100
it("should list gitpod tokens", async () => {
85-
await gs.generateNewGitpodToken(member.id, member.id, { name: "token1", type: GitpodTokenType.API_AUTH_TOKEN });
86-
await gs.generateNewGitpodToken(member.id, member.id, { name: "token2", type: GitpodTokenType.API_AUTH_TOKEN });
101+
await gs.generateNewGitpodToken(
102+
member.id,
103+
member.id,
104+
{ name: "token1", type: GitpodTokenType.API_AUTH_TOKEN },
105+
resourceAccessGuard,
106+
);
107+
await gs.generateNewGitpodToken(
108+
member.id,
109+
member.id,
110+
{ name: "token2", type: GitpodTokenType.API_AUTH_TOKEN },
111+
resourceAccessGuard,
112+
);
87113

88-
const tokens = await gs.getGitpodTokens(member.id, member.id);
114+
const tokens = await gs.getGitpodTokens(member.id, member.id, resourceAccessGuard);
89115
expect(tokens.length).to.equal(2);
90116
expect(tokens.some((t) => t.name === "token1")).to.be.true;
91117
expect(tokens.some((t) => t.name === "token2")).to.be.true;
92118

93-
await expectError(ErrorCodes.NOT_FOUND, gs.getGitpodTokens(stranger.id, member.id));
119+
await expectError(ErrorCodes.NOT_FOUND, gs.getGitpodTokens(stranger.id, member.id, resourceAccessGuard));
94120
});
95121

96122
it("should return gitpod token scopes", async () => {
97-
await gs.generateNewGitpodToken(member.id, member.id, {
98-
name: "token1",
99-
type: GitpodTokenType.API_AUTH_TOKEN,
100-
scopes: ["user:email", "read:user"],
101-
});
123+
await gs.generateNewGitpodToken(
124+
member.id,
125+
member.id,
126+
{
127+
name: "token1",
128+
type: GitpodTokenType.API_AUTH_TOKEN,
129+
scopes: ["user:email", "read:user"],
130+
},
131+
resourceAccessGuard,
132+
);
102133

103-
const tokens = await gs.getGitpodTokens(member.id, member.id);
134+
const tokens = await gs.getGitpodTokens(member.id, member.id, resourceAccessGuard);
104135
expect(tokens.length).to.equal(1);
105136

106-
const scopes = await gs.getGitpodTokenScopes(member.id, member.id, tokens[0].tokenHash);
137+
const scopes = await gs.getGitpodTokenScopes(member.id, member.id, tokens[0].tokenHash, resourceAccessGuard);
107138
expect(scopes.length).to.equal(2);
108139
expect(scopes.some((s) => s === "user:email")).to.be.true;
109140
expect(scopes.some((s) => s === "read:user")).to.be.true;
110141

111-
await expectError(ErrorCodes.NOT_FOUND, gs.getGitpodTokenScopes(stranger.id, member.id, tokens[0].tokenHash));
142+
await expectError(
143+
ErrorCodes.NOT_FOUND,
144+
gs.getGitpodTokenScopes(stranger.id, member.id, tokens[0].tokenHash, resourceAccessGuard),
145+
);
112146
});
113147

114148
it("should delete gitpod tokens", async () => {
115-
await gs.generateNewGitpodToken(member.id, member.id, {
116-
name: "token1",
117-
type: GitpodTokenType.API_AUTH_TOKEN,
118-
});
119-
await gs.generateNewGitpodToken(member.id, member.id, {
120-
name: "token2",
121-
type: GitpodTokenType.API_AUTH_TOKEN,
122-
});
149+
await gs.generateNewGitpodToken(
150+
member.id,
151+
member.id,
152+
{
153+
name: "token1",
154+
type: GitpodTokenType.API_AUTH_TOKEN,
155+
},
156+
resourceAccessGuard,
157+
);
158+
await gs.generateNewGitpodToken(
159+
member.id,
160+
member.id,
161+
{
162+
name: "token2",
163+
type: GitpodTokenType.API_AUTH_TOKEN,
164+
},
165+
resourceAccessGuard,
166+
);
123167

124-
const tokens = await gs.getGitpodTokens(member.id, member.id);
168+
const tokens = await gs.getGitpodTokens(member.id, member.id, resourceAccessGuard);
125169
expect(tokens.length).to.equal(2);
126170

127-
await gs.deleteGitpodToken(member.id, member.id, tokens[0].tokenHash);
171+
await gs.deleteGitpodToken(member.id, member.id, tokens[0].tokenHash, resourceAccessGuard);
128172

129-
const tokens2 = await gs.getGitpodTokens(member.id, member.id);
173+
const tokens2 = await gs.getGitpodTokens(member.id, member.id, resourceAccessGuard);
130174
expect(tokens2.length).to.equal(1);
131175

132-
await expectError(ErrorCodes.NOT_FOUND, gs.deleteGitpodToken(stranger.id, member.id, tokens[1].tokenHash));
176+
await expectError(
177+
ErrorCodes.NOT_FOUND,
178+
gs.deleteGitpodToken(stranger.id, member.id, tokens[1].tokenHash, resourceAccessGuard),
179+
);
133180
});
134181
});

‎components/server/src/user/gitpod-token-service.ts

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import { GitpodToken, GitpodTokenType } from "@gitpod/gitpod-protocol";
1010
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
1111
import { inject, injectable } from "inversify";
1212
import { Authorizer } from "../authorization/authorizer";
13+
import { GuardedResource, ResourceAccessGuard, ResourceAccessOp } from "../auth/resource-access";
14+
import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
1315

1416
@injectable()
1517
export class GitpodTokenService {
@@ -18,16 +20,24 @@ export class GitpodTokenService {
1820
@inject(Authorizer) private readonly auth: Authorizer,
1921
) {}
2022

21-
async getGitpodTokens(requestorId: string, userId: string): Promise<GitpodToken[]> {
23+
async getGitpodTokens(
24+
requestorId: string,
25+
userId: string,
26+
resouceGuardAccess: ResourceAccessGuard,
27+
): Promise<GitpodToken[]> {
2228
await this.auth.checkPermissionOnUser(requestorId, "read_tokens", userId);
2329
const res = (await this.userDB.findAllGitpodTokensOfUser(userId)).filter((v) => !v.deleted);
30+
await Promise.all(
31+
res.map((tkn) => this.guardAccess(resouceGuardAccess, { kind: "gitpodToken", subject: tkn }, "get")),
32+
);
2433
return res;
2534
}
2635

2736
async generateNewGitpodToken(
2837
requestorId: string,
2938
userId: string,
3039
options: { name?: string; type: GitpodTokenType; scopes?: string[] },
40+
resouceGuardAccess: ResourceAccessGuard,
3141
): Promise<string> {
3242
await this.auth.checkPermissionOnUser(requestorId, "write_tokens", userId);
3343
const token = crypto.randomBytes(30).toString("hex");
@@ -40,11 +50,17 @@ export class GitpodTokenService {
4050
scopes: options.scopes || [],
4151
created: new Date().toISOString(),
4252
};
53+
await this.guardAccess(resouceGuardAccess, { kind: "gitpodToken", subject: dbToken }, "create");
4354
await this.userDB.storeGitpodToken(dbToken);
4455
return token;
4556
}
4657

47-
async getGitpodTokenScopes(requestorId: string, userId: string, tokenHash: string): Promise<string[]> {
58+
async getGitpodTokenScopes(
59+
requestorId: string,
60+
userId: string,
61+
tokenHash: string,
62+
resouceGuardAccess: ResourceAccessGuard,
63+
): Promise<string[]> {
4864
await this.auth.checkPermissionOnUser(requestorId, "read_tokens", userId);
4965
let token: GitpodToken | undefined;
5066
try {
@@ -56,16 +72,36 @@ export class GitpodTokenService {
5672
if (!token || token.deleted) {
5773
return [];
5874
}
75+
await this.guardAccess(resouceGuardAccess, { kind: "gitpodToken", subject: token }, "get");
5976
return token.scopes;
6077
}
6178

62-
async deleteGitpodToken(requestorId: string, userId: string, tokenHash: string): Promise<void> {
79+
async deleteGitpodToken(
80+
requestorId: string,
81+
userId: string,
82+
tokenHash: string,
83+
resouceGuardAccess: ResourceAccessGuard,
84+
): Promise<void> {
6385
await this.auth.checkPermissionOnUser(requestorId, "write_tokens", userId);
64-
const existingTokens = await this.getGitpodTokens(requestorId, userId);
86+
const existingTokens = await this.getGitpodTokens(requestorId, userId, resouceGuardAccess);
6587
const tkn = existingTokens.find((token) => token.tokenHash === tokenHash);
6688
if (!tkn) {
6789
throw new Error(`User ${requestorId} tries to delete a token ${tokenHash} that does not exist.`);
6890
}
91+
await this.guardAccess(resouceGuardAccess, { kind: "gitpodToken", subject: tkn }, "delete");
6992
await this.userDB.deleteGitpodToken(tokenHash);
7093
}
94+
95+
private async guardAccess(
96+
resouceGuardAccess: ResourceAccessGuard,
97+
resource: GuardedResource,
98+
op: ResourceAccessOp,
99+
) {
100+
if (!(await resouceGuardAccess.canAccess(resource, op))) {
101+
throw new ApplicationError(
102+
ErrorCodes.PERMISSION_DENIED,
103+
`operation not permitted: missing ${op} permission on ${resource.kind}`,
104+
);
105+
}
106+
}
71107
}

‎components/server/src/workspace/gitpod-server-impl.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2958,7 +2958,7 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
29582958

29592959
public async getGitpodTokens(ctx: TraceContext): Promise<GitpodToken[]> {
29602960
const user = await this.checkAndBlockUser("getGitpodTokens");
2961-
return this.gitpodTokenService.getGitpodTokens(user.id, user.id);
2961+
return this.gitpodTokenService.getGitpodTokens(user.id, user.id, this.resourceAccessGuard);
29622962
}
29632963

29642964
public async generateNewGitpodToken(
@@ -2968,21 +2968,21 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
29682968
traceAPIParams(ctx, { options });
29692969

29702970
const user = await this.checkAndBlockUser("generateNewGitpodToken");
2971-
return this.gitpodTokenService.generateNewGitpodToken(user.id, user.id, options);
2971+
return this.gitpodTokenService.generateNewGitpodToken(user.id, user.id, options, this.resourceAccessGuard);
29722972
}
29732973

29742974
public async getGitpodTokenScopes(ctx: TraceContext, tokenHash: string): Promise<string[]> {
29752975
traceAPIParams(ctx, {}); // do not trace tokenHash
29762976

29772977
const user = await this.checkAndBlockUser("getGitpodTokenScopes");
2978-
return this.gitpodTokenService.getGitpodTokenScopes(user.id, user.id, tokenHash);
2978+
return this.gitpodTokenService.getGitpodTokenScopes(user.id, user.id, tokenHash, this.resourceAccessGuard);
29792979
}
29802980

29812981
public async deleteGitpodToken(ctx: TraceContext, tokenHash: string): Promise<void> {
29822982
traceAPIParams(ctx, {}); // do not trace tokenHash
29832983

29842984
const user = await this.checkAndBlockUser("deleteGitpodToken");
2985-
return this.gitpodTokenService.deleteGitpodToken(user.id, user.id, tokenHash);
2985+
return this.gitpodTokenService.deleteGitpodToken(user.id, user.id, tokenHash, this.resourceAccessGuard);
29862986
}
29872987

29882988
async guessGitTokenScopes(ctx: TraceContext, params: GuessGitTokenScopesParams): Promise<GuessedGitTokenScopes> {

0 commit comments

Comments
 (0)