Skip to content

Commit af64adf

Browse files
google-labs-jules[bot]joehannohe427
authored
Combine auth_disable_user and auth_set_claims into auth_update_user (#9166)
* Deleted `src/mcp/tools/auth/disable_user.ts`, `src/mcp/tools/auth/disable_user.spec.ts`, `src/mcp/tools/auth/set_claims.ts`, and `src/mcp/tools/auth/set_claims.spec.ts`. This indicates that my refactoring did not break any existing functionality. I have created the new spec file at `src/mcp/tools/auth/update_user.spec.ts` with a basic test structure. I have added a comprehensive suite of tests to `src/mcp/tools/auth/update_user.spec.ts` to cover all the functionality of the new tool. * Cleaning up unit tests * Fixing tests * PR suggestions * PR fixes * Few changes * Name update * Name update --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: Joe Hanley <[email protected]> Co-authored-by: Alexander Nohe <[email protected]>
1 parent 431e674 commit af64adf

File tree

9 files changed

+206
-219
lines changed

9 files changed

+206
-219
lines changed

src/gcp/auth.spec.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ describe("auth", () => {
189189
})
190190
.reply(200, {});
191191

192-
const result = await auth.disableUser(PROJECT_ID, "test-uid", true);
192+
const result = await auth.toggleUserEnablement(PROJECT_ID, "test-uid", true);
193193

194194
expect(result).to.be.true;
195195
expect(nock.isDone()).to.be.true;
@@ -204,7 +204,9 @@ describe("auth", () => {
204204
})
205205
.reply(404, { error: { message: "Not Found" } });
206206

207-
await expect(auth.disableUser(PROJECT_ID, "test-uid", true)).to.be.rejectedWith("Not Found");
207+
await expect(auth.toggleUserEnablement(PROJECT_ID, "test-uid", true)).to.be.rejectedWith(
208+
"Not Found",
209+
);
208210
expect(nock.isDone()).to.be.true;
209211
});
210212
});

src/gcp/auth.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ export async function listUsers(project: string, limit: number): Promise<UserInf
200200
* @param disabled sets whether the user is marked as disabled (true) or enabled (false).
201201
* @return the call succeeded (true).
202202
*/
203-
export async function disableUser(
203+
export async function toggleUserEnablement(
204204
project: string,
205205
uid: string,
206206
disabled: boolean,

src/mcp/tools/auth/disable_user.spec.ts

Lines changed: 0 additions & 63 deletions
This file was deleted.

src/mcp/tools/auth/disable_user.ts

Lines changed: 0 additions & 31 deletions
This file was deleted.

src/mcp/tools/auth/index.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { ServerTool } from "../../tool";
2+
import { update_user } from "./update_user";
23
import { get_users } from "./get_users";
3-
import { disable_user } from "./disable_user";
4-
import { set_claim } from "./set_claims";
54
import { set_sms_region_policy } from "./set_sms_region_policy";
65

7-
export const authTools: ServerTool[] = [get_users, disable_user, set_claim, set_sms_region_policy];
6+
export const authTools: ServerTool[] = [get_users, update_user, set_sms_region_policy];

src/mcp/tools/auth/set_claims.spec.ts

Lines changed: 0 additions & 72 deletions
This file was deleted.

src/mcp/tools/auth/set_claims.ts

Lines changed: 0 additions & 47 deletions
This file was deleted.
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
import { expect } from "chai";
2+
import * as sinon from "sinon";
3+
import { update_user } from "./update_user";
4+
import * as auth from "../../../gcp/auth";
5+
import { McpContext } from "../../types";
6+
import * as util from "../../util";
7+
8+
describe("update_user tool", () => {
9+
const projectId = "test-project";
10+
let setCustomClaimsStub: sinon.SinonStub;
11+
let toggleuserEnablementStub: sinon.SinonStub;
12+
let mcpErrorStub: sinon.SinonStub;
13+
14+
beforeEach(() => {
15+
setCustomClaimsStub = sinon.stub(auth, "setCustomClaim");
16+
toggleuserEnablementStub = sinon.stub(auth, "toggleUserEnablement");
17+
mcpErrorStub = sinon.stub(util, "mcpError");
18+
});
19+
20+
afterEach(() => {
21+
sinon.restore();
22+
});
23+
24+
it("should disable a user", async () => {
25+
toggleuserEnablementStub.resolves(true);
26+
27+
const result = await update_user.fn({ uid: "123", disabled: true }, {
28+
projectId,
29+
} as McpContext);
30+
31+
expect(result).to.deep.equal({
32+
content: [
33+
{
34+
text: "Successfully updated user 123. User disabled.",
35+
type: "text",
36+
},
37+
],
38+
});
39+
expect(toggleuserEnablementStub).to.have.been.calledWith(projectId, "123", true);
40+
expect(setCustomClaimsStub).to.not.have.been.called;
41+
});
42+
43+
it("should enable a user", async () => {
44+
toggleuserEnablementStub.resolves(true);
45+
46+
const result = await update_user.fn({ uid: "123", disabled: false }, {
47+
projectId,
48+
} as McpContext);
49+
50+
expect(result).to.deep.equal({
51+
content: [
52+
{
53+
text: "Successfully updated user 123. User enabled.",
54+
type: "text",
55+
},
56+
],
57+
});
58+
expect(toggleuserEnablementStub).to.have.been.calledWith(projectId, "123", false);
59+
expect(setCustomClaimsStub).to.not.have.been.called;
60+
});
61+
62+
it("should set a custom claim", async () => {
63+
setCustomClaimsStub.resolves({ uid: "123", customClaims: { admin: true } });
64+
65+
const result = await update_user.fn(
66+
{
67+
uid: "123",
68+
claim: { key: "admin", value: true },
69+
},
70+
{
71+
projectId,
72+
} as McpContext,
73+
);
74+
75+
expect(result).to.deep.equal({
76+
content: [
77+
{
78+
text: "Successfully updated user 123. Claim 'admin' set.",
79+
type: "text",
80+
},
81+
],
82+
});
83+
expect(setCustomClaimsStub).to.have.been.calledWith(projectId, "123", { admin: true });
84+
expect(toggleuserEnablementStub).to.not.have.been.called;
85+
});
86+
87+
it("should fail to set a custom claim and disable a user", async () => {
88+
setCustomClaimsStub.resolves({ uid: "123", customClaims: { admin: true } });
89+
toggleuserEnablementStub.resolves(true);
90+
91+
await update_user.fn(
92+
{
93+
uid: "123",
94+
claim: { key: "admin", value: true },
95+
disabled: true,
96+
},
97+
{
98+
projectId,
99+
} as McpContext,
100+
);
101+
102+
expect(mcpErrorStub).to.be.calledWith(
103+
"Can only enable/disable a user or set a claim, not both.",
104+
);
105+
});
106+
});

0 commit comments

Comments
 (0)