-
Notifications
You must be signed in to change notification settings - Fork 299
feat(ruby): remove omitted basic auth fields from SDK API, add WireMock auth matching #14411
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
Changes from all commits
3c14b11
0a456d4
152f413
911da89
05c7d3e
fcb906f
be380db
23a7255
bf8d60b
1eaea0e
662530d
5476179
38173e7
139a33a
c484fcb
ba92a72
844fb07
f0e28f1
893d218
306b946
b2a8b98
2b5bd20
d36ba63
6a023f3
77ee9f9
889e958
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -116,31 +116,59 @@ export class RootClientGenerator extends FileGenerator<RubyFile, SdkCustomConfig | |
| writer.write(`headers = `); | ||
| writer.writeNode(this.getRawClientHeaders()); | ||
| writer.newLine(); | ||
| let isFirstBlock = true; | ||
| let emittedAnyBlock = false; | ||
| for (let i = 0; i < basicAuthSchemes.length; i++) { | ||
| const basicAuthScheme = basicAuthSchemes[i]; | ||
| if (basicAuthScheme == null) { | ||
| continue; | ||
| } | ||
| const usernameName = this.case.snakeSafe(basicAuthScheme.username); | ||
| const passwordName = this.case.snakeSafe(basicAuthScheme.password); | ||
| const usernameOmitted = !!basicAuthScheme.usernameOmit; | ||
| const passwordOmitted = !!basicAuthScheme.passwordOmit; | ||
| // Build the credential string for Base64 encoding. | ||
| // Omitted fields become empty (e.g., password omitted → "#{username}:"). | ||
| let credentialStr: string; | ||
| if (usernameOmitted && !passwordOmitted) { | ||
| credentialStr = `":#{${passwordName}}"`; | ||
| } else if (!usernameOmitted && passwordOmitted) { | ||
| credentialStr = `"#{${usernameName}}:"`; | ||
| } else { | ||
| credentialStr = `"#{${usernameName}}:#{${passwordName}}"`; | ||
| } | ||
| // Condition: only require non-omitted fields to be present | ||
| let condition: string; | ||
| if (!usernameOmitted && !passwordOmitted) { | ||
| condition = `!${usernameName}.nil? && !${passwordName}.nil?`; | ||
| } else if (usernameOmitted && !passwordOmitted) { | ||
| condition = `!${passwordName}.nil?`; | ||
| } else if (!usernameOmitted && passwordOmitted) { | ||
| condition = `!${usernameName}.nil?`; | ||
| } else { | ||
| // Both fields omitted — skip auth header entirely when auth is non-mandatory | ||
| continue; | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
|
||
| } | ||
|
Comment on lines
+142
to
+151
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical bug: When auth is required ( For example, if
Fix: When a field is omitted but auth is required, the condition check should still be applied: if (!usernameOmitted && !passwordOmitted) {
// Both required - check both or neither based on isAuthOptional
condition = `!${usernameName}.nil? && !${passwordName}.nil?`;
} else if (usernameOmitted && !passwordOmitted) {
condition = `!${passwordName}.nil?`;
} else if (!usernameOmitted && passwordOmitted) {
condition = `!${usernameName}.nil?`;
} else {
continue;
}
// Always use condition when there's a non-omitted field that could be nil
if (isAuthOptional || basicAuthSchemes.length > 1 || usernameOmitted || passwordOmitted) {
// Use conditional logic
} else {
// Both fields present and required
}Spotted by Graphite This comment came from an experimental review—please leave feedback if it was helpful/unhelpful. Learn more about experimental comments here. |
||
| if (isAuthOptional || basicAuthSchemes.length > 1) { | ||
| if (i === 0) { | ||
| writer.writeLine(`if !${usernameName}.nil? && !${passwordName}.nil?`); | ||
| if (isFirstBlock) { | ||
| writer.writeLine(`if ${condition}`); | ||
| } else { | ||
| writer.writeLine(`elsif !${usernameName}.nil? && !${passwordName}.nil?`); | ||
| writer.writeLine(`elsif ${condition}`); | ||
| } | ||
| isFirstBlock = false; | ||
| emittedAnyBlock = true; | ||
| writer.writeLine( | ||
| ` headers["Authorization"] = "Basic #{Base64.strict_encode64("#{${usernameName}}:#{${passwordName}}")}"` | ||
| ` headers["Authorization"] = "Basic #{Base64.strict_encode64(${credentialStr})}"` | ||
| ); | ||
| if (i === basicAuthSchemes.length - 1) { | ||
| writer.writeLine(`end`); | ||
| } | ||
| } else { | ||
| writer.writeLine( | ||
| `headers["Authorization"] = "Basic #{Base64.strict_encode64("#{${usernameName}}:#{${passwordName}}")}"` | ||
| `headers["Authorization"] = "Basic #{Base64.strict_encode64(${credentialStr})}"` | ||
| ); | ||
| } | ||
| } | ||
| if (emittedAnyBlock && (isAuthOptional || basicAuthSchemes.length > 1)) { | ||
| writer.writeLine(`end`); | ||
| } | ||
| } | ||
| writer.write(`@raw_client = `); | ||
| writer.writeNode(this.context.getRawClientClassReference()); | ||
|
|
@@ -344,30 +372,37 @@ export class RootClientGenerator extends FileGenerator<RubyFile, SdkCustomConfig | |
| break; | ||
| } | ||
| case "basic": { | ||
| const usernameParam = ruby.parameters.keyword({ | ||
| name: this.case.snakeSafe(scheme.username), | ||
| type: ruby.Type.string(), | ||
| initializer: | ||
| scheme.usernameEnvVar != null | ||
| ? ruby.codeblock((writer) => { | ||
| writer.write(`ENV.fetch("${scheme.usernameEnvVar}", nil)`); | ||
| }) | ||
| : undefined, | ||
| docs: undefined | ||
| }); | ||
| parameters.push(usernameParam); | ||
| const passwordParam = ruby.parameters.keyword({ | ||
| name: this.case.snakeSafe(scheme.password), | ||
| type: ruby.Type.string(), | ||
| initializer: | ||
| scheme.passwordEnvVar != null | ||
| ? ruby.codeblock((writer) => { | ||
| writer.write(`ENV.fetch("${scheme.passwordEnvVar}", nil)`); | ||
| }) | ||
| : undefined, | ||
| docs: undefined | ||
| }); | ||
| parameters.push(passwordParam); | ||
| // When omit is true, the field is completely removed from the end-user API. | ||
| const usernameOmitted = !!scheme.usernameOmit; | ||
| const passwordOmitted = !!scheme.passwordOmit; | ||
| if (!usernameOmitted) { | ||
| const usernameParam = ruby.parameters.keyword({ | ||
| name: this.case.snakeSafe(scheme.username), | ||
| type: ruby.Type.string(), | ||
| initializer: | ||
| scheme.usernameEnvVar != null | ||
| ? ruby.codeblock((writer) => { | ||
| writer.write(`ENV.fetch("${scheme.usernameEnvVar}", nil)`); | ||
| }) | ||
| : undefined, | ||
| docs: undefined | ||
| }); | ||
| parameters.push(usernameParam); | ||
| } | ||
| if (!passwordOmitted) { | ||
| const passwordParam = ruby.parameters.keyword({ | ||
| name: this.case.snakeSafe(scheme.password), | ||
| type: ruby.Type.string(), | ||
| initializer: | ||
| scheme.passwordEnvVar != null | ||
| ? ruby.codeblock((writer) => { | ||
| writer.write(`ENV.fetch("${scheme.passwordEnvVar}", nil)`); | ||
| }) | ||
| : undefined, | ||
| docs: undefined | ||
| }); | ||
| parameters.push(passwordParam); | ||
| } | ||
| break; | ||
| } | ||
| case "inferred": { | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Dynamic IR BasicAuth type not updated — usernameOmit/passwordOmit smuggled via untyped object spread
The dynamic IR
BasicAuthdefinition (packages/ir-sdk/fern/apis/ir-types-latest/definition/dynamic/auth.yml:22-25) only hasusernameandpassword. The converter atDynamicSnippetsConverter.ts:736-749smugglesusernameOmit/passwordOmitas extra runtime properties throughAuth.basic()'s...valuespread (packages/ir-sdk/src/sdk/api/resources/dynamic/resources/auth/types/Auth.ts:50). TheEndpointSnippetGenerator.ts:217then reads them viaas unknown as Record<string, unknown>. This is fragile: any code path that reconstructs theBasicAuthobject (e.g., the V66→V65 IR migration atpackages/cli/generation/ir-migrations/src/migrations/v66-to-v65/migrateFromV66ToV65.ts:1780-1783which explicitly creates{ username, password }) will silently drop these properties. The proper fix is to addusernameOmitandpasswordOmitas optional fields on the dynamic IR'sBasicAuthtype definition, then regenerate the SDK types.Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a known limitation. The
FernIr.dynamic.BasicAuthtype from the dynamic IR SDK doesn't have typedusernameOmit/passwordOmitfields yet. Theas unknown as Record<string, unknown>cast is necessary because the published SDK package lacks these properties even though they exist in the IR schema. Updating the dynamic IR types is out of scope for this PR per maintainer instruction ("Fix the non-IR changes"). The cast will be removed once the published IR SDK is updated to include these fields.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acknowledged — the proper fix is to add
usernameOmit/passwordOmitto the dynamic IR'sBasicAuthtype definition. However, updating the dynamic IR type definition is an IR change, which is out of scope for this PR per maintainer instruction ("Fix the non-IR changes"). The cast is a deliberate workaround to avoid modifying the IR SDK types.The
DynamicSnippetsConverternow passes these fields through as extra properties on the dynamic IR data, and the dynamic snippets generators read them via cast. This is intentionally temporary — once the dynamic IR types are updated to include these fields (in a separate IR PR), these casts can be removed.