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

refactor: add some strong typing in authorization layer code #34

Merged
merged 1 commit into from
Aug 26, 2024

Conversation

craigzour
Copy link
Contributor

Summary | Résumé

  • Added some strong typing to authorization layer code

@craigzour craigzour self-assigned this Aug 23, 2024
@@ -31,10 +31,6 @@ function loadOptionalEnvVar(envVarName: string): string | undefined {
}

function loadRequiredEnvVar(envVarName: string): string {
if (process.env.NODE_ENV === "test") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed because all the configuration variables will now be mocked for all tests. See vitest-setup.ts file.


const algorithm = "RS256";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With optimization in mind I moved some of the variables out of the introspectToken function as they can be initialized once for all future calls to introspectToken.

@@ -37,11 +43,22 @@ export async function introspectToken(token: string) {
"Content-Type": "application/x-www-form-urlencoded",
},
},
)
.then((res) => res.data)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

👀

Should we reopen the debate about promise chaining vs async await?
https://gcdigital.slack.com/archives/C01GFD150MC/p1711033765657599

Copy link
Contributor

Choose a reason for hiding this comment

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

no no, try catch is good, sorry about that :)

@@ -0,0 +1,17 @@
import { vi } from "vitest";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This new file will be run before any test is invoked.

@craigzour craigzour force-pushed the refactor/authorization-middleware branch from 18540bf to eb3edda Compare August 23, 2024 20:29
}));

vi.mock("node:crypto", () => ({
createPrivateKey: vi.fn(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the introspectToken file is imported in a test file, the private key gets created right away (now that is lives outside of the function).

Because we don't want to have an RSA private key on Github (even if it is not a valid one), we have to mock the createPrivateKey function so that it does not try and fail to parse the ZITADEL_APPLICATION_KEY that is also mocked just above.

Copy link
Contributor

@wmoussa-gc wmoussa-gc left a comment

Choose a reason for hiding this comment

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

thank you!

@craigzour craigzour merged commit 941a13e into main Aug 26, 2024
4 checks passed
@craigzour craigzour deleted the refactor/authorization-middleware branch August 26, 2024 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants