Skip to content

Commit

Permalink
Disallow issuing temp credential with another temp credential. (#111)
Browse files Browse the repository at this point in the history
We should not allow users to "wrap" a temporary credential with another
temporary credential. This is fine from a security standpoint because
such a credential will fail later while fetching API secrets. However,
allowing this is a confusing and perhaps hard to debug behavior, so we
prefer to fail fast.
  • Loading branch information
kevin1 authored Oct 29, 2024
1 parent 49380fc commit bc912d3
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 0 deletions.
21 changes: 21 additions & 0 deletions packages/proxy/utils/tempCredentials.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
makeTempCredentialsJwt,
verifyTempCredentials,
verifyJwtOnly,
makeTempCredentials,
} from "./tempCredentials";
import {
sign as jwtSign,
Expand Down Expand Up @@ -118,6 +119,26 @@ test("makeTempCredentialsJwt no secret reuse", () => {
expect(payload1.jti).not.toStrictEqual(payload2.jti);
});

test("makeTempCredentials no wrapping other temp credential", async () => {
const result = makeTempCredentialsJwt({
request: { model: "model", ttl_seconds: 100 },
authToken: "auth token",
orgName: "my org name",
});

// Use the previous temp credential JWT to issue another one.
await expect(
makeTempCredentials({
authToken: result.jwt,
body: {
model: null,
ttl_seconds: 200,
},
cachePut: async () => undefined,
}),
).rejects.toThrow();
});

test("verifyJwtOnly basic", () => {
const credentialCacheValue: TempCredentialsCacheValue = {
authToken: "auth token",
Expand Down
10 changes: 10 additions & 0 deletions packages/proxy/utils/tempCredentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,16 @@ export async function makeTempCredentials({
ttl_seconds?: number,
) => Promise<void>;
}) {
if (isTempCredential(authToken)) {
// Disallow issuing a temp credential that wraps another temp credential.
// This is fine from a security standpoint because such a credential will
// fail later while fetching API secrets. However, allowing this is a
// confusing and perhaps hard to debug behavior, so we prefer to fail fast.
throw new Error(
"Temporary credential cannot be used to issue another temp credential.",
);
}

const body = credentialsRequestSchema.safeParse(rawBody);
if (!body.success) {
throw new Error(body.error.message);
Expand Down

0 comments on commit bc912d3

Please sign in to comment.