Conversation
2ee6d70 to
902aacd
Compare
ace3263 to
336f8c1
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1476 +/- ##
==========================================
- Coverage 59.14% 58.95% -0.19%
==========================================
Files 120 121 +1
Lines 21194 22585 +1391
==========================================
+ Hits 12535 13316 +781
- Misses 7841 8419 +578
- Partials 818 850 +32 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@KartikJha , Please fix the linters errors |
|
| --scopes string Comma-separated list of permission scopes for the API | ||
| --signing-alg string Token signing algorithm: RS256, PS256, or HS256 (leave blank to be prompted interactively) | ||
| --token-lifetime string Access token lifetime in seconds (default: 86400 = 24 hours) (default "86400") |
There was a problem hiding this comment.
Highlight that these flags are for the API
| Callbacks: []string{"http://localhost:5173/callback"}, | ||
| AllowedLogoutURLs: []string{"http://localhost:5173"}, | ||
| WebOrigins: []string{DetectionSub}, |
There was a problem hiding this comment.
As discussed earlier, the port value should be prompted with default values like 5173 or 4200, as it varies depending on the configuration.
| if hasDep(earlyDeps, "@ionic/react") { | ||
| result.Framework = "ionic-react" | ||
| result.Type = "native" | ||
| result.BuildTool = "vite" |
There was a problem hiding this comment.
Instead of static build tool, try to fetch the build tool details from other files like .vite.config.ts
02bace3 to
c37dcc3
Compare
2f95d6f to
f5c678f
Compare
a02bac9 to
b4c53bb
Compare
…on values interactive prompting flow
…on no input test added, setupExp flag declaration using File struct
…ed env for angular
26ae3bc to
34f57d1
Compare
| fmt.Fprintf(r.MessageWriter, format+"\n", a...) | ||
| } | ||
|
|
||
| func (r *Renderer) InfofNoSpace(format string, a ...interface{}) { |
There was a problem hiding this comment.
InfofNoSpace is not used at anywhere
| case dirExists(dir, "android") || dirExists(dir, "ios"): | ||
| result.Framework = "flutter" | ||
| result.Type = "native" | ||
| result.BundleID = readMobileBundleID(dir) |
There was a problem hiding this comment.
Can this case go along with the default case
There was a problem hiding this comment.
Current code: native dirs take priority over web target.
Collapsed version: web target takes priority over native dirs.
// Collapsed — web target wins even if android/ or ios/ exist
switch {
case pubspecHasFlutterWebTarget(data):
result.Framework = "flutter-web"
result.Type = "spa"
default:
result.Framework = "flutter"
result.Type = "native"
result.BundleID = readMobileBundleID(dir)
}
A Flutter project can legitimately have both — flutter create --platforms=android,ios,web produces a project with native dirs and a web: section in pubspec. The current code intentionally
treats native dirs as the stronger signal, since a developer running setup-experimental on such a project is more likely targeting mobile Auth0 integration.
So collapsing is valid only if you're okay with flipping that priority. If the intent is "native dirs beat web target when both exist", the explicit first case is load-bearing and should
stay.
@ramya18101 most probably in this scenario we want to force the native scenario. Let me know if we still want to target web first then I can collapse the switch.
| cli.renderer.InfofBullet("App name: %s", detection.AppName) | ||
| if detection.Port > 0 { | ||
| cli.renderer.InfofBullet("Port: %d", detection.Port) |
There was a problem hiding this comment.
We don't need to display the app name and port as the detected values,
We can prompt the details by giving the default value
| var mobileTFMRegex = regexp.MustCompile(`net\d+\.\d+-(?:android|ios)`) | ||
|
|
||
| // extractPortFromContent returns the first port number found in content, or 0 if none found. | ||
| func extractPortFromContent(content string) int { |
There was a problem hiding this comment.
As discussed, we are not going to read the port values from the files,
Instead use the pre-defined default values shared in the Command Redesign Spec doc - v. 6
Remove these additional code
There was a problem hiding this comment.
@ramya18101 as discussed in the call time complexity of using this function for extracting the port from signal files is not greater than usability of auto port detection as it runs on client machine and will make the command more AI tooling friendly.
The fallback behavior already covers the concern here. detectPortFromConfig returns defaultPort whenever extraction
fails — so using predefined defaults is what the code does when no port is found in the config file. The file read is
just a best-effort check before that.
The reason to keep it: inputs.Port feeds directly into the Auth0 callback URL registration (baseURL :=
fmt.Sprintf("http://localhost:%d", inputs.Port) at line 635). If a developer has customized their vite.config.ts to
port: 4000 and we register http://localhost:5173, Auth0 will reject the redirect at login. Detecting the actual port
prevents a silent misconfiguration that's painful to debug.
Removing extractPortFromContent would mean the fallback default is always used, trading a small file read for a
potential broken Auth0 callback URL. Given the function already has the safety net of falling back to the default on
any failure, I'd keep it.
| func collectPackageJSONCandidates(deps map[string]bool) []detectionCandidate { | ||
| var candidates []detectionCandidate | ||
| // React-native without expo (expo check would have matched earlier in DetectProject). | ||
| if hasDep(deps, "react-native") { | ||
| candidates = append(candidates, detectionCandidate{framework: "react-native", qsType: "native"}) | ||
| } | ||
| if hasDep(deps, "express") { | ||
| candidates = append(candidates, detectionCandidate{framework: "express", qsType: "regular", port: 3000}) | ||
| } | ||
| if hasDep(deps, "hono") { | ||
| candidates = append(candidates, detectionCandidate{framework: "hono", qsType: "regular", port: 3000}) | ||
| } | ||
| if hasDep(deps, "fastify") { | ||
| candidates = append(candidates, detectionCandidate{framework: "fastify", qsType: "regular", port: 3000}) | ||
| } | ||
| return candidates | ||
| } |
There was a problem hiding this comment.
recheck on the port values, as for all the non-native app types we are having the same default port
There was a problem hiding this comment.
As discussed this comment is not an issue main issue is here
| if inputs.Port == 0 { | ||
| inputs.Port = defaultPortForFramework(inputs.Framework) | ||
| // Port stays 0 for native apps (react-native, expo, flutter) - no port needed. | ||
| } |
There was a problem hiding this comment.
Isn't this handled again under - Step 3d: Prompt for port if not explicitly set --.
There was a problem hiding this comment.
The two checks serve different execution paths and are not redundant.
The check at line 1482 (inside getQuickstartConfigKey) is the non-interactive fallback — when --no-input is set,
the Step 3d block is skipped entirely (guarded by canPromptFlag), so getQuickstartConfigKey is what ensures the
port gets a framework default before the config key is resolved.
The check at line 972 (inside Step 3d) pre-fills the interactive prompt default — without it, the prompt would
display 0 for the user to edit instead of the framework-appropriate value (e.g. 5173 for Vite). It's not setting
the final value; it's setting the starting value shown to the user.
So removing either one would be a regression: removing line 1482 breaks --no-input mode, removing line 972 breaks
the interactive prompt UX.
There was a problem hiding this comment.
So, re-check and remove the redundant logic as it already handled here
| // Config key is only meaningful when an app is being created. | ||
| if !inputs.App { | ||
| return "", inputs, false, nil | ||
| } |
There was a problem hiding this comment.
Do we really need this check..?
As app check is already happened in the start!
There was a problem hiding this comment.
Good catch. Implemented the early return pattern — the redundant if inputs.App wrapper and the unreachable if !inputs.App check have been removed. The function
now starts with:
if !inputs.App {
return "", inputs, false, nil
}
and the rest of the logic sits flat without an extra nesting level.
| } | ||
| } | ||
|
|
||
| // Resolve port from framework default before prompting (Bug 11). |
There was a problem hiding this comment.
Why are we highlighting the bugs..?
| if len(candidates) > 0 { | ||
| // Sort by priority (vite > webpack > cra > others alphabetically) so modern | ||
| // build tools are preferred over legacy ones. | ||
| buildToolPriority := map[string]int{"vite": 0, "webpack": 1, "cra": 2} | ||
| sort.Slice(candidates, func(i, j int) bool { | ||
| pi, pj := len(buildToolPriority)+1, len(buildToolPriority)+1 | ||
| if parts := strings.SplitN(candidates[i], ":", 3); len(parts) == 3 { | ||
| if p, ok := buildToolPriority[parts[2]]; ok { | ||
| pi = p | ||
| } | ||
| } | ||
| if parts := strings.SplitN(candidates[j], ":", 3); len(parts) == 3 { | ||
| if p, ok := buildToolPriority[parts[2]]; ok { | ||
| pj = p | ||
| } | ||
| } | ||
| if pi != pj { | ||
| return pi < pj | ||
| } | ||
| return candidates[i] < candidates[j] | ||
| }) | ||
| configKey = candidates[0] | ||
| // Update inputs.BuildTool so the caller can notify the user of the auto-selection. | ||
| parts := strings.SplitN(configKey, ":", 3) | ||
| if len(parts) == 3 { | ||
| inputs.BuildTool = parts[2] | ||
| } |
There was a problem hiding this comment.
Instead of this complicated logic, Can we enhance the frameworksForType logic to store the supported buildTools for each Framework!
There was a problem hiding this comment.
Done — introduced auth0.FrameworkBuildTools, a package-level map built from QuickstartConfigs at startup, keyed by
"type:framework" with the supported build tools sorted alphabetically. The dynamic search-and-sort block in
getQuickstartConfigKey is replaced with a single map lookup before configKey is built:
if tools := auth0.FrameworkBuildTools[inputs.Type+":"+inputs.Framework]; len(tools) > 0 {
buildToolKey = tools[0]
inputs.BuildTool = buildToolKey
wasAutoSelected = true
}
Kept it in auth0/quickstart.go alongside QuickstartConfigs rather than inside frameworksForType since it's derived
directly from the config map and serves a different concern — build tool resolution rather than prompting.
| // getQuickstartConfigKey resolves remaining missing prompts for App and API creation | ||
| // and returns the config map key for the selected framework. | ||
| // App/API selection and project detection are handled by the caller before this is invoked. | ||
| func getQuickstartConfigKey(inputs SetupInputs) (string, SetupInputs, bool, error) { |
There was a problem hiding this comment.
Divide it into functions as fetchNecessaryInputs and GetQsConfigKey
and since both inputs was used in both input and output params of that func, Please wisely use reference(*)
There was a problem hiding this comment.
Done — extracted resolveSetupInputs (which handles type validation/prompting, framework prompting, port default,
and build tool resolution) out of getQuickstartConfigKey, keeping the latter focused on flow control and key
construction.
On the pointer suggestion: SetupInputs is a plain struct with only value-type fields (string, bool, int) plus one
map. In Go, the idiomatic pattern for "transform and return a struct" is pass-by-value with a named return — it
avoids aliasing, keeps callers from observing partial mutations on error paths, and makes the data flow explicit
in the signature. Since the struct is passed by value, any modifications inside resolveSetupInputs operate on a
local copy and do not affect the caller's original struct — the updated inputs are explicitly returned and the
caller decides what to do with them. A pointer would be appropriate if the struct were large enough to make
copying expensive, or if we needed shared mutable state across goroutines — neither applies here. The current
(SetupInputs, bool, error) return signature is the right call.
| defaultName = "my-api" | ||
| } | ||
| if canPromptFlag { | ||
| q := prompt.TextInput("name", "Application Name", "Name for the Auth0 API", defaultName, true) |
There was a problem hiding this comment.
Why didn't we use existing flags here..?
| q := prompt.TextInput("token-lifetime", "Access token lifetime (seconds)", | ||
| "How long access tokens remain valid (default: 86400 = 24 hours)", defaultLifetime, true) | ||
| if err := prompt.AskOne(q, &inputs.TokenLifetime); err != nil { |
There was a problem hiding this comment.
Instead of using prompt.AskOne and cmd.Flags().Changed use our in-bulit flag methods
| setupExpIdentifier.RegisterString(cmd, &inputs.Identifier, "") | ||
| setupExpAudience.RegisterString(cmd, &inputs.Audience, "") |
There was a problem hiding this comment.
Why do we have identifier and audience as two seperate flags..
Aren't these both same..?
There was a problem hiding this comment.
--audience is an intentional alias for --identifier to accommodate developers who are more familiar with Auth0's "audience"
terminology (used widely in Auth0 docs and SDK config). Both resolve to the same value — the resource server identifier. The else
if inputs.Identifier == "" { inputs.Identifier = inputs.Audience } line is exactly what handles that aliasing. The flag help
text already documents this: "Alias for --identifier (unique audience URL for the API)".
| if defaultID == "" { | ||
| defaultID = inputs.Audience | ||
| } | ||
| if defaultID == "" && inputs.Name != "" { |
There was a problem hiding this comment.
WHy are we having 2 flags..?
There was a problem hiding this comment.
Assuming this is related to audience and identifier, it has been addressed here
| // Err != nil from url.Parse only fires on malformed percent-encoding; the | ||
| // host check catches bare schemes like "http://" that Parse accepts without error. | ||
| // u.User != nil rejects URLs with embedded credentials (e.g. http://user:pass@host). | ||
| u, err := url.Parse(identifier) |
There was a problem hiding this comment.
Use the similar existing logic to handle validation:
auth0-cli/internal/cli/apis.go
Line 606 in 18e479e
| DetectionSub = "DETECTION_SUB" | ||
| // DetectionSubBase is replaced at runtime with just the baseURL (no path suffix). | ||
| // Use this for SPA callback/logout URLs where the path is the app root. | ||
| DetectionSubBase = "DETECTION_SUB_BASE" |
There was a problem hiding this comment.
Rename as DetectionSubAsBase
| if u == auth0.DetectionSub || u == auth0.DetectionSubBase { | ||
| logoutURLs[i] = baseURL |
There was a problem hiding this comment.
WHy are we checking u == auth0.DetectionSubBase as we never set in the whole codebase as DetectionSubBase, we only used DetectionSub
| for key, value := range envValues { | ||
| if value != auth0.DetectionSub && value != auth0.DetectionSubBase { | ||
| updatedEnvValues[key] = value | ||
| continue | ||
| } | ||
|
|
||
| switch key { | ||
| case "VITE_AUTH0_DOMAIN", "AUTH0_DOMAIN", "domain", "NUXT_AUTH0_DOMAIN", |
There was a problem hiding this comment.
Divide this function into: replaceDetectionSub and resolveDetectionSubValue
as it currently does 2 operations
| envValues[config.AudienceVar] = inputs.Identifier | ||
| } | ||
|
|
||
| envFileName, _, err := GenerateAndWriteQuickstartConfig(&config.Strategy, envValues, cli.tenant, client, inputs.Port) |
There was a problem hiding this comment.
If we are not using the 2nd field from the output param of GenerateAndWriteQuickstartConfig then why are we even sending it..?
There was a problem hiding this comment.
The second output is the path of the envFile and is used in Test Suites to check for temp env file existence and overall makes the test suite more reliable.
| if inputs.Scopes != "" { | ||
| scopeList := strings.Split(inputs.Scopes, ",") | ||
| apiScopes := make([]management.ResourceServerScope, 0, len(scopeList)) | ||
| for _, s := range scopeList { | ||
| s = strings.TrimSpace(s) | ||
| if s != "" { | ||
| v := s | ||
| apiScopes = append(apiScopes, management.ResourceServerScope{Value: &v}) | ||
| } | ||
| } | ||
| if len(apiScopes) > 0 { | ||
| rs.Scopes = &apiScopes | ||
| } | ||
| } |
There was a problem hiding this comment.
Take the similar reference from :
auth0-cli/internal/cli/apis.go
Line 693 in 18e479e
and checkout the whole createAPI logic from the apis.go/createAPICmd
Changes
internal/cli/quickstart_detect.go(new file)DetectProject(dir string) DetectionResult— scans the working directory for framework signal files and returns a structured result withFramework,Type,BuildTool,Port,AppName,Detected, andAmbiguousCandidatesangular.json,vite.config.ts) take priority over generic dep scanning to avoid false positivesAmbiguousCandidatesis populated so the caller can prompt for disambiguationinternal/cli/quickstarts.goNew
setup-experimentalcommand flow:SetupInputsnamed struct replacing 3x repeated anonymous structsresolveRequestParams()— resolvesDetectionSubplaceholders inRequestParams(callbacks, logout URLs, web origins, name) using actual app name and port at runtimeDetectProject()intoRunE: on an existing project the detected framework/type/build-tool/port/name are shown as a summary and the user is asked to confirm; accepted values pre-fillSetupInputsso downstream prompts are skippedgetQuickstartConfigKeyskips any field already populated (by flags or detection) and only asks for what is missingmanagement.ResourceServer+cli.api.ResourceServer.Create()instead ofmanagement.Client<app-name>-API; API identifier defaults tohttps://<app-name-slug>--app,--api,--name,--type,--framework,--build-tool,--port,--callback-url,--logout-url,--web-origin-url,--identifier,--audience,--signing-alg,--scopes,--token-lifetime,--offline-accessOther fixes applied during review:
RS256overwrite on API signing algorithmAUTH0_SECRET,SESSION_SECRET) now usegenerateState(32)instead of a static placeholderEnvValuesis now sorted for deterministic config file outputgopkg.in/yaml.v2withbuildNestedMap()to correctly produce nested YAML from dot-delimited keys (e.g.okta.oauth2.issuer)auth0.DetectionSubconstantfmt.Printftocli.renderer.Infoffor consistencym2mremoved from the application type selection list"::none"error when only--apiwas selected (config key lookup now correctly skipped for API-only flows)internal/auth0/quickstart.goReferences
setup-experimentalTesting
Manual — existing project:
Manual — empty directory:
Manual — API-only:
auth0 quickstarts setup-experimental --api --identifier https://my-api # Expected: prompts to select an existing app, creates ResourceServer, no app client createdManual — App + API:
Unit tests for
DetectProjectcovering each of the 14 signal rules: N/A (to be added in a follow-up — mockingos.ReadFile/os.Statfor the full matrix is tracked separately).Checklist
--helptext and flag descriptions updated inline)