feat(auth0): switch from cosmiconfig to configliere (#362)#363
Conversation
commit: |
Package Changes Through 646b85bNo changes. Add a change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
|
@jbolda Will you be able to help test this, and make improvements. I can't login with the default user due to SSL errors even when setting |
|
Hmm, how are you testing it out? If it uses the auth0 libs, those tend to throw errors if they aren't pointing at https. Is that the error you are getting? That is why we had the instructions to use |
|
I was running the start script, which executes For the unit tests, I modified them in a separate commit to pass 
|
|
As a bit of a separate question, do you have some requirements that prevent you from installing a local dev-only cert? I was considering automating that process more, a few libs can help some with it, but it the ramifications of trying to do it automatically were unclear. |
|
Not really, Regardless, if you want to drop the third commit of this PR, please feel free. |
| debug: boolean, | ||
| ): Record<Routes, RequestHandler> => { | ||
| let { audience, scope, clientID, rulesDirectory } = options; | ||
| let { audience, scope, clientId, rulesDirectory } = options; |
There was a problem hiding this comment.
I think clientID was explicitly used as that aligns with Auth0 docs. Could we confirm that and maybe link it to their docs for future reference?
There was a problem hiding this comment.
The WebAuth instantiation in views/login.ts still specifies clientID, but using it as our config option was creating --client-i-d and CLIENT_I_D as the parameters.
There was a problem hiding this comment.
Ahhh I see. @cowboyd do we have a good way to deal with this?
There was a problem hiding this comment.
I think we are going to have to put in an override API into configliere for this use case. I will do my best to whip that up here. I suspect that we are going to run into issues otherwise in getting it to work with auth0 libs if we don't follow their semantics.
There was a problem hiding this comment.
Alright. Although I do think that this project should be able to use any naming format within itself, and only the interface layer with Auth0 needs to worry about using parameters that Auth0 accepts.
There was a problem hiding this comment.
I don't inherently disagree. We want the incoming config to match whatever official auth0 accepts though. Also some of those variables are leaky into like the JWT if I recall.
I did get the PR going though, so we can update that soon here.
thefrontside/configliere#19
There was a problem hiding this comment.
Ah right, makes sense.
| const envs = [{ name: "env", value: process.env as Record<string, string> }]; | ||
| const values = [{ name: "options", value: options }]; | ||
| const result = auth0Program.parse({ args, envs, values }); | ||
|
|
||
| if (!result.ok) throw result.error; | ||
|
|
||
| return result.value.config; |
There was a problem hiding this comment.
It looks like you have some good progress here. We will need to parse with a file as well. The inject is the new part of the API to help in handling it. This example that uses the newer inject which we will need to load a config file. After we have our parser, the return of that is effectively an object. That object is what we can pass directly to the auth0 simulator.
There was a problem hiding this comment.
I need some help here. Would you mind taking this further in the direction that we want to go to?
There was a problem hiding this comment.
Sure I can toss a commit here in a little bit.
There was a problem hiding this comment.
Ok I pushed a commit. I think this is a good direction. We will still want to check the APIs to make sure we understand any breaking changes.
|
|
||
| export const simulation: Auth0Simulator = (args = {}) => { | ||
| const config = getConfig(args.options); | ||
| const config = getConfig(args.options, args.args); |
There was a problem hiding this comment.
The slight adjustment here is that we don't do the config handling here. args.options should just be passed all of the data that we get from config. We won't parse in this function as that should be complete by the time we call this function. I don't think it will create a breaking change as before and after we only have an object. This distinction though means we can do more advanced parsing and pass in the result here.
There was a problem hiding this comment.
How would start.cjs look? Would index.ts export getConfig which start.cjs will call and pass to simulation()?
There was a problem hiding this comment.
Yep! Or at least the config parser. getConfig() could probably be removed as you would want to handle commands and then call simulation only in some cases (instead of the early return that you have).
📝 WalkthroughWalkthroughReplaces ChangesAuth0 configliere migration and CLI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/auth0/src/handlers/auth0-handlers.ts (1)
130-134:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssertion checks the wrong client ID variable in failed-login flow.
Line 133 asserts
clientID, but this branch usesresponseClientID = query.client_id ?? clientID. The assert should validateresponseClientIDto match the actual value sent tologinView.💡 Proposed fix
- assert(!!clientID, `no clientID assigned`); + assert(!!responseClientID, `no clientID assigned`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/auth0/src/handlers/auth0-handlers.ts` around lines 130 - 134, The assertion at line 133 checks the wrong variable in the failed-login flow. The code assigns `responseClientID` as either `query.client_id` or `clientID`, and this `responseClientID` value is what gets sent to `loginView`. However, the assert statement validates `clientID` instead of `responseClientID`. Update the assertion to check `responseClientID` instead of `clientID` to ensure the actual value being used downstream is properly assigned.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/auth0/bin/start.mjs`:
- Around line 53-59: The startup URL construction in the console.log statement
hardcodes the protocol as https:// instead of using the configured protocol
parameter, and does not handle IPv6 address formatting (which requires brackets
around the address in URLs). Modify the URL string construction to use the
protocol variable that should be available from the command-line arguments or
configuration, and add logic to check if the host is an IPv6 address (contains
colons) and wrap it in brackets before inserting it into the URL template.
In `@packages/auth0/example/index.mts`:
- Around line 15-21: The startup message in the console.log statement hardcodes
"https://" in the URL regardless of the actual protocol configuration of the
simulator, and IPv6 addresses like "::" need to be wrapped in brackets to form
valid URLs. Modify the console.log message to use the correct protocol that
matches the simulator's actual configuration (likely HTTP for a local simulator)
and ensure IPv6 addresses are wrapped in brackets when constructing the URL
string for host.
In `@packages/auth0/README.md`:
- Line 31: The README.md file at line 31 contains an instance of "github issue"
that should be capitalized as "GitHub issue" to follow proper product naming
conventions. Locate the text "please create a github issue to start a
conversation about" and capitalize the "g" in "github" to "GitHub" to match the
official product name.
- Around line 39-41: The README documentation contains an inconsistency between
the HTTPS requirement stated in lines 39-41 and the Quick Start protocol example
on line 48. Update the default startup URL example on line 48 from using HTTP to
HTTPS protocol (change http://localhost:4400 to https://localhost:4400) to align
with both the Auth0 client HTTPS requirement documented in lines 39-41 and the
default config sample shown in lines 56-57, ensuring consistent messaging
throughout the documentation.
In `@packages/auth0/src/types.ts`:
- Around line 28-32: The validation message for the clientID field is misleading
because it says "must be 32 characters long" but the schema uses .max(32) which
only enforces a maximum length, not an exact length. Either change the
validation to use .length(32) to enforce exactly 32 characters (which appears to
match the 32-character default value), or update the error message to say "must
be at most 32 characters long" to accurately reflect the maximum length
constraint. Choose based on whether exact or maximum length validation is
actually required for the clientID field.
---
Outside diff comments:
In `@packages/auth0/src/handlers/auth0-handlers.ts`:
- Around line 130-134: The assertion at line 133 checks the wrong variable in
the failed-login flow. The code assigns `responseClientID` as either
`query.client_id` or `clientID`, and this `responseClientID` value is what gets
sent to `loginView`. However, the assert statement validates `clientID` instead
of `responseClientID`. Update the assertion to check `responseClientID` instead
of `clientID` to ensure the actual value being used downstream is properly
assigned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 79b2a5b4-8b0c-49d4-b0f9-96ab85c87183
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
packages/auth0/README.mdpackages/auth0/bin/start.mjspackages/auth0/example/index.mtspackages/auth0/package.jsonpackages/auth0/src/config/get-config.tspackages/auth0/src/handlers/auth0-handlers.tspackages/auth0/src/index.tspackages/auth0/src/types.tspackages/auth0/src/views/login.tspackages/auth0/test/config.test.ts
| const host = | ||
| typeof info === "object" && info?.address && !["::", "0.0.0.0"].includes(info.address) | ||
| ? info.address | ||
| : "localhost"; | ||
| console.log( | ||
| `Auth0 simulation server started at https://${host}:${port}\n` + | ||
| `Visit the root route to view all available routes.\n\n` + |
There was a problem hiding this comment.
Use the configured protocol in the startup URL (and normalize IPv6 host formatting).
Line 58 hardcodes https://, so --protocol http still prints an HTTPS URL. Also, IPv6 addresses like ::1 should be bracketed before composing a URL.
💡 Proposed fix
- const host =
- typeof info === "object" && info?.address && !["::", "0.0.0.0"].includes(info.address)
- ? info.address
- : "localhost";
+ const protocol = result.value.protocol ?? "https";
+ const rawHost =
+ typeof info === "object" && info?.address && !["::", "0.0.0.0"].includes(info.address)
+ ? info.address
+ : "localhost";
+ const host = rawHost.includes(":") ? `[${rawHost}]` : rawHost;
console.log(
- `Auth0 simulation server started at https://${host}:${port}\n` +
+ `Auth0 simulation server started at ${protocol}://${host}:${port}\n` +
`Visit the root route to view all available routes.\n\n` +
`Point your configuration at this simulation server and use the default user below.\n` +
`Email: ${defaultUser.email}\nPassword: ${defaultUser.password}\n` +📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const host = | |
| typeof info === "object" && info?.address && !["::", "0.0.0.0"].includes(info.address) | |
| ? info.address | |
| : "localhost"; | |
| console.log( | |
| `Auth0 simulation server started at https://${host}:${port}\n` + | |
| `Visit the root route to view all available routes.\n\n` + | |
| const protocol = result.value.protocol ?? "https"; | |
| const rawHost = | |
| typeof info === "object" && info?.address && !["::", "0.0.0.0"].includes(info.address) | |
| ? info.address | |
| : "localhost"; | |
| const host = rawHost.includes(":") ? `[${rawHost}]` : rawHost; | |
| console.log( | |
| `Auth0 simulation server started at ${protocol}://${host}:${port}\n` + | |
| `Visit the root route to view all available routes.\n\n` + |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/auth0/bin/start.mjs` around lines 53 - 59, The startup URL
construction in the console.log statement hardcodes the protocol as https://
instead of using the configured protocol parameter, and does not handle IPv6
address formatting (which requires brackets around the address in URLs). Modify
the URL string construction to use the protocol variable that should be
available from the command-line arguments or configuration, and add logic to
check if the host is an IPv6 address (contains colons) and wrap it in brackets
before inserting it into the URL template.
| const host = | ||
| typeof info === "object" && info?.address && !["::", "0.0.0.0"].includes(info.address) | ||
| ? info.address | ||
| : "localhost"; | ||
| console.log( | ||
| `auth0 simulation server started at https://localhost:4400\nusername: default@example.com\npassword: 12345\n`, | ||
| ), | ||
| ); | ||
| `Auth0 simulation server started at https://${host}:${port}\n` + | ||
| `username: default@example.com\n` + |
There was a problem hiding this comment.
Startup message should not hardcode HTTPS in the example output.
Line 20 always prints https://... even when the simulator is configured for HTTP, and raw IPv6 hosts should be bracketed for a valid URL string.
💡 Proposed fix
app.listen().then(({ server, port }) => {
const info = server.address();
- const host =
+ const protocol = "https"; // or derive from configured protocol if exposed in this example
+ const rawHost =
typeof info === "object" && info?.address && !["::", "0.0.0.0"].includes(info.address)
? info.address
: "localhost";
+ const host = rawHost.includes(":") ? `[${rawHost}]` : rawHost;
console.log(
- `Auth0 simulation server started at https://${host}:${port}\n` +
+ `Auth0 simulation server started at ${protocol}://${host}:${port}\n` +
`username: default@example.com\n` +
`password: 12345\n`,
);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const host = | |
| typeof info === "object" && info?.address && !["::", "0.0.0.0"].includes(info.address) | |
| ? info.address | |
| : "localhost"; | |
| console.log( | |
| `auth0 simulation server started at https://localhost:4400\nusername: default@example.com\npassword: 12345\n`, | |
| ), | |
| ); | |
| `Auth0 simulation server started at https://${host}:${port}\n` + | |
| `username: default@example.com\n` + | |
| const protocol = "https"; // or derive from configured protocol if exposed in this example | |
| const rawHost = | |
| typeof info === "object" && info?.address && !["::", "0.0.0.0"].includes(info.address) | |
| ? info.address | |
| : "localhost"; | |
| const host = rawHost.includes(":") ? `[${rawHost}]` : rawHost; | |
| console.log( | |
| `Auth0 simulation server started at ${protocol}://${host}:${port}\n` + | |
| `username: default@example.com\n` + | |
| `password: 12345\n`, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/auth0/example/index.mts` around lines 15 - 21, The startup message
in the console.log statement hardcodes "https://" in the URL regardless of the
actual protocol configuration of the simulator, and IPv6 addresses like "::"
need to be wrapped in brackets to form valid URLs. Modify the console.log
message to use the correct protocol that matches the simulator's actual
configuration (likely HTTP for a local simulator) and ensure IPv6 addresses are
wrapped in brackets when constructing the URL string for host.
| flow](https://developer.okta.com/docs/concepts/oauth-openid/). | ||
|
|
||
| If this does not meet your needs then please create a github issue to start a conversation about adding new OpenID flows. | ||
| If this does not meet your needs then please create a github issue to start a conversation about |
There was a problem hiding this comment.
Capitalize “GitHub” in user-facing docs.
Line 31 should use GitHub issue for correct product naming.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~31-~31: The official name of this software platform is spelled with a capital “H”.
Context: ...ot meet your needs then please create a github issue to start a conversation about add...
(GITHUB)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/auth0/README.md` at line 31, The README.md file at line 31 contains
an instance of "github issue" that should be capitalized as "GitHub issue" to
follow proper product naming conventions. Locate the text "please create a
github issue to start a conversation about" and capitalize the "g" in "github"
to "GitHub" to match the official product name.
Source: Linters/SAST tools
| > The Auth0 clients expect the server to be served as `https`, and will throw an error if it is | ||
| > served as `http`. Currently, we rely on a certificate available in the home directory. On first | ||
| > run, you will see instructions on how to set up this certificate through `mkcert`. |
There was a problem hiding this comment.
Quick Start protocol example is inconsistent with the surrounding HTTPS guidance.
Line 48 says the default startup URL is http://localhost:4400, while Lines 39-41 and the default config sample (Lines 56-57) point to HTTPS. Please align these to a single default behavior to avoid setup confusion.
Also applies to: 48-48, 56-57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/auth0/README.md` around lines 39 - 41, The README documentation
contains an inconsistency between the HTTPS requirement stated in lines 39-41
and the Quick Start protocol example on line 48. Update the default startup URL
example on line 48 from using HTTP to HTTPS protocol (change
http://localhost:4400 to https://localhost:4400) to align with both the Auth0
client HTTPS requirement documented in lines 39-41 and the default config sample
shown in lines 56-57, ensuring consistent messaging throughout the
documentation.
| clientID: { | ||
| schema: z.optional(z.string().max(32, "must be 32 characters long")), | ||
| description: "auth0 client ID", | ||
| default: "00000000000000000000000000000000" as const, | ||
| }, |
There was a problem hiding this comment.
Misleading validation message for clientID.
The validation message says "must be 32 characters long" but max(32) enforces a maximum length, not an exact length. If exact length is required, use .length(32). If up to 32 is correct, update the message.
Suggested fix
If max length is intended:
clientID: {
- schema: z.optional(z.string().max(32, "must be 32 characters long")),
+ schema: z.optional(z.string().max(32, "must be at most 32 characters")),
description: "auth0 client ID",
default: "00000000000000000000000000000000" as const,
},Or if exact length is intended:
clientID: {
- schema: z.optional(z.string().max(32, "must be 32 characters long")),
+ schema: z.optional(z.string().length(32, "must be exactly 32 characters")),
description: "auth0 client ID",
default: "00000000000000000000000000000000" as const,
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| clientID: { | |
| schema: z.optional(z.string().max(32, "must be 32 characters long")), | |
| description: "auth0 client ID", | |
| default: "00000000000000000000000000000000" as const, | |
| }, | |
| clientID: { | |
| schema: z.optional(z.string().length(32, "must be exactly 32 characters")), | |
| description: "auth0 client ID", | |
| default: "00000000000000000000000000000000" as const, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/auth0/src/types.ts` around lines 28 - 32, The validation message for
the clientID field is misleading because it says "must be 32 characters long"
but the schema uses .max(32) which only enforces a maximum length, not an exact
length. Either change the validation to use .length(32) to enforce exactly 32
characters (which appears to match the 32-character default value), or update
the error message to say "must be at most 32 characters long" to accurately
reflect the maximum length constraint. Choose based on whether exact or maximum
length validation is actually required for the clientID field.
3 participants
Motivation
Migrates from cosmiconfig to in-house configliere for parsing configuration options.
Adds a
protocolconfig option to allow users to choose serving overhttporhttps.Summary by CodeRabbit
New Features
--helpand explicitstartcommand.Documentation
Chores