Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Credentials Middleware tests #461

Merged
merged 14 commits into from
Dec 5, 2024
7 changes: 3 additions & 4 deletions src/lib/account.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export async function makeAccountRequest({
const baseUrl = process.env.FAUNA_ACCOUNT_URL ?? "https://account.fauna.com";
const paramsString = params ? `?${new URLSearchParams(params)}` : "";
let fullUrl;

try {
fullUrl = new URL(`/api/v1${path}${paramsString}`, baseUrl).href;
} catch (e) {
Expand Down Expand Up @@ -68,8 +67,9 @@ export async function makeAccountRequest({
* @returns
*/
async function parseResponse(response, shouldThrow) {
const responseType = response.headers.get("content-type");
const responseIsJSON = responseType?.includes("application/json");
const responseType =
response?.headers?.get("content-type") || "application/json";
const responseIsJSON = responseType.includes("application/json");
if (response.status >= 400 && shouldThrow) {
let message = `Failed to make request to Fauna account API [${response.status}]`;
if (responseIsJSON) {
Expand All @@ -79,7 +79,6 @@ async function parseResponse(response, shouldThrow) {
}
switch (response.status) {
case 401:
// TODO: try and refresh creds and then redo the call, if not then throw.
throw new InvalidCredsError(message);
case 403:
throw new UnauthorizedError(message);
Expand Down
4 changes: 2 additions & 2 deletions src/lib/auth/accountKeys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,11 @@
* refreshes it and returns it.
* @returns {string} - The account key
*/
async getOrRereshKey() {
async getOrRefreshKey() {
if (this.keySource === "credentials-file") {
const key = this.keyStore.get();
// TODO: track ttl for account and refresh keys

Check warning on line 85 in src/lib/auth/accountKeys.mjs

View workflow job for this annotation

GitHub Actions / lint

Unexpected 'todo' comment: 'TODO: track ttl for account and refresh...'
if (!key || (key.expiresAt && key.expiresAt < Date.now())) {
if (!key) {
this.logger.debug(
"Found account key, but it is expired. Refreshing...",
"creds",
Expand Down
6 changes: 3 additions & 3 deletions src/lib/auth/databaseKeys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,10 @@ export class DatabaseKeys {
ttl: new Date(expiration).toISOString(),
});
this.keyStore.save(this.keyName, {
secret: newSecret.secret,
secret: newSecret?.secret,
expiresAt: expiration,
});
this.key = newSecret.secret;
return newSecret.secret;
this.key = newSecret?.secret;
return newSecret?.secret;
}
}
5 changes: 5 additions & 0 deletions src/lib/command-helpers.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ const COMMON_QUERY_OPTIONS = {
required: false,
group: "API:",
},
accountKey: {
type: "string",
description: "The account key to use when calling Fauna",
required: false,
},
Comment on lines +55 to +59
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when are customers going to want/need to use this flag? for operations vs frontdoor?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We called it out in the design phase as something that can be set by the user. I don't know the use case yet, but I know we have a UI page to allow users to create one of these keys so who knows how they might want to use those.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also said we want flag parity for everything. It's probably the "right" thing to do.

It would allow you to use the cli even if frontdoor login was failing but the rest of frontdoor was okay, right?

database: {
alias: "d",
type: "string",
Expand Down
5 changes: 3 additions & 2 deletions src/lib/fauna-account-client.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
);
await this.accountKeys.onInvalidCreds();
// onInvalidCreds will refresh the account key
return await original(await this.getRequestArgs(args));
const updatedArgs = await this.getRequestArgs(args);
result = await original(updatedArgs);
} catch (e) {
if (e instanceof InvalidCredsError) {
logger.debug(
Expand All @@ -49,7 +50,7 @@
// By the time we are inside the retryableAccountRequest,
// the account key will have been refreshed. Use the latest value
async getRequestArgs(args) {
const updatedKey = await this.accountKeys.getOrRereshKey();
const updatedKey = await this.accountKeys.getOrRefreshKey();
return {
...args,
secret: updatedKey,
Expand Down Expand Up @@ -129,7 +130,7 @@
* @throws {Error} - Throws an error if there is an issue during session retrieval.
*/

// TODO: get/set expiration details

Check warning on line 133 in src/lib/fauna-account-client.mjs

View workflow job for this annotation

GitHub Actions / lint

Unexpected 'todo' comment: 'TODO: get/set expiration details'
static async getSession(accessToken) {
const makeAccountRequest = container.resolve("makeAccountRequest");
try {
Expand All @@ -146,7 +147,7 @@
}
}

// TODO: get/set expiration details

Check warning on line 150 in src/lib/fauna-account-client.mjs

View workflow job for this annotation

GitHub Actions / lint

Unexpected 'todo' comment: 'TODO: get/set expiration details'
/**
* Uses refreshToken to get a new accountKey and refreshToken.
* @param {*} refreshToken
Expand Down
Loading
Loading