Repository navigation
feat: add OAuth 1.0 authentication support - #7482
Merged
Merged
Conversation
lohit-bruno
requested review from
bijin-bruno,
helloanoop and
naman-bruno
as code owners
March 13, 2026 17:20
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds end-to-end OAuth 1.0 (RFC 5849) support: UI components and menus, signer library (HMAC/PLAINTEXT/RSA), CLI/Electron request prep and signing, converters/types/schemas, BRU/YML language support, Redux persistence, extensive unit and Playwright tests, and test fixtures. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI
participant Redux as Redux Store
participant Prep as Request Prep (CLI/Electron)
participant Sign as OAuth1 Signer (`@usebruno/requests`)
participant HTTP as HTTP Client
UI->>Redux: save oauth1 fields / set mode=oauth1
Redux->>Redux: persist draft/request/collection oauth1
Prep->>Prep: interpolate variables into oauth1 config
Prep->>Sign: pass axiosRequest.oauth1config (request or inherited)
Sign->>Sign: resolve private key (inline or read file & cache)
Sign->>Sign: build params, base string, signing key
Sign->>Sign: compute signature (HMAC/RSA/PLAINTEXT)
Sign->>Prep: attach oauth params to header/query/body
Prep->>HTTP: execute signed request
HTTP-->>Prep: response -> UI
sequenceDiagram
participant BruLang as BRU Parser/Serializer
participant AST as JSON/AST
participant Conv as Converters
participant Store as File Formats (BRU/YML/OpenCollection)
BruLang->>AST: parse auth:oauth1 { ... } (support `@file`)
AST->>Conv: produce bruno auth.oauth1 + privateKey type
Conv->>Store: serialize to BRU/YML/OpenCollection
Store->>BruLang: round-trip parsing/serialization tests
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
lohit-bruno
changed the title
OAuth 1.0 authentication support
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (15)
tests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 Body 200.bru (1)
20-49: Consider centralizing the PEM test key to reduce duplication.If this same RSA key appears in other OAuth1 RSA fixtures, pulling it from a shared fixture source would lower copy/paste drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auth/oauth1/fixtures/collections/bru/OAuth1` RSA-SHA1 Body 200.bru around lines 20 - 49, The RSA PEM private_key block in the OAuth1 RSA-SHA1 fixture is duplicated; extract the PEM into a single shared fixture (e.g., a shared "rsa_private_key" fixture) and replace the inline private_key value in OAuth1 RSA-SHA1 Body 200.bru with a reference to that shared fixture; update any other OAuth1 RSA fixtures to reference the same shared symbol so all tests consume the centralized key and eliminate copy/paste drift.tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Body JSON 200.yml (1)
11-14: Use clearly fake credential literals to reduce secret-scanner noise.These are test fixtures, but current values can still trip generic secret detectors and create noisy security reports.
Suggested placeholder cleanup
- consumerKey: consumer_key_1 - consumerSecret: consumer_secret_1 - accessToken: access_token_1 - tokenSecret: token_secret_1 + consumerKey: oauth1-test-id + consumerSecret: oauth1-test-secret + accessToken: oauth1-test-token + tokenSecret: oauth1-test-token-secret🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` HMAC-SHA1 Body JSON 200.yml around lines 11 - 14, Replace the realistic-looking credential values in the fixture by using clearly fake placeholder literals for the keys consumerKey, consumerSecret, accessToken, and tokenSecret (for example values like "fake_consumer_key", "fake_consumer_secret", "fake_access_token", "fake_token_secret") so the test remains meaningful but won't trigger secret scanners; update the corresponding entries in the OAuth1 HMAC-SHA1 Body JSON fixture to use those fake placeholders.tests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT Query Params 200.bru (1)
29-32: Consider adding email assertion for parity with other 200 fixtures.The HMAC-SHA1 200.bru fixture includes an assertion for
res.body.resource.email. Adding it here would maintain consistency:assert { res.status: eq 200 res.body.resource.name: eq oauth1-test-resource res.body.resource.email: eq oauth1@example.com }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auth/oauth1/fixtures/collections/bru/OAuth1` PLAINTEXT Query Params 200.bru around lines 29 - 32, Add an email assertion to the existing response assertions: inside the assert block that currently checks res.status and res.body.resource.name (the assertions referencing res.status and res.body.resource.name: eq oauth1-test-resource), add a line asserting res.body.resource.email: eq oauth1@example.com so this PLAINTEXT fixture matches the parity of the HMAC-SHA1 200.bru fixture.tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 Body 200.yml (1)
22-30: Missing email assertion for consistency.Other 200 fixtures (e.g., OAuth1 HMAC-SHA1 200.bru, OAuth1 RSA-SHA256 200.yml) include an assertion for
res.body.resource.email. Consider adding it here for consistency:- expression: res.body.resource.email operator: eq value: oauth1@example.com🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` HMAC-SHA256 Body 200.yml around lines 22 - 30, The 200 fixture is missing the email assertion for consistency with other OAuth1 200 fixtures; add an assertion entry under runtime.assertions that checks res.body.resource.email equals oauth1@example.com by inserting an assertion with expression: res.body.resource.email, operator: eq, and value: oauth1@example.com (matching the existing assertion style used for res.body.resource.name).tests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Body 200.yml (1)
53-61: Consider adding email assertion for consistency.Other OAuth1 200 fixtures (e.g., HMAC-SHA1, RSA-SHA1) include an assertion for
res.body.resource.email. This fixture only checksresource.name. Adding the email check would improve consistency across your test suite.💡 Suggested addition
runtime: assertions: - expression: res.status operator: eq value: '200' - expression: res.body.resource.name operator: eq value: oauth1-test-resource + - expression: res.body.resource.email + operator: eq + value: oauth1@example.com🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` RSA-SHA1 Body 200.yml around lines 53 - 61, This fixture's runtime assertions only check res.body.resource.name but should also assert res.body.resource.email for consistency with other OAuth1 200 fixtures; update the assertions array in the YAML (the block containing the expressions "res.body.resource.name" and "res.status") to add a new assertion whose expression is "res.body.resource.email", operator "eq", and value matching the expected test email used elsewhere in OAuth1 fixtures (use the same email literal used by HMAC-SHA1/RSA-SHA1 fixtures) so the test verifies both name and email.tests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 200.yml (1)
9-17: MissingconsumerSecretfield for schema consistency.The
OAuth1 RSA-SHA1 Body 200.ymlfixture explicitly includesconsumerSecret: ''while this file omits it entirely. For consistency across fixtures (and to ensure the YAML schema is exercised uniformly), consider adding the field.💡 Suggested addition
auth: type: oauth1 consumerKey: consumer_key_1 + consumerSecret: '' accessToken: access_token_1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` RSA-SHA1 200.yml around lines 9 - 17, Add the missing consumerSecret field to the auth block so the fixture matches the schema used by other OAuth1 RSA-SHA1 fixtures; update the auth map by adding consumerSecret: '' alongside consumerKey, accessToken, tokenSecret, signatureMethod, version, addParamsTo, and includeBodyHash to ensure consistent schema coverage.tests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT Query Params 200.yml (1)
20-28: Consider adding email assertion for consistency.Most other OAuth1 200 fixtures include assertions for both
resource.nameandresource.email. This fixture only checksresource.name. Adding the email assertion would maintain test consistency.💡 Suggested addition
runtime: assertions: - expression: res.status operator: eq value: '200' - expression: res.body.resource.name operator: eq value: oauth1-test-resource + - expression: res.body.resource.email + operator: eq + value: oauth1@example.com🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` PLAINTEXT Query Params 200.yml around lines 20 - 28, The fixture is missing an assertion for resource.email; update the runtime.assertions array in the OAuth1 PLAINTEXT Query Params 200.yml fixture (alongside the existing assertions that check res.status and res.body.resource.name) by adding an assertion that checks expression "res.body.resource.email" with operator "eq" and the expected email value used across other OAuth1 200 fixtures (e.g., "oauth1-test@example.com"), placing it after the resource.name assertion for consistency.tests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA512 200.yml (1)
20-48: PEM keys in test fixtures are properly scoped, but add secret-scan allowlist if CI scanning is implemented.The inline PEM keys across the OAuth1 test fixtures are correctly limited to test code only and are intentional for testing the
privateKey.type: textpath. No current secret-scan configuration exists in the repository, so there's no immediate risk. However, when secret scanning (e.g., gitleaks, checkov) is added to CI, create a narrowly scoped allowlist fortests/auth/oauth1/fixtures/collections/to prevent false positives on these test PEM blocks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` RSA-SHA512 200.yml around lines 20 - 48, This fixture contains an inline PEM private key (the multi-line "-----BEGIN PRIVATE KEY-----" value in the OAuth1 RSA-SHA512 200.yml fixture) used only for tests; add a narrowly scoped secret-scan allowlist entry in your CI secret-scanning config (e.g., gitleaks/checkov rules) that whitelists this fixture directory or filename pattern and/or the specific YAML "value" field pattern to suppress false positives, ensuring the rule is limited to tests/auth/oauth1 fixtures and does not broadly exempt other files.packages/bruno-cli/src/runner/prepare-request.js (1)
152-170: Extract OAuth1 config mapping into one helper to avoid drift.Both branches map the same OAuth1 fields. A shared builder keeps this maintainable and prevents subtle divergence later.
♻️ Suggested refactor
+const buildOAuth1Config = (source, basePath) => { + return { + consumerKey: get(source, `${basePath}.consumerKey`), + consumerSecret: get(source, `${basePath}.consumerSecret`), + accessToken: get(source, `${basePath}.accessToken`), + tokenSecret: get(source, `${basePath}.tokenSecret`), + callbackUrl: get(source, `${basePath}.callbackUrl`), + verifier: get(source, `${basePath}.verifier`), + signatureMethod: get(source, `${basePath}.signatureMethod`), + privateKey: get(source, `${basePath}.privateKey`), + privateKeyType: get(source, `${basePath}.privateKeyType`), + timestamp: get(source, `${basePath}.timestamp`), + nonce: get(source, `${basePath}.nonce`), + version: get(source, `${basePath}.version`), + realm: get(source, `${basePath}.realm`), + addParamsTo: get(source, `${basePath}.addParamsTo`), + includeBodyHash: get(source, `${basePath}.includeBodyHash`) + }; +}; @@ - if (collectionAuth.mode === 'oauth1') { - axiosRequest.oauth1config = { - consumerKey: get(collectionAuth, 'oauth1.consumerKey'), - consumerSecret: get(collectionAuth, 'oauth1.consumerSecret'), - accessToken: get(collectionAuth, 'oauth1.accessToken'), - tokenSecret: get(collectionAuth, 'oauth1.tokenSecret'), - callbackUrl: get(collectionAuth, 'oauth1.callbackUrl'), - verifier: get(collectionAuth, 'oauth1.verifier'), - signatureMethod: get(collectionAuth, 'oauth1.signatureMethod'), - privateKey: get(collectionAuth, 'oauth1.privateKey'), - privateKeyType: get(collectionAuth, 'oauth1.privateKeyType'), - timestamp: get(collectionAuth, 'oauth1.timestamp'), - nonce: get(collectionAuth, 'oauth1.nonce'), - version: get(collectionAuth, 'oauth1.version'), - realm: get(collectionAuth, 'oauth1.realm'), - addParamsTo: get(collectionAuth, 'oauth1.addParamsTo'), - includeBodyHash: get(collectionAuth, 'oauth1.includeBodyHash') - }; - } + if (collectionAuth.mode === 'oauth1') { + axiosRequest.oauth1config = buildOAuth1Config(collectionAuth, 'oauth1'); + } @@ - if (request.auth.mode === 'oauth1') { - axiosRequest.oauth1config = { - consumerKey: get(request, 'auth.oauth1.consumerKey'), - consumerSecret: get(request, 'auth.oauth1.consumerSecret'), - accessToken: get(request, 'auth.oauth1.accessToken'), - tokenSecret: get(request, 'auth.oauth1.tokenSecret'), - callbackUrl: get(request, 'auth.oauth1.callbackUrl'), - verifier: get(request, 'auth.oauth1.verifier'), - signatureMethod: get(request, 'auth.oauth1.signatureMethod'), - privateKey: get(request, 'auth.oauth1.privateKey'), - privateKeyType: get(request, 'auth.oauth1.privateKeyType'), - timestamp: get(request, 'auth.oauth1.timestamp'), - nonce: get(request, 'auth.oauth1.nonce'), - version: get(request, 'auth.oauth1.version'), - realm: get(request, 'auth.oauth1.realm'), - addParamsTo: get(request, 'auth.oauth1.addParamsTo'), - includeBodyHash: get(request, 'auth.oauth1.includeBodyHash') - }; - } + if (request.auth.mode === 'oauth1') { + axiosRequest.oauth1config = buildOAuth1Config(request, 'auth.oauth1'); + }Also applies to: 218-236
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-cli/src/runner/prepare-request.js` around lines 152 - 170, The OAuth1 mapping is duplicated; extract a helper (e.g., buildOAuth1Config(collectionAuth)) that reads oauth1.* keys using get(...) and returns the config object, then replace both occurrences (the branch where collectionAuth.mode === 'oauth1' that assigns axiosRequest.oauth1config and the similar block around lines 218-236) with a call to that helper so both branches use the same builder and avoid drift.packages/bruno-tests/src/auth/oauth1/index.js (2)
244-250: RedundantalgoMapmapping.The map keys and values are identical. You can simplify by using
signatureMethoddirectly.♻️ Simplify signature verification
case 'RSA-SHA1': case 'RSA-SHA256': case 'RSA-SHA512': { - const algoMap = { 'RSA-SHA1': 'RSA-SHA1', 'RSA-SHA256': 'RSA-SHA256', 'RSA-SHA512': 'RSA-SHA512' }; - const verifier = crypto.createVerify(algoMap[signatureMethod]); + const verifier = crypto.createVerify(signatureMethod); verifier.update(baseString); return verifier.verify(TEST_RSA_PUBLIC_KEY, signature, 'base64'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-tests/src/auth/oauth1/index.js` around lines 244 - 250, The RSA branch creates an unnecessary algoMap with identical keys/values; simplify the code by calling crypto.createVerify(signatureMethod) directly, keep the existing verifier.update(baseString) and return verifier.verify(TEST_RSA_PUBLIC_KEY, signature, 'base64') so the logic around signatureMethod, baseString, TEST_RSA_PUBLIC_KEY, and verifier.verify remains unchanged but without the redundant mapping.
557-559: Prefer named exports or consistent module.exports pattern.The current pattern assigns to
module.exportsthen adds a property. Consider using a single object export for clarity.♻️ Consolidate exports
-module.exports = router; -module.exports.TEST_RSA_PRIVATE_KEY = TEST_RSA_PRIVATE_KEY; +module.exports = router; +module.exports.TEST_RSA_PRIVATE_KEY = TEST_RSA_PRIVATE_KEY;Alternatively, use a cleaner pattern:
router.TEST_RSA_PRIVATE_KEY = TEST_RSA_PRIVATE_KEY; module.exports = router;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-tests/src/auth/oauth1/index.js` around lines 557 - 559, The file currently assigns module.exports = router and then sets module.exports.TEST_RSA_PRIVATE_KEY separately; fix this by exporting consistently with a single object or attaching the key to the router before exporting. Update the code so either router.TEST_RSA_PRIVATE_KEY = TEST_RSA_PRIVATE_KEY and then module.exports = router, or replace both lines with module.exports = { router, TEST_RSA_PRIVATE_KEY } (choosing the pattern used elsewhere), ensuring references to router and TEST_RSA_PRIVATE_KEY remain valid.tests/auth/oauth1/oauth1-runner.spec.ts (1)
13-21: Potential race condition when writing PEM file at module load.If multiple test workers load this module simultaneously, concurrent
existsSync/writeFileSynccalls could race. Consider usingwriteFileSyncwith a flag like{ flag: 'wx' }to fail if file exists, or move setup to a fixture/hook.♻️ Safer file write
for (const subdir of ['bru', 'yml']) { const pemPath = path.join(fixtureBase, subdir, 'test-private-key.pem'); - if (!fs.existsSync(pemPath)) { - fs.writeFileSync(pemPath, TEST_RSA_PRIVATE_KEY); - } + try { + fs.writeFileSync(pemPath, TEST_RSA_PRIVATE_KEY, { flag: 'wx' }); + } catch (err) { + // File already exists, which is fine + if (err.code !== 'EEXIST') throw err; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auth/oauth1/oauth1-runner.spec.ts` around lines 13 - 21, The module-level setup that writes TEST_RSA_PRIVATE_KEY to pemPath can race across parallel test workers; update the logic around TEST_RSA_PRIVATE_KEY/pemPath to perform an atomic create-or-ignore instead of existsSync then writeFileSync: attempt to write using writeFileSync with a failing-when-exists flag (e.g., { flag: 'wx' }) wrapped in a try/catch that ignores EEXIST, or move the write into a test fixture/hook that runs single-threaded before tests; ensure you reference and update the code touching TEST_RSA_PRIVATE_KEY and pemPath in oauth1-runner.spec.ts so concurrent loads don't clobber each other.packages/bruno-app/src/components/RequestPane/Auth/OAuth1/index.js (2)
103-105: Usepath.basenamefor cross-platform filename extraction.Manual splitting with
/and\\works but the codebase already imports apathutility. Usingpath.basenameis more robust and consistent with OS-agnostic guidelines.♻️ Suggested fix
- const fileName = isFileRef ? privateKeyValue.split('/').pop().split('\\').pop() : ''; + const fileName = isFileRef ? path.basename(privateKeyValue) : '';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RequestPane/Auth/OAuth1/index.js` around lines 103 - 105, Replace the manual split logic that derives fileName from privateKeyValue with a cross-platform basename call: when computing fileName (currently using privateKeyValue.split('/').pop().split('\\').pop()), import/use the existing path utility and call path.basename(privateKeyValue) when isFileRef is true (keep privateKeyValue and isFileRef checks as-is) so filename extraction is robust across OSes.
287-304: Potential double-toggle on checkbox interaction.The
<input>hasonChangeand the<label>hasonClickwithe.preventDefault()— but clicking the checkbox directly still triggersonChange. The label'sonClickalso fires when clicking the checkbox (due to label association), butpreventDefault()on the label doesn't prevent the checkbox's native change. This should work correctly, but the pattern is unusual.A cleaner approach: wrap both in a standard
<label>and rely solely ononChange.♻️ Simplified pattern
- <div className="flex items-center gap-2"> - <input - type="checkbox" - checked={oauth1.includeBodyHash || false} - onChange={(e) => handleChange('includeBodyHash', e.target.checked)} - /> - <label - className="block cursor-pointer" - onClick={(e) => { - e.preventDefault(); handleChange('includeBodyHash', !oauth1.includeBodyHash); - }} - > - Include Body Hash - </label> - </div> + <label className="flex items-center gap-2 cursor-pointer"> + <input + type="checkbox" + checked={oauth1.includeBodyHash || false} + onChange={(e) => handleChange('includeBodyHash', e.target.checked)} + /> + <span>Include Body Hash</span> + </label>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RequestPane/Auth/OAuth1/index.js` around lines 287 - 304, The checkbox currently handles toggling in two places (input onChange and label onClick) which can double-trigger; simplify by using a single control: remove the label's onClick and e.preventDefault(), wrap the <input> and text inside one <label> (or associate via htmlFor/id) and rely only on the input's onChange to call handleChange('includeBodyHash', e.target.checked); reference oauth1.includeBodyHash and the handleChange call to locate where to change this.tests/auth/oauth1/oauth1.spec.ts (1)
127-134: Consider a more specific checkbox selector.
page.locator('input[type="checkbox"]')could match multiple checkboxes if more are added to the OAuth1 form. Consider scoping it to the "Include Body Hash" row.♻️ Suggested selector refinement
- const checkbox = page.locator('input[type="checkbox"]'); - const bodyHashLabel = page.locator('label').filter({ hasText: 'Include Body Hash' }); + const bodyHashRow = fieldRow(page, 'Include Body Hash'); + const checkbox = bodyHashRow.locator('input[type="checkbox"]');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auth/oauth1/oauth1.spec.ts` around lines 127 - 134, The selector for the checkbox is too broad and may match multiple inputs; scope the checkbox lookup to the "Include Body Hash" row by finding the input relative to the bodyHashLabel instead of a global input[type="checkbox"]. Replace the standalone checkbox locator with something like using bodyHashLabel.locator('input[type="checkbox"]') or bodyHashLabel.locator('input') (or use a selector that targets the row/container for the "Include Body Hash" label) so the test uses the checkbox tied to the bodyHashLabel variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-app/src/components/RequestPane/WSRequestPane/WSAuth/index.js`:
- Around line 82-92: The oauth1 UI branch in the WSAuth component is never
reachable because supportedAuthModes (checked in the useEffect that resets mode)
does not include 'oauth1' and thus the effect immediately sets mode to 'none';
fix by either adding 'oauth1' to the supportedAuthModes array or changing the
useEffect logic in WSAuth (the effect that compares current mode against
supportedAuthModes and calls setMode('none')) to preserve 'oauth1' when the
component renders the oauth1 case (or only reset when mode is truly
unsupported), so the oauth1 case in the switch can actually persist and display.
In `@packages/bruno-app/src/utils/codegenerator/auth.js`:
- Around line 58-69: The current function returns an Authorization header string
containing literal placeholders ("<signature>", "<timestamp>", "<nonce>") which
produces invalid OAuth1 auth; update the logic in the auth generation path
(where oauth1Config is read and consumerKey, token, signatureMethod,
oauthVersion are defined) to either (A) compute a proper OAuth1 signature and
runtime oauth_timestamp and oauth_nonce for the cloned request and inject those
concrete values into the Authorization value, or (B) mark OAuth1 unsupported by
returning enabled: false (or an explicit "Unsupported: OAuth1 requires runtime
signing" value) instead of emitting a misleading template; modify the code that
builds the header object so Authorization never contains raw placeholder tokens.
In `@packages/bruno-cli/src/runner/interpolate-vars.js`:
- Around line 280-293: The oauth1config.privateKey field may be an object (RSA
config) whose nested string properties need interpolation; update the
interpolation step for request.oauth1config.privateKey to detect if it's an
object and recursively walk its properties, applying _interpolate to every
string leaf (preserving non-string values), otherwise fall back to
_interpolate(... ) || ''. Modify the block handling
request.oauth1config.privateKey to call a small helper that uses the existing
_interpolate function to transform nested string fields in-place (or return the
interpolated value) so nested {{...}} templates are resolved before signing.
In `@packages/bruno-electron/src/ipc/network/interpolate-vars.js`:
- Around line 364-378: The OAuth1 interpolation block misses interpolating
addParamsTo and includeBodyHash which can change signing behavior; inside the
request.oauth1config handling (where _interpolate is used for consumerKey,
consumerSecret, accessToken, tokenSecret, callbackUrl, verifier,
signatureMethod, privateKey, timestamp, nonce, version, realm) add lines to set
request.oauth1config.addParamsTo =
_interpolate(request.oauth1config.addParamsTo) ||
request.oauth1config.addParamsTo || '' and request.oauth1config.includeBodyHash
= _interpolate(request.oauth1config.includeBodyHash) ||
request.oauth1config.includeBodyHash || '' so both fields respect
variable-driven values and fall back to existing config.
In `@packages/bruno-lang/v2/tests/oauth1.spec.js`:
- Around line 742-775: The test "should survive round-trip with multiline PEM
private key" uses a PEM-like literal in the pem variable that trips scanners;
replace that hardcoded-looking key with a clearly fake marker or generate it at
runtime instead. Update the pem assignment (the pem constant) used in the json
object passed to jsonToBru/bruToJson so it contains a non-sensitive placeholder
(e.g., include "FAKE" or "TEST-KEY" inside the BEGIN/END block) or call a small
helper that constructs a dummy multiline PEM for the test; keep all assertions
(expect(parsed.auth.oauth1.privateKey) and privateKeyType) unchanged. Ensure the
new value still contains line breaks so the round-trip behavior for multiline
strings is exercised.
In `@packages/bruno-requests/src/auth/oauth1-request-authorization.ts`:
- Around line 45-54: The code currently forces oauth_version to '1.0' via a
truthy fallback (e.g. using version || '1.0'), which prevents preserving an
explicitly empty oauth_version and changes the signed parameter set; update the
logic that builds OAuth parameters (the places that reference oauth_version and
any helper like the OAuth1 parameter construction) to only default to '1.0' when
oauth_version is undefined or null (not when it's an empty string), and apply
the same change to the other occurrences that use the truthy fallback so empty
values are preserved in the signed parameters while undefined still falls back
to '1.0'.
- Around line 428-453: The 'body' branch currently overwrites non-form payloads
by initializing URLSearchParams(''), which can drop JSON/raw bodies or add
bodies to GET/HEAD; update the 'body' case in the switch (the code using
isFormUrlEncoded, request.data, URLSearchParams, ctKey and addParamsTo) to first
validate that the request is form-urlencoded and not a GET/HEAD: if
isFormUrlEncoded is false or request.method is GET/HEAD, throw or return an
error indicating OAuth body placement is invalid for non-form requests; only
when isFormUrlEncoded is true should you build URLSearchParams, set request.data
and (if needed) set Content-Type via ctKey or 'Content-Type'.
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` HMAC-SHA1 Body JSON
200.yml:
- Around line 17-21: The fixture currently uses addParamsTo: body together with
body.type: json (and includeBodyHash: false), which can cause OAuth params to be
ignored; update the fixture so that either set addParamsTo to header to sign
JSON payloads via the Authorization header, or change body.type from json to
form (form-encoded) when you intend to exercise body-parameter signing; adjust
the addParamsTo and/or body.type entries accordingly to ensure consistent
body-auth behavior.
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` RSA-SHA1 Query Params
200.yml:
- Around line 18-48: The YAML fixture contains an embedded RSA private key under
the privateKey field which triggers secret scanners; remove the inline PEM and
either (a) replace it with a reference to a test-only key loader (e.g.,
loadPrivateKeyFromEnv or generateTestKeyAtRuntime) so the key is created/loaded
outside source control, or (b) if you must include it in the PR add the
repository secret-scanner allowlist entry for this fixture in the same PR;
update any tests that consume privateKey to read from the new loader/generator
or env variable instead of the inline PEM.
---
Nitpick comments:
In `@packages/bruno-app/src/components/RequestPane/Auth/OAuth1/index.js`:
- Around line 103-105: Replace the manual split logic that derives fileName from
privateKeyValue with a cross-platform basename call: when computing fileName
(currently using privateKeyValue.split('/').pop().split('\\').pop()), import/use
the existing path utility and call path.basename(privateKeyValue) when isFileRef
is true (keep privateKeyValue and isFileRef checks as-is) so filename extraction
is robust across OSes.
- Around line 287-304: The checkbox currently handles toggling in two places
(input onChange and label onClick) which can double-trigger; simplify by using a
single control: remove the label's onClick and e.preventDefault(), wrap the
<input> and text inside one <label> (or associate via htmlFor/id) and rely only
on the input's onChange to call handleChange('includeBodyHash',
e.target.checked); reference oauth1.includeBodyHash and the handleChange call to
locate where to change this.
In `@packages/bruno-cli/src/runner/prepare-request.js`:
- Around line 152-170: The OAuth1 mapping is duplicated; extract a helper (e.g.,
buildOAuth1Config(collectionAuth)) that reads oauth1.* keys using get(...) and
returns the config object, then replace both occurrences (the branch where
collectionAuth.mode === 'oauth1' that assigns axiosRequest.oauth1config and the
similar block around lines 218-236) with a call to that helper so both branches
use the same builder and avoid drift.
In `@packages/bruno-tests/src/auth/oauth1/index.js`:
- Around line 244-250: The RSA branch creates an unnecessary algoMap with
identical keys/values; simplify the code by calling
crypto.createVerify(signatureMethod) directly, keep the existing
verifier.update(baseString) and return verifier.verify(TEST_RSA_PUBLIC_KEY,
signature, 'base64') so the logic around signatureMethod, baseString,
TEST_RSA_PUBLIC_KEY, and verifier.verify remains unchanged but without the
redundant mapping.
- Around line 557-559: The file currently assigns module.exports = router and
then sets module.exports.TEST_RSA_PRIVATE_KEY separately; fix this by exporting
consistently with a single object or attaching the key to the router before
exporting. Update the code so either router.TEST_RSA_PRIVATE_KEY =
TEST_RSA_PRIVATE_KEY and then module.exports = router, or replace both lines
with module.exports = { router, TEST_RSA_PRIVATE_KEY } (choosing the pattern
used elsewhere), ensuring references to router and TEST_RSA_PRIVATE_KEY remain
valid.
In `@tests/auth/oauth1/fixtures/collections/bru/OAuth1` PLAINTEXT Query Params
200.bru:
- Around line 29-32: Add an email assertion to the existing response assertions:
inside the assert block that currently checks res.status and
res.body.resource.name (the assertions referencing res.status and
res.body.resource.name: eq oauth1-test-resource), add a line asserting
res.body.resource.email: eq oauth1@example.com so this PLAINTEXT fixture matches
the parity of the HMAC-SHA1 200.bru fixture.
In `@tests/auth/oauth1/fixtures/collections/bru/OAuth1` RSA-SHA1 Body 200.bru:
- Around line 20-49: The RSA PEM private_key block in the OAuth1 RSA-SHA1
fixture is duplicated; extract the PEM into a single shared fixture (e.g., a
shared "rsa_private_key" fixture) and replace the inline private_key value in
OAuth1 RSA-SHA1 Body 200.bru with a reference to that shared fixture; update any
other OAuth1 RSA fixtures to reference the same shared symbol so all tests
consume the centralized key and eliminate copy/paste drift.
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` HMAC-SHA1 Body JSON
200.yml:
- Around line 11-14: Replace the realistic-looking credential values in the
fixture by using clearly fake placeholder literals for the keys consumerKey,
consumerSecret, accessToken, and tokenSecret (for example values like
"fake_consumer_key", "fake_consumer_secret", "fake_access_token",
"fake_token_secret") so the test remains meaningful but won't trigger secret
scanners; update the corresponding entries in the OAuth1 HMAC-SHA1 Body JSON
fixture to use those fake placeholders.
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` HMAC-SHA256 Body 200.yml:
- Around line 22-30: The 200 fixture is missing the email assertion for
consistency with other OAuth1 200 fixtures; add an assertion entry under
runtime.assertions that checks res.body.resource.email equals oauth1@example.com
by inserting an assertion with expression: res.body.resource.email, operator:
eq, and value: oauth1@example.com (matching the existing assertion style used
for res.body.resource.name).
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` PLAINTEXT Query Params
200.yml:
- Around line 20-28: The fixture is missing an assertion for resource.email;
update the runtime.assertions array in the OAuth1 PLAINTEXT Query Params 200.yml
fixture (alongside the existing assertions that check res.status and
res.body.resource.name) by adding an assertion that checks expression
"res.body.resource.email" with operator "eq" and the expected email value used
across other OAuth1 200 fixtures (e.g., "oauth1-test@example.com"), placing it
after the resource.name assertion for consistency.
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` RSA-SHA1 200.yml:
- Around line 9-17: Add the missing consumerSecret field to the auth block so
the fixture matches the schema used by other OAuth1 RSA-SHA1 fixtures; update
the auth map by adding consumerSecret: '' alongside consumerKey, accessToken,
tokenSecret, signatureMethod, version, addParamsTo, and includeBodyHash to
ensure consistent schema coverage.
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` RSA-SHA1 Body 200.yml:
- Around line 53-61: This fixture's runtime assertions only check
res.body.resource.name but should also assert res.body.resource.email for
consistency with other OAuth1 200 fixtures; update the assertions array in the
YAML (the block containing the expressions "res.body.resource.name" and
"res.status") to add a new assertion whose expression is
"res.body.resource.email", operator "eq", and value matching the expected test
email used elsewhere in OAuth1 fixtures (use the same email literal used by
HMAC-SHA1/RSA-SHA1 fixtures) so the test verifies both name and email.
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` RSA-SHA512 200.yml:
- Around line 20-48: This fixture contains an inline PEM private key (the
multi-line "-----BEGIN PRIVATE KEY-----" value in the OAuth1 RSA-SHA512 200.yml
fixture) used only for tests; add a narrowly scoped secret-scan allowlist entry
in your CI secret-scanning config (e.g., gitleaks/checkov rules) that whitelists
this fixture directory or filename pattern and/or the specific YAML "value"
field pattern to suppress false positives, ensuring the rule is limited to
tests/auth/oauth1 fixtures and does not broadly exempt other files.
In `@tests/auth/oauth1/oauth1-runner.spec.ts`:
- Around line 13-21: The module-level setup that writes TEST_RSA_PRIVATE_KEY to
pemPath can race across parallel test workers; update the logic around
TEST_RSA_PRIVATE_KEY/pemPath to perform an atomic create-or-ignore instead of
existsSync then writeFileSync: attempt to write using writeFileSync with a
failing-when-exists flag (e.g., { flag: 'wx' }) wrapped in a try/catch that
ignores EEXIST, or move the write into a test fixture/hook that runs
single-threaded before tests; ensure you reference and update the code touching
TEST_RSA_PRIVATE_KEY and pemPath in oauth1-runner.spec.ts so concurrent loads
don't clobber each other.
In `@tests/auth/oauth1/oauth1.spec.ts`:
- Around line 127-134: The selector for the checkbox is too broad and may match
multiple inputs; scope the checkbox lookup to the "Include Body Hash" row by
finding the input relative to the bodyHashLabel instead of a global
input[type="checkbox"]. Replace the standalone checkbox locator with something
like using bodyHashLabel.locator('input[type="checkbox"]') or
bodyHashLabel.locator('input') (or use a selector that targets the row/container
for the "Include Body Hash" label) so the test uses the checkbox tied to the
bodyHashLabel variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7f137db4-7acc-478d-8726-4950f375cce5
📒 Files selected for processing (93)
package.jsonpackages/bruno-app/src/components/CollectionSettings/Auth/AuthMode/index.jspackages/bruno-app/src/components/CollectionSettings/Auth/Oauth1/index.jspackages/bruno-app/src/components/CollectionSettings/Auth/index.jspackages/bruno-app/src/components/FolderSettings/Auth/index.jspackages/bruno-app/src/components/FolderSettings/AuthMode/index.jspackages/bruno-app/src/components/RequestPane/Auth/AuthMode/index.jspackages/bruno-app/src/components/RequestPane/Auth/OAuth1/StyledWrapper.jspackages/bruno-app/src/components/RequestPane/Auth/OAuth1/index.jspackages/bruno-app/src/components/RequestPane/Auth/index.jspackages/bruno-app/src/components/RequestPane/WSRequestPane/WSAuth/index.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/index.jspackages/bruno-app/src/utils/codegenerator/auth.jspackages/bruno-app/src/utils/collections/index.jspackages/bruno-cli/src/runner/interpolate-vars.jspackages/bruno-cli/src/runner/prepare-request.jspackages/bruno-cli/src/runner/run-single-request.jspackages/bruno-converters/src/opencollection/common/auth.tspackages/bruno-converters/src/opencollection/types.tspackages/bruno-converters/src/postman/postman-to-bruno.jspackages/bruno-electron/src/ipc/network/index.jspackages/bruno-electron/src/ipc/network/interpolate-vars.jspackages/bruno-electron/src/ipc/network/prepare-grpc-request.jspackages/bruno-electron/src/ipc/network/prepare-request.jspackages/bruno-filestore/src/formats/yml/common/auth.tspackages/bruno-js/src/bruno-request.jspackages/bruno-lang/v2/src/bruToJson.jspackages/bruno-lang/v2/src/collectionBruToJson.jspackages/bruno-lang/v2/src/jsonToBru.jspackages/bruno-lang/v2/src/jsonToCollectionBru.jspackages/bruno-lang/v2/tests/oauth1.spec.jspackages/bruno-requests/package.jsonpackages/bruno-requests/src/auth/index.tspackages/bruno-requests/src/auth/oauth1-request-authorization.spec.tspackages/bruno-requests/src/auth/oauth1-request-authorization.tspackages/bruno-requests/src/index.tspackages/bruno-schema-types/src/common/auth.tspackages/bruno-schema/src/collections/index.jspackages/bruno-tests/src/auth/index.jspackages/bruno-tests/src/auth/oauth1/index.jsplaywright.config.tstests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 401.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 Body 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 Body JSON 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 POST 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 Query Params 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA256 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA256 401.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA256 Body 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA512 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA512 401.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT 401.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT Body 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT Query Params 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 Body 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 File Key 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 Query Params 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 Variable Key 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA256 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA512 200.brutests/auth/oauth1/fixtures/collections/bru/bruno.jsontests/auth/oauth1/fixtures/collections/bru/environments/Local.brutests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 401.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Body 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Body JSON 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 POST 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Query Params 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 401.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 Body 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA512 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA512 401.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT 401.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT Body 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT Query Params 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Body 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 File Key 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Query Params 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Variable Key 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA256 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA512 200.ymltests/auth/oauth1/fixtures/collections/yml/environments/Local.ymltests/auth/oauth1/fixtures/collections/yml/opencollection.ymltests/auth/oauth1/init-user-data/preferences.jsontests/auth/oauth1/oauth1-runner.spec.tstests/auth/oauth1/oauth1.spec.tstests/utils/page/actions.ts
Comment on lines
+428
to
+453
| switch (addParamsTo || 'header') { | ||
| case 'header': | ||
| request.headers['Authorization'] = authorizer.toHeader(oauthData).Authorization; | ||
| break; | ||
| case 'queryparams': { | ||
| const url = new URL(request.url); | ||
| Object.entries(oauthData).forEach(([key, value]) => { | ||
| if (value) url.searchParams.set(key, value); | ||
| }); | ||
| request.url = url.toString(); | ||
| break; | ||
| } | ||
| case 'body': { | ||
| const params = new URLSearchParams(isFormUrlEncoded ? request.data : ''); | ||
| Object.entries(oauthData).forEach(([key, value]) => { | ||
| if (value !== undefined) params.set(key, value); | ||
| }); | ||
| request.data = params.toString(); | ||
|
|
||
| if (!isFormUrlEncoded) { | ||
| if (ctKey) { | ||
| request.headers[ctKey] = 'application/x-www-form-urlencoded'; | ||
| } else { | ||
| request.headers['Content-Type'] = 'application/x-www-form-urlencoded'; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Reject body placement for non-form requests instead of rewriting the payload.
Line 441 initializes URLSearchParams from '' whenever the request is not form-urlencoded, so JSON/raw payloads are discarded and even GET/HEAD requests can gain a synthetic form body. This should fail fast instead of silently mutating the request into something else.
🛠 Suggested fix
case 'body': {
- const params = new URLSearchParams(isFormUrlEncoded ? request.data : '');
+ if (!hasBody || !isFormUrlEncoded) {
+ throw new Error('OAuth1 body placement requires a non-GET/HEAD application/x-www-form-urlencoded request');
+ }
+
+ const params = new URLSearchParams(typeof request.data === 'string' ? request.data : '');
Object.entries(oauthData).forEach(([key, value]) => {
if (value !== undefined) params.set(key, value);
});
request.data = params.toString();
-
- if (!isFormUrlEncoded) {
- if (ctKey) {
- request.headers[ctKey] = 'application/x-www-form-urlencoded';
- } else {
- request.headers['Content-Type'] = 'application/x-www-form-urlencoded';
- }
- }
break;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-requests/src/auth/oauth1-request-authorization.ts` around
lines 428 - 453, The 'body' branch currently overwrites non-form payloads by
initializing URLSearchParams(''), which can drop JSON/raw bodies or add bodies
to GET/HEAD; update the 'body' case in the switch (the code using
isFormUrlEncoded, request.data, URLSearchParams, ctKey and addParamsTo) to first
validate that the request is form-urlencoded and not a GET/HEAD: if
isFormUrlEncoded is false or request.method is GET/HEAD, throw or return an
error indicating OAuth body placement is invalid for non-form requests; only
when isFormUrlEncoded is true should you build URLSearchParams, set request.data
and (if needed) set Content-Type via ctKey or 'Content-Type'.
Comment on lines
+17
to
+21
| addParamsTo: body | ||
| includeBodyHash: false | ||
| body: | ||
| type: json | ||
| json: '{"test":"data"}' |
Contributor
There was a problem hiding this comment.
addParamsTo: body with type: json is a risky auth-placement combination.
This can lead to OAuth params being ignored or injected in a way that doesn’t reliably validate body-auth behavior. Prefer signing via header for JSON payload fixtures, or switch body type to form-encoded for body-param tests.
Suggested fixture adjustment (JSON body case)
- addParamsTo: body
+ addParamsTo: header
includeBodyHash: false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` HMAC-SHA1 Body JSON
200.yml around lines 17 - 21, The fixture currently uses addParamsTo: body
together with body.type: json (and includeBodyHash: false), which can cause
OAuth params to be ignored; update the fixture so that either set addParamsTo to
header to sign JSON payloads via the Authorization header, or change body.type
from json to form (form-encoded) when you intend to exercise body-parameter
signing; adjust the addParamsTo and/or body.type entries accordingly to ensure
consistent body-auth behavior.
sanish-bruno
requested changes
Mar 17, 2026
| bearer: null, | ||
| digest: null, | ||
| ntlm: null, | ||
| oauth2: null, |
Collaborator
There was a problem hiding this comment.
@lohit-bruno don't we have to set oauth1 to null here
| bearer: null, | ||
| digest: null, | ||
| ntlm: null, | ||
| oauth2: null, |
Collaborator
There was a problem hiding this comment.
@lohit-bruno don't er have to default oauth1 to null, like we are doing for oauth2?
| }); | ||
|
|
||
| test('Verify Add Params To placement via timeline', async ({ pageWithUserData: page }) => { | ||
| test.setTimeout(3 * 60 * 1000); |
Collaborator
There was a problem hiding this comment.
@lohit-bruno we can case the value to a constant, and reuse it everywhere
| }; | ||
|
|
||
| const selectRequestPaneTab = async (page: Page, tabName: string) => { | ||
| await selectPaneTab(page, '.request-pane > .px-4', tabName); |
Collaborator
There was a problem hiding this comment.
@lohit-bruno is it possible to add a better selector for request and response panes? may be we can use testIds
| const result = collectionBruToJson(input); | ||
|
|
||
| expect(result.auth.oauth1.privateKeyType).toBe('text'); | ||
| expect(result.auth.oauth1.privateKey).toContain('-----BEGIN FAKE RSA TEST KEY-----'); |
Collaborator
There was a problem hiding this comment.
@lohit-bruno should we also assert with the entire key itself? just to make sure than the initial spaces are not included in the final key.
sanish-bruno
left a comment
sanish-bruno
left a comment
Collaborator
There was a problem hiding this comment.
PR Review
Found 5 issue(s): 0 major, 2 minor, 1 suggestion, 2 nits.
Overall this is a very well-structured PR. The OAuth 1.0 implementation follows RFC 5849 carefully, has comprehensive unit and E2E tests covering all signature methods and parameter placements, and follows the existing patterns in the codebase for adding new auth types. The Bru format serialization/deserialization and round-trip tests are thorough.
Summary
- Test fixture
OAuth1 RSA-SHA1 200.brucontains what appears to be accidental debug data invars:pre-request - Debug
console.logleft inOAuth1 HMAC-SHA1 Body 200.brufixture - Module-level
privateKeyCachehas no eviction strategy - Silent input rejection in
handlePrivateKeyChangefor@file(values lacks user feedback - Good incidental fix of missing
breakin NTLM case in prepare-request.js
| include_body_hash: false | ||
| } | ||
|
|
||
| vars:pre-request { |
Collaborator
There was a problem hiding this comment.
Nit: This vars:pre-request block has an empty variable name and a ~3KB repeating string of token_secret_1. This looks like accidentally pasted debug data. The variable is never used (no name) and adds unnecessary noise to the fixture.
Suggested fix: Remove the entire vars:pre-request block.
|
|
||
| script:post-response { | ||
| console.log(req.getBody()); | ||
| } |
Collaborator
There was a problem hiding this comment.
Nit: This console.log(req.getBody()) looks like leftover debug output.
Suggested fix: Remove the script:post-response block, or replace with a meaningful assertion.
| import crypto from 'node:crypto'; | ||
| import fs from 'node:fs'; | ||
| import nodePath from 'node:path'; | ||
|
|
Collaborator
There was a problem hiding this comment.
Suggestion: privateKeyCache is a module-level Map that never evicts entries. In a long-running Electron process, if users switch between collections with different private key files, stale entries accumulate indefinitely. The mtime-based invalidation handles content changes correctly, but entries for deleted or no-longer-used files are never removed.
Consider adding a simple max-size cap (e.g., evict oldest entry when the map exceeds 50 entries), or clearing the cache when a collection is closed.
| }; | ||
|
|
||
| const handlePrivateKeyChange = (val) => { | ||
| if (val && /^@file\(/.test(val.trim())) return; |
Collaborator
There was a problem hiding this comment.
Bug (Minor): This silently swallows any value starting with @file(, giving the user no feedback that their input was rejected. This is necessary to prevent @file() markers from being entered via the text editor (since they're a serialization-level construct), but the silent return is confusing UX.
Suggested fix: Consider showing a brief toast or visual indicator explaining that file references should use the "Upload File" button instead.
| password: get(request, 'auth.ntlm.password'), | ||
| domain: get(request, 'auth.ntlm.domain') | ||
| }; | ||
| break; |
Collaborator
There was a problem hiding this comment.
Bug (Minor): Good catch — this break was missing from the pre-existing ntlm case, causing it to fall through to the next case. Worth calling out in the PR description as an incidental fix, since it affects existing NTLM auth behavior (the fall-through would have caused the oauth1 case to also execute after ntlm).
sanish-bruno
requested changes
Mar 17, 2026
| password: authValues.password || '' | ||
| }; | ||
| break; | ||
| case AUTH_TYPES.OAUTH1: |
Collaborator
There was a problem hiding this comment.
@lohit-bruno can we have few test cases that verify the oauth1 import from postman is working as expected
sreelakshmi-bruno
requested changes
Mar 17, 2026
Collaborator
There was a problem hiding this comment.
When using OAuth 1.0 authentication with addParamsTo: body and a GET request with a form-urlencoded body, the body parameters are not included in the signature calculation, but they ARE sent in the request body. This causes signature verification to fail on the server.
Steps to Reproduce
-
Create a request with:
- Method:
GET - Body: Form URL Encoded with
foo=bar - Auth: OAuth 1.0
- Add Params To:
Body
- Method:
-
Send the request to an OAuth 1.0 protected endpoint
-
Observe 401 Unauthorized response
Expected Behavior
Body parameters (foo=bar) should be included in the OAuth signature base string when addParamsTo: body is selected, regardless of HTTP method. Since Bruno sends the body even for GET requests, the signature should include those parameters.
sreelakshmi-bruno
requested changes
Mar 17, 2026
sreelakshmi-bruno
left a comment
sreelakshmi-bruno
left a comment
Collaborator
There was a problem hiding this comment.
Can you also add this in Bruno to Postman conversion?
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bruno-converters/src/opencollection/common/auth.ts (1)
46-78:⚠️ Potential issue | 🟡 MinorMissing
oauth1: nullinbuildOAuth2Config.This function constructs a
BrunoAuthobject but omits theoauth1field. For consistency withdefaultAuth(line 171-182) and the default case infromOpenCollectionOAuth2(line 155-166), addoauth1: null.Proposed fix
const buildOAuth2Config = (base: Partial<BrunoOAuth2>): BrunoAuth => { const brunoAuth: BrunoAuth = { mode: 'oauth2', awsv4: null, basic: null, bearer: null, digest: null, ntlm: null, + oauth1: null, oauth2: { grantType: base.grantType || 'client_credentials', ... }, wsse: null, apikey: null }; return brunoAuth; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-converters/src/opencollection/common/auth.ts` around lines 46 - 78, The BrunoAuth object built by buildOAuth2Config is missing the oauth1 field; update buildOAuth2Config to include oauth1: null alongside the other auth keys so the constructed BrunoAuth matches defaultAuth and the fromOpenCollectionOAuth2 default case.
♻️ Duplicate comments (2)
tests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Query Params 200.yml (1)
18-45:⚠️ Potential issue | 🟠 MajorDo not keep inline PEM private key in committed fixture.
Line 18 through Line 45 embeds private key material, which will repeatedly trigger secret scanners and creates avoidable security/process risk. Please move it to test runtime input (env/file generated in setup) or add an explicit repository allowlist entry in this PR if this is intentionally committed test data.
If you want, I can draft a follow-up patch to switch this fixture to a runtime-injected key path/value.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` RSA-SHA1 Query Params 200.yml around lines 18 - 45, The fixture "OAuth1 RSA-SHA1 Query Params 200.yml" contains an inline PEM private key between the BEGIN/END markers; remove the embedded key and change the fixture to reference an external secret (e.g., a placeholder like {{RSA_PRIVATE_KEY_PATH}} or {{RSA_PRIVATE_KEY_PEM}}) and update the test setup code that loads this fixture to inject the real key at runtime from an environment variable or a generated temporary file (set/read via TEST_RSA_KEY_PATH or TEST_RSA_KEY_PEM in your test bootstrap), or if this key must remain, add an explicit repository allowlist entry in the PR metadata; ensure any code that previously parsed the inline PEM (tests/helpers or fixture-loader) now reads the key from the injected source.tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Body JSON 200.yml (1)
10-21:⚠️ Potential issue | 🟠 Major
body.type: json+addParamsTo: bodyis a risky OAuth1 combination.Line 10 and Line 20 pair a JSON payload with body-placed OAuth params, which is not the standard OAuth1 entity-body pattern and can hide interoperability bugs.
Suggested adjustment
body: - type: json - data: "" + type: form-urlencoded + data: + - name: test + value: test @@ - addParamsTo: body + addParamsTo: bodyOr keep JSON and move OAuth params to header:
- addParamsTo: body + addParamsTo: header🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` HMAC-SHA1 Body JSON 200.yml around lines 10 - 21, The test fixture mixes a JSON payload (data.type or body.type set to json) with OAuth1 params placed in the request body (auth.addParamsTo: body), which is nonstandard; update the fixture by either changing body.type from json to form-urlencoded (application/x-www-form-urlencoded) so OAuth params in the body are correct, or keep body.type: json and set auth.addParamsTo: header so OAuth params are sent in the Authorization header; adjust the auth.addParamsTo, body.type (or data/type) and keep auth.signatureMethod, auth.consumerKey, auth.consumerSecret, auth.accessToken, auth.tokenSecret, and auth.includeBodyHash unchanged.
🧹 Nitpick comments (3)
tests/utils/page/actions.ts (1)
789-791: Add an explicit visibility wait before clicking overflow tab items.Direct click on
dropdownItemcan race with tooltip/dropdown rendering in slower CI runs.Proposed stabilization
- const dropdownItem = page.locator('.tippy-box .dropdown-item').filter({ hasText: tabName }); - await dropdownItem.click(); + const dropdownItem = page.locator('.tippy-box .dropdown-item').filter({ hasText: tabName }); + await dropdownItem.waitFor({ state: 'visible' }); + await dropdownItem.click();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/utils/page/actions.ts` around lines 789 - 791, The click on the locator created as dropdownItem (page.locator('.tippy-box .dropdown-item').filter({ hasText: tabName })) can race with slow tooltip/dropdown rendering; add an explicit wait for visibility on that locator before clicking (e.g., wait for state 'visible' or use the test expect toBeVisible) so the dropdown is rendered and stable before invoking dropdownItem.click().tests/auth/oauth1/oauth1-runner.spec.ts (2)
77-85: Add JSDoc for helper function clarity.Per coding guidelines, adding a brief JSDoc to
openTimelineRequestwould help clarify the return type.♻️ Suggested improvement
/** * After sending a request, switch to the Timeline tab, expand the latest timeline item, * and return locators for the request URL and headers section. + * `@returns` {Promise<Locator>} The expanded timeline item locator */ const openTimelineRequest = async (page) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auth/oauth1/oauth1-runner.spec.ts` around lines 77 - 85, The helper function openTimelineRequest lacks a JSDoc comment describing its purpose and return type; add a brief JSDoc above the openTimelineRequest function that states what it does (selects the "Timeline" tab, expands the latest timeline item) and documents the return type (e.g., returns a Playwright Locator for the timeline item) and parameter (page) so callers understand its contract and usage.
13-13: Prefer ES import over require().Using
require()here works but is inconsistent with the ES imports at the top of the file.♻️ Suggested refactor
-const { TEST_RSA_PRIVATE_KEY } = require('../../../packages/bruno-tests/src/auth/oauth1'); +import { TEST_RSA_PRIVATE_KEY } from '../../../packages/bruno-tests/src/auth/oauth1';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auth/oauth1/oauth1-runner.spec.ts` at line 13, Replace the CommonJS require usage for TEST_RSA_PRIVATE_KEY with an ES import to match the file's other imports: change the line that defines const { TEST_RSA_PRIVATE_KEY } = require('../../../packages/bruno-tests/src/auth/oauth1'); to use an ES named import (import { TEST_RSA_PRIVATE_KEY } from '...') and ensure the module specifier matches the original path; update any TypeScript/ESM config (if needed) so named imports are allowed and run tests to confirm no runtime import semantics changed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-lang/v2/tests/oauth1.spec.js`:
- Around line 616-681: Add a test exercising the collection-level text private
key path for jsonToCollectionBru: create a new it() in the "OAuth1
jsonToCollectionBru (collection/folder-level)" suite that sets
oauth1.privateKeyType = 'text' and oauth1.privateKey to a multi-line PEM string
(include the typical -----BEGIN/END RSA PRIVATE KEY----- headers and newlines)
and call jsonToCollectionBru(json); assert the resulting bru contains the
serialized private_key in the triple-quoted/text form the serializer emits
(e.g., private_key represented as a multiline string), so both the
text/privateKey branch and PEM quoting behavior are covered.
In `@tests/auth/oauth1/fixtures/collections/bru/OAuth1` RSA-SHA1 Body 200
formurlencoded.bru:
- Around line 1-5: The fixture "OAuth1 RSA-SHA1 Body formurlencoded 200" has a
duplicate sequence number (seq: 20) that conflicts with "OAuth1 RSA-SHA1 Body
200.bru"; open the meta block in this fixture and assign a unique seq value
(e.g., increment or pick an unused integer) so the meta.name "OAuth1 RSA-SHA1
Body formurlencoded 200" no longer shares seq: 20 with the other fixture to
prevent nondeterministic ordering.
---
Outside diff comments:
In `@packages/bruno-converters/src/opencollection/common/auth.ts`:
- Around line 46-78: The BrunoAuth object built by buildOAuth2Config is missing
the oauth1 field; update buildOAuth2Config to include oauth1: null alongside the
other auth keys so the constructed BrunoAuth matches defaultAuth and the
fromOpenCollectionOAuth2 default case.
---
Duplicate comments:
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` HMAC-SHA1 Body JSON
200.yml:
- Around line 10-21: The test fixture mixes a JSON payload (data.type or
body.type set to json) with OAuth1 params placed in the request body
(auth.addParamsTo: body), which is nonstandard; update the fixture by either
changing body.type from json to form-urlencoded
(application/x-www-form-urlencoded) so OAuth params in the body are correct, or
keep body.type: json and set auth.addParamsTo: header so OAuth params are sent
in the Authorization header; adjust the auth.addParamsTo, body.type (or
data/type) and keep auth.signatureMethod, auth.consumerKey, auth.consumerSecret,
auth.accessToken, auth.tokenSecret, and auth.includeBodyHash unchanged.
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` RSA-SHA1 Query Params
200.yml:
- Around line 18-45: The fixture "OAuth1 RSA-SHA1 Query Params 200.yml" contains
an inline PEM private key between the BEGIN/END markers; remove the embedded key
and change the fixture to reference an external secret (e.g., a placeholder like
{{RSA_PRIVATE_KEY_PATH}} or {{RSA_PRIVATE_KEY_PEM}}) and update the test setup
code that loads this fixture to inject the real key at runtime from an
environment variable or a generated temporary file (set/read via
TEST_RSA_KEY_PATH or TEST_RSA_KEY_PEM in your test bootstrap), or if this key
must remain, add an explicit repository allowlist entry in the PR metadata;
ensure any code that previously parsed the inline PEM (tests/helpers or
fixture-loader) now reads the key from the injected source.
---
Nitpick comments:
In `@tests/auth/oauth1/oauth1-runner.spec.ts`:
- Around line 77-85: The helper function openTimelineRequest lacks a JSDoc
comment describing its purpose and return type; add a brief JSDoc above the
openTimelineRequest function that states what it does (selects the "Timeline"
tab, expands the latest timeline item) and documents the return type (e.g.,
returns a Playwright Locator for the timeline item) and parameter (page) so
callers understand its contract and usage.
- Line 13: Replace the CommonJS require usage for TEST_RSA_PRIVATE_KEY with an
ES import to match the file's other imports: change the line that defines const
{ TEST_RSA_PRIVATE_KEY } =
require('../../../packages/bruno-tests/src/auth/oauth1'); to use an ES named
import (import { TEST_RSA_PRIVATE_KEY } from '...') and ensure the module
specifier matches the original path; update any TypeScript/ESM config (if
needed) so named imports are allowed and run tests to confirm no runtime import
semantics changed.
In `@tests/utils/page/actions.ts`:
- Around line 789-791: The click on the locator created as dropdownItem
(page.locator('.tippy-box .dropdown-item').filter({ hasText: tabName })) can
race with slow tooltip/dropdown rendering; add an explicit wait for visibility
on that locator before clicking (e.g., wait for state 'visible' or use the test
expect toBeVisible) so the dropdown is rendered and stable before invoking
dropdownItem.click().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c89f5664-c2b0-4287-80af-6cc9a2058aea
📒 Files selected for processing (55)
package.jsonpackages/bruno-app/src/components/RequestPane/Auth/OAuth1/index.jspackages/bruno-app/src/components/RequestTabPanel/index.jspackages/bruno-app/src/components/ResponseExample/index.jspackages/bruno-converters/src/opencollection/common/auth.tspackages/bruno-converters/tests/postman/postman-to-bruno/process-auth.spec.jspackages/bruno-electron/src/ipc/network/prepare-request.jspackages/bruno-lang/v2/tests/oauth1.spec.jspackages/bruno-requests/src/auth/oauth1-request-authorization.tstests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 401.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 Body 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 Body JSON 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 POST 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 Query Params 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA256 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA256 401.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA256 Body 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA512 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA512 401.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT 401.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT Body 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT Query Params 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 Body 200 formurlencoded.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 Body 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 File Key 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 Query Params 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 Variable Key 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA256 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA512 200.brutests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 401.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Body 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Body JSON 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 POST 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Query Params 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 401.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 Body 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA512 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA512 401.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT 401.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT Body 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT Query Params 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Body 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Body formurlencoded 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 File Key 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Query Params 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Variable Key 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA256 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA512 200.ymltests/auth/oauth1/oauth1-runner.spec.tstests/utils/page/actions.ts
✅ Files skipped from review due to trivial changes (26)
- packages/bruno-app/src/components/ResponseExample/index.js
- packages/bruno-app/src/components/RequestTabPanel/index.js
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 401.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT 401.bru
- package.json
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 Query Params 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT Body 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA512 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA256 200.bru
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 POST 200.yml
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 Body JSON 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA256 401.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT Query Params 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA512 401.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 200.bru
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT 401.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT Body 200.yml
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 File Key 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA256 Body 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA256 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 POST 200.bru
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Query Params 200.yml
- packages/bruno-converters/tests/postman/postman-to-bruno/process-auth.spec.js
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 Body 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 Query Params 200.bru
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/bruno-app/src/components/RequestPane/Auth/OAuth1/index.js
- packages/bruno-requests/src/auth/oauth1-request-authorization.ts
Comment on lines
+1
to
+5
| meta { | ||
| name: OAuth1 RSA-SHA1 Body formurlencoded 200 | ||
| type: http | ||
| seq: 20 | ||
| } |
Contributor
There was a problem hiding this comment.
Duplicate sequence number.
Both this fixture and OAuth1 RSA-SHA1 Body 200.bru use seq: 20. This may cause non-deterministic ordering. Consider assigning unique sequence numbers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/auth/oauth1/fixtures/collections/bru/OAuth1` RSA-SHA1 Body 200
formurlencoded.bru around lines 1 - 5, The fixture "OAuth1 RSA-SHA1 Body
formurlencoded 200" has a duplicate sequence number (seq: 20) that conflicts
with "OAuth1 RSA-SHA1 Body 200.bru"; open the meta block in this fixture and
assign a unique seq value (e.g., increment or pick an unused integer) so the
meta.name "OAuth1 RSA-SHA1 Body formurlencoded 200" no longer shares seq: 20
with the other fixture to prevent nondeterministic ordering.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
packages/bruno-lang/v2/tests/oauth1.spec.js (1)
616-681:⚠️ Potential issue | 🟡 MinorAdd the collection-level multiline text-key serializer case.
The collection/folder suites still never exercise
jsonToCollectionBru()or collection round-tripping with a populatedprivateKeyType: 'text'. A multiline PEM quoting regression at collection scope would still pass because only the request-level tests cover that branch.As per coding guidelines, 'Add tests for any new functionality or meaningful changes. If code is added, removed, or significantly modified, corresponding tests should be updated or created' and 'Cover both the 'happy path' and the realistically problematic paths.'
Also applies to: 788-850
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-lang/v2/tests/oauth1.spec.js` around lines 616 - 681, Add a collection-level test in the "OAuth1 jsonToCollectionBru (collection/folder-level)" suite to exercise jsonToCollectionBru() when oauth1.privateKeyType === 'text' by providing a multiline PEM string in oauth1.privateKey (mirroring the request-level multiline PEM test) and asserting the resulting bru contains the correctly quoted multiline private_key serialization; this ensures the collection/folder scope exercises the text-key serializer and prevents PEM quoting regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-app/src/components/RequestPane/Auth/OAuth1/index.js`:
- Around line 320-333: Replace the clickable <div> used for the Advanced toggle
with a semantic <button> element (use type="button") so keyboard users can focus
and activate it; keep the onClick handler that calls
setAdvancedOpen(!advancedOpen), preserve the existing className values like
"oauth1-icon-container" and "oauth1-section-label" and the child icons
(IconAdjustmentsHorizontal, IconChevronRight), and add
aria-expanded={advancedOpen} (and optionally aria-controls pointing to the
advanced panel) so screen readers get state information while retaining the
rotate-90 class toggle on IconChevronRight.
- Around line 46-58: The OAuth1 UI displays defaults ("HMAC-SHA1" and "Header")
but those values aren't persisted because dispatches to updateAuth only spread
existing oauth1; update the places that build the new auth content (e.g., the
handleChange function and other blocks that call updateAuth for mode 'oauth1')
to merge in explicit defaults when missing—ensure the content passed to
updateAuth includes signatureMethod: oauth1.signatureMethod || 'HMAC-SHA1' and
transmission: oauth1.transmission || 'Header' (so updateAuth, oauth1, and
handleChange are where to apply the merge) so the shown defaults are saved even
if the user never reselects the menus.
In `@packages/bruno-lang/v2/src/collectionBruToJson.js`:
- Around line 327-360: In authOAuth1 (in collectionBruToJson.js) the findValue
lookup currently returns empty strings for signature_method, version, and
add_params_to which causes inconsistencies; update the returned values used for
signatureEncoding, privateKeyType defaulting logic, version, and addParamsTo so
that signature_method defaults to "HMAC-SHA1", version defaults to "1.0", and
add_params_to defaults to "header" when findValue(...) yields a falsy/empty
result (preserving existing file/@file(...) handling for private_key). Ensure
you change the expressions that assign signatureEncoding, version, and
addParamsTo (and any dependent logic that reads private_key) to apply these
defaults so parsing matches bruToJson.js behavior.
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` RSA-SHA1 200.yml:
- Around line 18-45: The RSA private key PEM in the OAuth1 fixture(s) no longer
matches the canonical test key, causing signature verification to fail; update
the PEM in both fixtures (OAuth1 RSA-SHA1 200.yml and OAuth1 RSA-SHA1 Variable
Key 200.yml) to the canonical TEST_RSA_PRIVATE_KEY (the same content as
test-private-key.pem — e.g., fix the drifted substring from "+wizRjDr" to
"+sizRjDr" at the mismatched spot), or better yet stop duplicating the PEM and
load/reference the shared TEST_RSA_PRIVATE_KEY/test-private-key.pem used by the
mock server so signatures verify against TEST_RSA_PUBLIC_KEY.
In `@tests/auth/oauth1/oauth1-runner.spec.ts`:
- Around line 77-84: The test currently returns only the expanded timelineItem
from openTimelineRequest, which prevents callers (like verifyAddParamsTo) from
asserting whether OAuth params ended up in headers vs body; update
openTimelineRequest (and the similar code at 94-105) to return the timelineItem
plus scoped locators for headers and body (e.g., a headersLocator =
timelineItem.locator('.oauth-request-headers') and a bodyLocator =
timelineItem.locator('.oauth-request-body') or equivalent DOM selectors used in
the spec) so callers can perform positive and negative placement assertions;
adjust verifyAddParamsTo to accept and use these scoped locators to assert
presence/absence in headers and body.
---
Duplicate comments:
In `@packages/bruno-lang/v2/tests/oauth1.spec.js`:
- Around line 616-681: Add a collection-level test in the "OAuth1
jsonToCollectionBru (collection/folder-level)" suite to exercise
jsonToCollectionBru() when oauth1.privateKeyType === 'text' by providing a
multiline PEM string in oauth1.privateKey (mirroring the request-level multiline
PEM test) and asserting the resulting bru contains the correctly quoted
multiline private_key serialization; this ensures the collection/folder scope
exercises the text-key serializer and prevents PEM quoting regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c7dbc8dd-8a79-440f-a5b5-46da689292ee
📒 Files selected for processing (46)
packages/bruno-app/src/components/RequestPane/Auth/OAuth1/index.jspackages/bruno-app/src/utils/collections/index.jspackages/bruno-cli/src/runner/interpolate-vars.jspackages/bruno-cli/src/runner/prepare-request.jspackages/bruno-converters/src/opencollection/common/auth.tspackages/bruno-converters/src/postman/postman-to-bruno.jspackages/bruno-converters/tests/postman/postman-to-bruno/process-auth.spec.jspackages/bruno-electron/src/ipc/network/interpolate-vars.jspackages/bruno-electron/src/ipc/network/prepare-request.jspackages/bruno-filestore/src/formats/yml/common/auth.tspackages/bruno-lang/v2/src/bruToJson.jspackages/bruno-lang/v2/src/collectionBruToJson.jspackages/bruno-lang/v2/src/jsonToBru.jspackages/bruno-lang/v2/src/jsonToCollectionBru.jspackages/bruno-lang/v2/tests/oauth1.spec.jspackages/bruno-requests/src/auth/oauth1-request-authorization.spec.tspackages/bruno-requests/src/auth/oauth1-request-authorization.tspackages/bruno-schema-types/src/common/auth.tspackages/bruno-schema/src/collections/index.jstests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 Query Params 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT Query Params 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 Query Params 200.brutests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 401.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Body 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Body JSON 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 POST 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Query Params 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 401.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 Body 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA512 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA512 401.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT 401.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT Body 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT Query Params 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Body 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Body formurlencoded 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 File Key 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Query Params 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Variable Key 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA256 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA512 200.ymltests/auth/oauth1/oauth1-runner.spec.ts
✅ Files skipped from review due to trivial changes (21)
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 Query Params 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT Query Params 200.bru
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 Body 200.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Body 200.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 POST 200.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT 200.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA512 200.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 200.yml
- packages/bruno-electron/src/ipc/network/interpolate-vars.js
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 401.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT Query Params 200.yml
- packages/bruno-app/src/utils/collections/index.js
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 File Key 200.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT Body 200.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Body JSON 200.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 401.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA512 401.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Query Params 200.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT 401.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 200.yml
- packages/bruno-requests/src/auth/oauth1-request-authorization.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/bruno-electron/src/ipc/network/prepare-request.js
- packages/bruno-lang/v2/src/jsonToCollectionBru.js
- packages/bruno-schema-types/src/common/auth.ts
- packages/bruno-lang/v2/src/jsonToBru.js
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 Query Params 200.bru
- packages/bruno-cli/src/runner/interpolate-vars.js
Comment on lines
+46
to
+58
| const handleChange = (field, value) => { | ||
| dispatch( | ||
| updateAuth({ | ||
| mode: 'oauth1', | ||
| collectionUid: collection.uid, | ||
| itemUid: item.uid, | ||
| content: { | ||
| ...oauth1, | ||
| [field]: value | ||
| } | ||
| }) | ||
| ); | ||
| }; |
Contributor
There was a problem hiding this comment.
Persist the shown OAuth1 defaults.
The UI shows HMAC-SHA1 and Header, but those values stay unset in auth.oauth1 until the user explicitly reopens each menu. A fresh OAuth1 config can therefore be saved with state that does not match what the editor is displaying.
💡 Possible fix
+ const effectiveSignatureEncoding = oauth1.signatureEncoding || 'HMAC-SHA1';
+ const effectiveAddParamsTo = oauth1.addParamsTo || 'header';
+
const handleChange = (field, value) => {
dispatch(
updateAuth({
mode: 'oauth1',
collectionUid: collection.uid,
itemUid: item.uid,
content: {
...oauth1,
+ signatureEncoding: effectiveSignatureEncoding,
+ addParamsTo: effectiveAddParamsTo,
[field]: value
}
})
);
};
...
- selectedItemId={oauth1.signatureEncoding}
+ selectedItemId={effectiveSignatureEncoding}
...
- {signatureMethodLabels[oauth1.signatureEncoding] || 'HMAC-SHA1'}
+ {signatureMethodLabels[effectiveSignatureEncoding]}
...
- selectedItemId={oauth1.addParamsTo}
+ selectedItemId={effectiveAddParamsTo}
...
- {addParamsToLabels[oauth1.addParamsTo] || 'Header'}
+ {addParamsToLabels[effectiveAddParamsTo]}Also applies to: 206-216, 274-285
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-app/src/components/RequestPane/Auth/OAuth1/index.js` around
lines 46 - 58, The OAuth1 UI displays defaults ("HMAC-SHA1" and "Header") but
those values aren't persisted because dispatches to updateAuth only spread
existing oauth1; update the places that build the new auth content (e.g., the
handleChange function and other blocks that call updateAuth for mode 'oauth1')
to merge in explicit defaults when missing—ensure the content passed to
updateAuth includes signatureMethod: oauth1.signatureMethod || 'HMAC-SHA1' and
transmission: oauth1.transmission || 'Header' (so updateAuth, oauth1, and
handleChange are where to apply the merge) so the shown defaults are saved even
if the user never reselects the menus.
Comment on lines
+320
to
+333
| <div | ||
| className="flex items-center gap-2.5 mt-2 cursor-pointer select-none" | ||
| onClick={() => setAdvancedOpen(!advancedOpen)} | ||
| > | ||
| <div className="flex items-center px-2.5 py-1.5 oauth1-icon-container rounded-md"> | ||
| <IconAdjustmentsHorizontal size={14} className="oauth1-icon" /> | ||
| </div> | ||
| <span className="oauth1-section-label"> | ||
| Advanced | ||
| </span> | ||
| <IconChevronRight | ||
| size={14} | ||
| className={`oauth1-icon transition-transform ${advancedOpen ? 'rotate-90' : ''}`} | ||
| /> |
Contributor
There was a problem hiding this comment.
Use a real button for the Advanced toggle.
Keyboard users cannot focus or expand this control today because it is a clickable <div>. That blocks access to the callback/verifier/timestamp/nonce/version fields without a mouse.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-app/src/components/RequestPane/Auth/OAuth1/index.js` around
lines 320 - 333, Replace the clickable <div> used for the Advanced toggle with a
semantic <button> element (use type="button") so keyboard users can focus and
activate it; keep the onClick handler that calls setAdvancedOpen(!advancedOpen),
preserve the existing className values like "oauth1-icon-container" and
"oauth1-section-label" and the child icons (IconAdjustmentsHorizontal,
IconChevronRight), and add aria-expanded={advancedOpen} (and optionally
aria-controls pointing to the advanced panel) so screen readers get state
information while retaining the rotate-90 class toggle on IconChevronRight.
Comment on lines
+327
to
+360
| authOAuth1(_1, dictionary) { | ||
| const auth = mapPairListToKeyValPairs(dictionary.ast, false); | ||
| const findValue = (name) => { | ||
| const item = _.find(auth, { name }); | ||
| return item ? item.value : ''; | ||
| }; | ||
| return { | ||
| auth: { | ||
| oauth1: { | ||
| consumerKey: findValue('consumer_key'), | ||
| consumerSecret: findValue('consumer_secret'), | ||
| accessToken: findValue('access_token'), | ||
| accessTokenSecret: findValue('token_secret'), | ||
| callbackUrl: findValue('callback_url'), | ||
| verifier: findValue('verifier'), | ||
| signatureEncoding: findValue('signature_method'), | ||
| privateKey: (() => { | ||
| const val = findValue('private_key'); | ||
| return val && val.startsWith('@file(') && val.endsWith(')') ? val.slice(6, -1) : val; | ||
| })(), | ||
| privateKeyType: (() => { | ||
| const val = findValue('private_key'); | ||
| return val && val.startsWith('@file(') && val.endsWith(')') ? 'file' : 'text'; | ||
| })(), | ||
| timestamp: findValue('timestamp'), | ||
| nonce: findValue('nonce'), | ||
| version: findValue('version'), | ||
| realm: findValue('realm'), | ||
| addParamsTo: findValue('add_params_to'), | ||
| includeBodyHash: findValue('include_body_hash') === 'true' | ||
| } | ||
| } | ||
| }; | ||
| }, |
Contributor
There was a problem hiding this comment.
Default the core OAuth1 fields during BRU parsing.
Lines 342, 353, and 355 currently fall back to '' when signature_method, version, or add_params_to are omitted, but the OpenCollection/YML conversion paths normalize those same fields to HMAC-SHA1, 1.0, and header. That means a collection .bru relying on defaults parses into a different auth shape than every other import path, and the signer can end up with an empty signature method or placement. The identical handler in packages/bruno-lang/v2/src/bruToJson.js needs the same normalization.
🛠️ Suggested normalization for this handler
authOAuth1(_1, dictionary) {
const auth = mapPairListToKeyValPairs(dictionary.ast, false);
const findValue = (name) => {
const item = _.find(auth, { name });
return item ? item.value : '';
};
+ const privateKeyValue = findValue('private_key');
+ const isPrivateKeyFile = privateKeyValue.startsWith('@file(') && privateKeyValue.endsWith(')');
return {
auth: {
oauth1: {
consumerKey: findValue('consumer_key'),
consumerSecret: findValue('consumer_secret'),
accessToken: findValue('access_token'),
accessTokenSecret: findValue('token_secret'),
callbackUrl: findValue('callback_url'),
verifier: findValue('verifier'),
- signatureEncoding: findValue('signature_method'),
- privateKey: (() => {
- const val = findValue('private_key');
- return val && val.startsWith('@file(') && val.endsWith(')') ? val.slice(6, -1) : val;
- })(),
- privateKeyType: (() => {
- const val = findValue('private_key');
- return val && val.startsWith('@file(') && val.endsWith(')') ? 'file' : 'text';
- })(),
+ signatureEncoding: findValue('signature_method') || 'HMAC-SHA1',
+ privateKey: isPrivateKeyFile ? privateKeyValue.slice(6, -1) : privateKeyValue,
+ privateKeyType: isPrivateKeyFile ? 'file' : 'text',
timestamp: findValue('timestamp'),
nonce: findValue('nonce'),
- version: findValue('version'),
+ version: findValue('version') || '1.0',
realm: findValue('realm'),
- addParamsTo: findValue('add_params_to'),
+ addParamsTo: findValue('add_params_to') || 'header',
includeBodyHash: findValue('include_body_hash') === 'true'
}
}
};
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-lang/v2/src/collectionBruToJson.js` around lines 327 - 360, In
authOAuth1 (in collectionBruToJson.js) the findValue lookup currently returns
empty strings for signature_method, version, and add_params_to which causes
inconsistencies; update the returned values used for signatureEncoding,
privateKeyType defaulting logic, version, and addParamsTo so that
signature_method defaults to "HMAC-SHA1", version defaults to "1.0", and
add_params_to defaults to "header" when findValue(...) yields a falsy/empty
result (preserving existing file/@file(...) handling for private_key). Ensure
you change the expressions that assign signatureEncoding, version, and
addParamsTo (and any dependent logic that reads private_key) to apply these
defaults so parsing matches bruToJson.js behavior.
Comment on lines
+18
to
+45
| -----BEGIN PRIVATE KEY----- | ||
| MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCf/ioOwb6uGm01 | ||
| WjL89sBOKmvMuwyGW33w+ud3eV5dWbHZbJTgCHMtjUVqGE4sVPnX8wfKgb00hVGZ | ||
| ZhbRsqquUMJL+7RHZA1G3ZcC2WPmrUvIpi13W7F/+wizRjDrutgK0oc4L7zUPhgE | ||
| +d+Qe6JHvFL376hqkD9Z6vd9jVNRDv7D2uJcG+Uc7ePM6yLzQeJILnfvSwxna89G | ||
| 6YxfbYNjf/VKAcRM6UAR8aRGm/A9Qmdwsy08iRzlM0K5lid7Ath7ADJ+D6ezUIhl | ||
| qiI6sVrZdpsFXzuvcxjgAts54XoQsLCDZudTmzMs4BftswULCQFdj6c2oye9QKsu | ||
| EoZ0CvwNAgMBAAECggEAA1kIJu2QQIhhB0rEjQfaF5309NW9JK/pag91+claW3hd | ||
| q6papc1yIN6MjfRwPlE7i8npZygL03uEAkJhRoYHOEU3AOwFZluw7hiuPkBaQiDC | ||
| Ld1RpOZlnRidoqgHV1y+LzZ0ieJwgGhu4ZEbnSRZIvMihqRHJo5aJQGGUuQ60r6w | ||
| Z5N6j7GGCV14oGck6+0k/NhYrkhpbyl+AHPZeQ20L9ZaSpl+GD3RUgvx4nOhN1Fx | ||
| agovDCKwmRPSpuuBw6Wun1hKC9MuuL89CCy2yZ+MrjQmQJ/onKZKR2fc1I5g2JRu | ||
| z7xObQq/tAXIHKM27YYY5J3NcGtsy9tLpdtFDle/wQKBgQDfBaBURNs/ccVMMXSZ | ||
| T5lOo337Rv3bGFnoLwURUZrjJNGrHqLzeZvTNt0sXQX6Jbdeb5qnC/icng+QDEod | ||
| 9hRVDz1rN+2YqG6AMvrX5dQSK1PmkwVHLles4sX7DsZaiKNYFFbXc9rt7kXBwDoE | ||
| LXYidqUIqZiOjMqCyNlfyHHnRQKBgQC3ppq7TCEWIfqLzsfOoqaKD/fDOrQLLmor | ||
| 7RvAdGa+EyVeB1G+NQO00KN6U9W+SNPz0cEUYUZQiAaAghUGkNurrS1shr5k7aTX | ||
| pXpXtaA+xSEAd6w8lTl9mAfwZMBCcsfjyjPf1RPjZ6Tj2fdHqnsllSU5843LOqZK | ||
| CBXiitdKKQKBgQCVNEloN0zLLE1HxUpxiwxQzRZqtrr9ClST/mkQhhzuW+Kd7fgs | ||
| la5HZ0we8vkdun/sARRhL6Qa+7ADugUX+Frv8SsxARDG8eBDilfBevQfV7dg6fk8 | ||
| /ucPNgQoC2Fujj1hnvHeYJcWWTN4BSeLRfLj6aZNnlD/BXgyeTbcWtjBVQKBgBhG | ||
| npd5hboePbczWzgWSftgBvk4jkoYFZK+4fc7q8UeVMcsIoMJEPdayPFHma5whAvr | ||
| wyEFhrzobiuYhlz60v7LgoChAxPmUe7rgdOMP6Vse2NLbmoHs7TFXu9I8h0WfRPA | ||
| S8EfsmRR8/rmeghwIZ0jLOuPJUQi+Y45qWLrxW+ZAoGAMXYhio9y93M0nRqjiiCR | ||
| YibnhpZxvrNiLPxNiUi/WxcWEvulpmbKxLUhiWJB1ZmRTiGYlnclQXUuRyaOQNTo | ||
| 5TVAaNzDXayWVbxhx3Lb8NUV+QNUJEJOgjq4+NYw8fUZCr7T64pGGM4DJHPuHCBo | ||
| dJv7UByPuMKBIOYpy3Z+iWs= | ||
| -----END PRIVATE KEY----- |
Contributor
There was a problem hiding this comment.
This PEM no longer matches the OAuth1 test key.
Line 21 has .../+wizRjDr..., but the canonical TEST_RSA_PRIVATE_KEY / test-private-key.pem used by the mock server has .../+sizRjDr... at the same spot. That changes the private key, so RSA-SHA1 signatures from this fixture will fail verification against TEST_RSA_PUBLIC_KEY. The same drift appears in tests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Variable Key 200.yml, so please fix both copies or source the shared PEM instead of duplicating it inline.
🔧 Minimal fix for the drifted key material
- ZhbRsqquUMJL+7RHZA1G3ZcC2WPmrUvIpi13W7F/+wizRjDrutgK0oc4L7zUPhgE
+ ZhbRsqquUMJL+7RHZA1G3ZcC2WPmrUvIpi13W7F/+sizRjDrutgK0oc4L7zUPhgE📝 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.
Suggested change
| -----BEGIN PRIVATE KEY----- | |
| MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCf/ioOwb6uGm01 | |
| WjL89sBOKmvMuwyGW33w+ud3eV5dWbHZbJTgCHMtjUVqGE4sVPnX8wfKgb00hVGZ | |
| ZhbRsqquUMJL+7RHZA1G3ZcC2WPmrUvIpi13W7F/+wizRjDrutgK0oc4L7zUPhgE | |
| +d+Qe6JHvFL376hqkD9Z6vd9jVNRDv7D2uJcG+Uc7ePM6yLzQeJILnfvSwxna89G | |
| 6YxfbYNjf/VKAcRM6UAR8aRGm/A9Qmdwsy08iRzlM0K5lid7Ath7ADJ+D6ezUIhl | |
| qiI6sVrZdpsFXzuvcxjgAts54XoQsLCDZudTmzMs4BftswULCQFdj6c2oye9QKsu | |
| EoZ0CvwNAgMBAAECggEAA1kIJu2QQIhhB0rEjQfaF5309NW9JK/pag91+claW3hd | |
| q6papc1yIN6MjfRwPlE7i8npZygL03uEAkJhRoYHOEU3AOwFZluw7hiuPkBaQiDC | |
| Ld1RpOZlnRidoqgHV1y+LzZ0ieJwgGhu4ZEbnSRZIvMihqRHJo5aJQGGUuQ60r6w | |
| Z5N6j7GGCV14oGck6+0k/NhYrkhpbyl+AHPZeQ20L9ZaSpl+GD3RUgvx4nOhN1Fx | |
| agovDCKwmRPSpuuBw6Wun1hKC9MuuL89CCy2yZ+MrjQmQJ/onKZKR2fc1I5g2JRu | |
| z7xObQq/tAXIHKM27YYY5J3NcGtsy9tLpdtFDle/wQKBgQDfBaBURNs/ccVMMXSZ | |
| T5lOo337Rv3bGFnoLwURUZrjJNGrHqLzeZvTNt0sXQX6Jbdeb5qnC/icng+QDEod | |
| 9hRVDz1rN+2YqG6AMvrX5dQSK1PmkwVHLles4sX7DsZaiKNYFFbXc9rt7kXBwDoE | |
| LXYidqUIqZiOjMqCyNlfyHHnRQKBgQC3ppq7TCEWIfqLzsfOoqaKD/fDOrQLLmor | |
| 7RvAdGa+EyVeB1G+NQO00KN6U9W+SNPz0cEUYUZQiAaAghUGkNurrS1shr5k7aTX | |
| pXpXtaA+xSEAd6w8lTl9mAfwZMBCcsfjyjPf1RPjZ6Tj2fdHqnsllSU5843LOqZK | |
| CBXiitdKKQKBgQCVNEloN0zLLE1HxUpxiwxQzRZqtrr9ClST/mkQhhzuW+Kd7fgs | |
| la5HZ0we8vkdun/sARRhL6Qa+7ADugUX+Frv8SsxARDG8eBDilfBevQfV7dg6fk8 | |
| /ucPNgQoC2Fujj1hnvHeYJcWWTN4BSeLRfLj6aZNnlD/BXgyeTbcWtjBVQKBgBhG | |
| npd5hboePbczWzgWSftgBvk4jkoYFZK+4fc7q8UeVMcsIoMJEPdayPFHma5whAvr | |
| wyEFhrzobiuYhlz60v7LgoChAxPmUe7rgdOMP6Vse2NLbmoHs7TFXu9I8h0WfRPA | |
| S8EfsmRR8/rmeghwIZ0jLOuPJUQi+Y45qWLrxW+ZAoGAMXYhio9y93M0nRqjiiCR | |
| YibnhpZxvrNiLPxNiUi/WxcWEvulpmbKxLUhiWJB1ZmRTiGYlnclQXUuRyaOQNTo | |
| 5TVAaNzDXayWVbxhx3Lb8NUV+QNUJEJOgjq4+NYw8fUZCr7T64pGGM4DJHPuHCBo | |
| dJv7UByPuMKBIOYpy3Z+iWs= | |
| -----END PRIVATE KEY----- | |
| -----BEGIN PRIVATE KEY----- | |
| MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCf/ioOwb6uGm01 | |
| WjL89sBOKmvMuwyGW33w+ud3eV5dWbHZbJTgCHMtjUVqGE4sVPnX8wfKgb00hVGZ | |
| ZhbRsqquUMJL+7RHZA1G3ZcC2WPmrUvIpi13W7F/+sizRjDrutgK0oc4L7zUPhgE | |
| d+Qe6JHvFL376hqkD9Z6vd9jVNRDv7D2uJcG+Uc7ePM6yLzQeJILnfvSwxna89G | |
| 6YxfbYNjf/VKAcRM6UAR8aRGm/A9Qmdwsy08iRzlM0K5lid7Ath7ADJ+D6ezUIhl | |
| qiI6sVrZdpsFXzuvcxjgAts54XoQsLCDZudTmzMs4BftswULCQFdj6c2oye9QKsu | |
| EoZ0CvwNAgMBAAECggEAA1kIJu2QQIhhB0rEjQfaF5309NW9JK/pag91+claW3hd | |
| q6papc1yIN6MjfRwPlE7i8npZygL03uEAkJhRoYHOEU3AOwFZluw7hiuPkBaQiDC | |
| Ld1RpOZlnRidoqgHV1y+LzZ0ieJwgGhu4ZEbnSRZIvMihqRHJo5aJQGGUuQ60r6w | |
| Z5N6j7GGCV14oGck6+0k/NhYrkhpbyl+AHPZeQ20L9ZaSpl+GD3RUgvx4nOhN1Fx | |
| agovDCKwmRPSpuuBw6Wun1hKC9MuuL89CCy2yZ+MrjQmQJ/onKZKR2fc1I5g2JRu | |
| z7xObQq/tAXIHKM27YYY5J3NcGtsy9tLpdtFDle/wQKBgQDfBaBURNs/ccVMMXSZ | |
| T5lOo337Rv3bGFnoLwURUZrjJNGrHqLzeZvTNt0sXQX6Jbdeb5qnC/icng+QDEod | |
| 9hRVDz1rN+2YqG6AMvrX5dQSK1PmkwVHLles4sX7DsZaiKNYFFbXc9rt7kXBwDoE | |
| LXYidqUIqZiOjMqCyNlfyHHnRQKBgQC3ppq7TCEWIfqLzsfOoqaKD/fDOrQLLmor | |
| 7RvAdGa+EyVeB1G+NQO00KN6U9W+SNPz0cEUYUZQiAaAghUGkNurrS1shr5k7aTX | |
| pXpXtaA+xSEAd6w8lTl9mAfwZMBCcsfjyjPf1RPjZ6Tj2fdHqnsllSU5843LOqZK | |
| CBXiitdKKQKBgQCVNEloN0zLLE1HxUpxiwxQzRZqtrr9ClST/mkQhhzuW+Kd7fgs | |
| la5HZ0we8vkdun/sARRhL6Qa+7ADugUX+Frv8SsxARDG8eBDilfBevQfV7dg6fk8 | |
| /ucPNgQoC2Fujj1hnvHeYJcWWTN4BSeLRfLj6aZNnlD/BXgyeTbcWtjBVQKBgBhG | |
| npd5hboePbczWzgWSftgBvk4jkoYFZK+4fc7q8UeVMcsIoMJEPdayPFHma5whAvr | |
| wyEFhrzobiuYhlz60v7LgoChAxPmUe7rgdOMP6Vse2NLbmoHs7TFXu9I8h0WfRPA | |
| S8EfsmRR8/rmeghwIZ0jLOuPJUQi+Y45qWLrxW+ZAoGAMXYhio9y93M0nRqjiiCR | |
| YibnhpZxvrNiLPxNiUi/WxcWEvulpmbKxLUhiWJB1ZmRTiGYlnclQXUuRyaOQNTo | |
| 5TVAaNzDXayWVbxhx3Lb8NUV+QNUJEJOgjq4+NYw8fUZCr7T64pGGM4DJHPuHCBo | |
| dJv7UByPuMKBIOYpy3Z+iWs= | |
| -----END PRIVATE KEY----- |
🧰 Tools
🪛 Betterleaks (1.1.1)
[high] 18-45: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` RSA-SHA1 200.yml around
lines 18 - 45, The RSA private key PEM in the OAuth1 fixture(s) no longer
matches the canonical test key, causing signature verification to fail; update
the PEM in both fixtures (OAuth1 RSA-SHA1 200.yml and OAuth1 RSA-SHA1 Variable
Key 200.yml) to the canonical TEST_RSA_PRIVATE_KEY (the same content as
test-private-key.pem — e.g., fix the drifted substring from "+wizRjDr" to
"+sizRjDr" at the mismatched spot), or better yet stop duplicating the PEM and
load/reference the shared TEST_RSA_PRIVATE_KEY/test-private-key.pem used by the
mock server so signatures verify against TEST_RSA_PUBLIC_KEY.
Comment on lines
+77
to
+84
| const openTimelineRequest = async (page) => { | ||
| await selectResponsePaneTab(page, 'Timeline'); | ||
|
|
||
| // Click the first (latest) timeline item header to expand it | ||
| const timelineItem = page.locator('.timeline-item').first(); | ||
| await timelineItem.locator('.oauth-request-item-header').click(); | ||
|
|
||
| return timelineItem; |
Contributor
There was a problem hiding this comment.
Return scoped timeline locators for placement checks.
verifyAddParamsTo() currently only proves the param left the URL and appeared somewhere in the expanded item. If signing regresses to sending OAuth params in both the Authorization header and the body, this still passes because callers never get a headers locator for a negative assertion.
As per coding guidelines, 'Cover both the 'happy path' and the realistically problematic paths. Validate expected success behaviour, but also validate error handling, edge cases, and degraded-mode behaviour.'
Also applies to: 94-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/auth/oauth1/oauth1-runner.spec.ts` around lines 77 - 84, The test
currently returns only the expanded timelineItem from openTimelineRequest, which
prevents callers (like verifyAddParamsTo) from asserting whether OAuth params
ended up in headers vs body; update openTimelineRequest (and the similar code at
94-105) to return the timelineItem plus scoped locators for headers and body
(e.g., a headersLocator = timelineItem.locator('.oauth-request-headers') and a
bodyLocator = timelineItem.locator('.oauth-request-body') or equivalent DOM
selectors used in the spec) so callers can perform positive and negative
placement assertions; adjust verifyAddParamsTo to accept and use these scoped
locators to assert presence/absence in headers and body.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (7)
tests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 200.yml (1)
21-21:⚠️ Potential issue | 🔴 CriticalFix RSA private key drift in fixture PEM.
Line 21 still has
.../+wizRjDr...; this does not match the canonical OAuth1 test private key and can break RSA-SHA1 signature verification against the test public key. Please update it to the canonical value (.../+sizRjDr...) and keep both RSA-SHA1 fixtures in sync to avoid divergence.🔧 Minimal fix
- ZhbRsqquUMJL+7RHZA1G3ZcC2WPmrUvIpi13W7F/+wizRjDrutgK0oc4L7zUPhgE + ZhbRsqquUMJL+7RHZA1G3ZcC2WPmrUvIpi13W7F/+sizRjDrutgK0oc4L7zUPhgE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` RSA-SHA1 200.yml at line 21, The RSA private key in the OAuth1 RSA-SHA1 fixture contains a drifted substring (`.../+wizRjDr...`) that must be corrected to the canonical value (`.../+sizRjDr...`); update that substring in the "OAuth1 RSA-SHA1 200.yml" fixture (and the other RSA-SHA1 fixture) so the PEM matches the canonical test private key exactly and keep both RSA-SHA1 fixtures identical to avoid future divergence.packages/bruno-app/src/components/RequestPane/Auth/OAuth1/index.js (2)
320-333:⚠️ Potential issue | 🟠 MajorUse a real button for the Advanced toggle.
This clickable
divis not keyboard-focusable and does not expose expanded state, so the advanced OAuth1 fields are inaccessible without a mouse.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RequestPane/Auth/OAuth1/index.js` around lines 320 - 333, Replace the clickable div used for the Advanced toggle with a real button element (preserving classes and the onClick handler that calls setAdvancedOpen(!advancedOpen)), give it type="button", add aria-expanded={advancedOpen} and aria-controls pointing to the advanced panel id, and remove any non-semantic role/onclick-only behavior so the control is keyboard-focusable and announces state; keep the IconChevronRight rotation logic and oauth1-section-label so visual behavior remains the same while accessibility is restored.
46-58:⚠️ Potential issue | 🟠 MajorPersist the OAuth1 defaults the UI is already showing.
HMAC-SHA1andHeaderare rendered as fallbacks, but this dispatch only writes the field being edited. New OAuth1 configs can therefore be saved with state that does not match the visible selections. Apply the same default merge to the otherupdateAuth({ mode: 'oauth1' })paths in this component too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RequestPane/Auth/OAuth1/index.js` around lines 46 - 58, The dispatch in handleChange only writes the single edited field, so UI fallback defaults like "HMAC-SHA1" and "Header" aren’t persisted; update the payloads for handleChange and every other updateAuth({ mode: 'oauth1', ... }) call in this component to merge the current oauth1 object plus the UI defaults before overriding the edited field (e.g., content: { ...oauth1, signatureMethod: oauth1.signatureMethod || 'HMAC-SHA1', transport: oauth1.transport || 'Header', [field]: value } or the equivalent merge in other updateAuth usages) so saved state always includes the visible default values.packages/bruno-requests/src/auth/oauth1-request-authorization.ts (2)
451-464:⚠️ Potential issue | 🔴 CriticalReject invalid body placement instead of rewriting the payload.
This branch silently replaces non-form bodies with OAuth params and flips the request to
application/x-www-form-urlencoded. OAuth1 body placement is only valid for form-urlencoded requests, so JSON/raw/GET/HEAD cases should fail fast instead of being mutated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-requests/src/auth/oauth1-request-authorization.ts` around lines 451 - 464, The branch handling case 'body' currently rewrites non-form payloads to form-encoded; instead, check isFormUrlEncoded and if false (or if method is GET/HEAD or request.data is non-form like JSON) throw a clear error indicating "OAuth1 body placement requires application/x-www-form-urlencoded" rather than mutating request.data/headers. Locate the case 'body' block in oauth1-request-authorization.ts and replace the silent rewrite logic that touches request.data and request.headers (variables: isFormUrlEncoded, request.data, oauthData, ctKey) with an explicit validation that rejects invalid body placement, and update any tests that assumed silent rewriting.
54-63:⚠️ Potential issue | 🟠 MajorPreserve an explicitly blank
oauth_version.
version = '1.0'/version || '1.0'make it impossible to omitoauth_version, even though the rest of this PR preserves emptyversionvalues. That changes the signed parameter set for configs that intentionally clear the field.Also applies to: 236-258, 385-389
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-requests/src/auth/oauth1-request-authorization.ts` around lines 54 - 63, The code currently forces oauth_version to '1.0' via expressions like version || '1.0', which prevents preserving an explicitly blank oauth_version; update all places that default version (references to the oauth_version property on OAuth1AuthData and any local variable named version in the OAuth parameter-building functions) to only substitute '1.0' when version is strictly undefined (e.g., use version === undefined ? '1.0' : version), and ensure the signing/parameter set logic includes oauth_version even when it's an empty string so intentional blank values are preserved (apply the same change where similar defaulting appears in the parameter assembly and header creation logic).packages/bruno-lang/v2/src/collectionBruToJson.js (1)
327-360:⚠️ Potential issue | 🟠 MajorDefault the core OAuth1 fields during collection BRU parsing.
When
signature_method,version, orplacementare omitted, this returns''while the YML/OpenCollection path already normalizes them toHMAC-SHA1,1.0, andheader. A valid collection.bruthat relies on OAuth1 defaults will deserialize into an incomplete auth config and can fail at signing time.🛠️ Suggested normalization
authOAuth1(_1, dictionary) { const auth = mapPairListToKeyValPairs(dictionary.ast, false); const findValue = (name) => { const item = _.find(auth, { name }); return item ? item.value : ''; }; + const privateKeyValue = findValue('private_key'); + const isPrivateKeyFile = privateKeyValue.startsWith('@file(') && privateKeyValue.endsWith(')'); return { auth: { oauth1: { consumerKey: findValue('consumer_key'), consumerSecret: findValue('consumer_secret'), accessToken: findValue('access_token'), accessTokenSecret: findValue('token_secret'), callbackUrl: findValue('callback_url'), verifier: findValue('verifier'), - signatureEncoding: findValue('signature_method'), - privateKey: (() => { - const val = findValue('private_key'); - return val && val.startsWith('@file(') && val.endsWith(')') ? val.slice(6, -1) : val; - })(), - privateKeyType: (() => { - const val = findValue('private_key'); - return val && val.startsWith('@file(') && val.endsWith(')') ? 'file' : 'text'; - })(), + signatureEncoding: findValue('signature_method') || 'HMAC-SHA1', + privateKey: isPrivateKeyFile ? privateKeyValue.slice(6, -1) : privateKeyValue, + privateKeyType: isPrivateKeyFile ? 'file' : 'text', timestamp: findValue('timestamp'), nonce: findValue('nonce'), - version: findValue('version'), + version: findValue('version') || '1.0', realm: findValue('realm'), - placement: findValue('placement'), + placement: findValue('placement') || 'header', includeBodyHash: findValue('include_body_hash') === 'true' } } }; },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-lang/v2/src/collectionBruToJson.js` around lines 327 - 360, In authOAuth1, when mapping values via findValue (in function authOAuth1 and its local findValue helper), ensure missing signature_method, version, and placement are normalized to the same defaults used by the YML/OpenCollection path: set signatureEncoding (signature_method) to 'HMAC-SHA1' when findValue('signature_method') is falsy, set version to '1.0' when findValue('version') is falsy, and set placement to 'header' when findValue('placement') is falsy; keep the existing file/text handling for private_key unchanged.packages/bruno-lang/v2/src/bruToJson.js (1)
714-747:⚠️ Potential issue | 🟠 MajorDefault the same OAuth1 fields in request-level BRU parsing.
This parser has the same gap as the collection variant: omitted
signature_method,version, orplacementcome back as empty strings instead of the OAuth1 defaults. That makes request-level.brufiles behave differently from the YML/OpenCollection import path.🛠️ Suggested normalization
authOAuth1(_1, dictionary) { const auth = mapPairListToKeyValPairs(dictionary.ast, false); const findValue = (name) => { const item = _.find(auth, { name }); return item ? item.value : ''; }; + const privateKeyValue = findValue('private_key'); + const isPrivateKeyFile = privateKeyValue.startsWith('@file(') && privateKeyValue.endsWith(')'); return { auth: { oauth1: { consumerKey: findValue('consumer_key'), consumerSecret: findValue('consumer_secret'), accessToken: findValue('access_token'), accessTokenSecret: findValue('token_secret'), callbackUrl: findValue('callback_url'), verifier: findValue('verifier'), - signatureEncoding: findValue('signature_method'), - privateKey: (() => { - const val = findValue('private_key'); - return val && val.startsWith('@file(') && val.endsWith(')') ? val.slice(6, -1) : val; - })(), - privateKeyType: (() => { - const val = findValue('private_key'); - return val && val.startsWith('@file(') && val.endsWith(')') ? 'file' : 'text'; - })(), + signatureEncoding: findValue('signature_method') || 'HMAC-SHA1', + privateKey: isPrivateKeyFile ? privateKeyValue.slice(6, -1) : privateKeyValue, + privateKeyType: isPrivateKeyFile ? 'file' : 'text', timestamp: findValue('timestamp'), nonce: findValue('nonce'), - version: findValue('version'), + version: findValue('version') || '1.0', realm: findValue('realm'), - placement: findValue('placement'), + placement: findValue('placement') || 'header', includeBodyHash: findValue('include_body_hash') === 'true' } } }; },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-lang/v2/src/bruToJson.js` around lines 714 - 747, The authOAuth1 request-level parser returns empty strings for omitted OAuth1 fields (signature_method, version, placement) causing inconsistent behavior with the collection/YML path; update authOAuth1 to apply the same default normalization used by the collection variant by reusing the same default constants/normalization logic (or function) rather than leaving findValue(...) results empty—specifically adjust the oauth1 object construction in authOAuth1 (where mapPairListToKeyValPairs and findValue are used) so signatureEncoding/signature_method, version, and placement fall back to the collection parser's defaults.
🧹 Nitpick comments (2)
packages/bruno-converters/tests/postman/postman-to-bruno/process-auth.spec.js (1)
513-513: Consider addingoauth1assertion to similar tests for consistency.This assertion was added here for the unknown auth type test, but the earlier tests at lines 423-433 (
should handle auth object with undefined type), 435-448 (should handle type as null and auth as null), 450-465, 467-479, and 481-497 are missing the correspondingoauth1assertion.🔧 Proposed fix to add missing assertions
For each of the tests mentioned, add:
expect(requestObject.auth.apikey).toBe(null); + expect(requestObject.auth.oauth1).toBe(null); expect(requestObject.auth.oauth2).toBe(null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-converters/tests/postman/postman-to-bruno/process-auth.spec.js` at line 513, Add the same oauth1 assertion to the earlier unit tests so they consistently verify unknown OAuth1 state: in the spec functions for "should handle auth object with undefined type", "should handle type as null and auth as null", and the tests covering lines around 450-465, 467-479, and 481-497, update each test to include an assertion like expect(requestObject.auth.oauth1).toBe(null) after the existing auth assertions; locate these in the file's test blocks where requestObject is produced (the same place as the new unknown-auth test that contains expect(requestObject.auth.oauth1).toBe(null)) and add the identical oauth1 null-check to each corresponding it(...) block to keep behavior consistent.packages/bruno-filestore/src/formats/yml/common/auth.ts (1)
119-145: Add JSDoc forbuildOAuth1Auth().This helper now owns the OAuth1 filestore contract, so a short docblock would make the text-vs-file private-key normalization easier to maintain.
💡 Suggested docblock
+/** + * Builds the OpenCollection OAuth1 auth shape from Bruno's persisted auth config. + * Preserves whether the private key is stored inline or referenced as a file path. + */ const buildOAuth1Auth = (config?: BrunoAuth['oauth1']): AuthOAuth1 => {As per coding guidelines, "Add JSDoc comments to abstractions for additional details".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-filestore/src/formats/yml/common/auth.ts` around lines 119 - 145, Add a concise JSDoc block above buildOAuth1Auth explaining its responsibility (normalize BrunoAuth['oauth1'] into the AuthOAuth1 filestore contract), describing the expected input shape (config: BrunoAuth['oauth1']) and the returned AuthOAuth1 object, and documenting the key normalization behavior (especially how privateKey + privateKeyType produce either {type: 'file', value: ...} or {type: 'text', value: ...}), plus notes on optional fields like consumerKey, consumerSecret, accessToken, accessTokenSecret, callbackUrl, verifier, signatureEncoding, timestamp, nonce, version, realm, placement and the boolean includeBodyHash; include examples/edge-cases (undefined config returns { type: 'oauth1' }) and any invariants callers should rely on.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bruno-requests/src/auth/oauth1-request-authorization.ts`:
- Around line 421-426: The oauth_body_hash calculation in the includeBodyHash
branch currently serializes request.data via JSON.stringify (in the block around
includeBodyHash/isFormUrlEncoded and computeBodyHash), which is invalid for
multipart, file Buffers, or streams; change the logic to compute the hash from
the actual serialized request bytes after the request body is serialized (or
only compute it for known-safe body types). Specifically, move the
computeBodyHash call to run after the codepath that builds the wire payload (or
accept Buffer/Uint8Array bodies directly), detect and handle multipart/form-data
and stream/FileBuffer cases by hashing the final byte payload or skipping adding
oauth_body_hash for unsupported types, and keep using
signatureEncoding/computeBodyHash for the actual bytes so oauth_body_hash
reflects the real wire payload.
- Around line 385-390: Normalize consumerSecret the same way accessTokenSecret
is normalized before passing it to createOAuth1Authorizer: compute a
normalizedConsumerSecret (e.g., treat undefined/null/blank as empty string or
use the same normalization routine used for accessTokenSecret) and use that in
the authorizer's consumer.secret field so HMAC/PLAINTEXT signing keys are built
correctly; update the consumer: { key: consumerKey, secret: ... } passed to
createOAuth1Authorizer accordingly (references: consumerSecret,
accessTokenSecret, createOAuth1Authorizer, authorizer).
In `@tests/auth/oauth1/fixtures/collections/bru/OAuth1` RSA-SHA512 200.bru:
- Around line 1-63: The fixtures directory is misnamed as fixtures/collections
(plural) but should be fixtures/collection (singular); rename the directory from
fixtures/collections to fixtures/collection, move the subdirectories (e.g., bru
and yml containing files like "OAuth1 RSA-SHA512 200.bru") into the new singular
folder, and update any test references or imports that point to
fixtures/collections to the new fixtures/collection path so tests and loaders
(the OAuth1 collection fixtures) resolve correctly.
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` RSA-SHA1 Body
formurlencoded 200.yml:
- Around line 20-50: The fixture currently embeds a PEM private key inline under
the YAML field privateKey -> value; remove that inline PEM and instead reference
an external secret (e.g., use an environment variable like OAUTH1_PRIVATE_KEY or
load from a secure test fixture file) and update the test fixture loader to
substitute that value at runtime; specifically, replace the multiline value in
the privateKey field with a placeholder and modify the test
setup/fixture-parsing code to inject process.env.OAUTH1_PRIVATE_KEY (or read
from a secure file) so the key is not stored in-repo.
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` RSA-SHA256 200.yml:
- Around line 15-45: The fixture embeds a real PEM under the YAML key privateKey
in "OAuth1 RSA-SHA256 200.yml"; remove the inline PEM and replace it with a
non-sensitive reference (e.g., a placeholder string or an env/fixture variable
reference) so tests load the actual private key from the test environment or a
secrets fixture at runtime; update any test helper that reads this fixture
(lookup by privateKey) to read process.env (or the designated fixture loader)
when the placeholder is present.
---
Duplicate comments:
In `@packages/bruno-app/src/components/RequestPane/Auth/OAuth1/index.js`:
- Around line 320-333: Replace the clickable div used for the Advanced toggle
with a real button element (preserving classes and the onClick handler that
calls setAdvancedOpen(!advancedOpen)), give it type="button", add
aria-expanded={advancedOpen} and aria-controls pointing to the advanced panel
id, and remove any non-semantic role/onclick-only behavior so the control is
keyboard-focusable and announces state; keep the IconChevronRight rotation logic
and oauth1-section-label so visual behavior remains the same while accessibility
is restored.
- Around line 46-58: The dispatch in handleChange only writes the single edited
field, so UI fallback defaults like "HMAC-SHA1" and "Header" aren’t persisted;
update the payloads for handleChange and every other updateAuth({ mode:
'oauth1', ... }) call in this component to merge the current oauth1 object plus
the UI defaults before overriding the edited field (e.g., content: { ...oauth1,
signatureMethod: oauth1.signatureMethod || 'HMAC-SHA1', transport:
oauth1.transport || 'Header', [field]: value } or the equivalent merge in other
updateAuth usages) so saved state always includes the visible default values.
In `@packages/bruno-lang/v2/src/bruToJson.js`:
- Around line 714-747: The authOAuth1 request-level parser returns empty strings
for omitted OAuth1 fields (signature_method, version, placement) causing
inconsistent behavior with the collection/YML path; update authOAuth1 to apply
the same default normalization used by the collection variant by reusing the
same default constants/normalization logic (or function) rather than leaving
findValue(...) results empty—specifically adjust the oauth1 object construction
in authOAuth1 (where mapPairListToKeyValPairs and findValue are used) so
signatureEncoding/signature_method, version, and placement fall back to the
collection parser's defaults.
In `@packages/bruno-lang/v2/src/collectionBruToJson.js`:
- Around line 327-360: In authOAuth1, when mapping values via findValue (in
function authOAuth1 and its local findValue helper), ensure missing
signature_method, version, and placement are normalized to the same defaults
used by the YML/OpenCollection path: set signatureEncoding (signature_method) to
'HMAC-SHA1' when findValue('signature_method') is falsy, set version to '1.0'
when findValue('version') is falsy, and set placement to 'header' when
findValue('placement') is falsy; keep the existing file/text handling for
private_key unchanged.
In `@packages/bruno-requests/src/auth/oauth1-request-authorization.ts`:
- Around line 451-464: The branch handling case 'body' currently rewrites
non-form payloads to form-encoded; instead, check isFormUrlEncoded and if false
(or if method is GET/HEAD or request.data is non-form like JSON) throw a clear
error indicating "OAuth1 body placement requires
application/x-www-form-urlencoded" rather than mutating request.data/headers.
Locate the case 'body' block in oauth1-request-authorization.ts and replace the
silent rewrite logic that touches request.data and request.headers (variables:
isFormUrlEncoded, request.data, oauthData, ctKey) with an explicit validation
that rejects invalid body placement, and update any tests that assumed silent
rewriting.
- Around line 54-63: The code currently forces oauth_version to '1.0' via
expressions like version || '1.0', which prevents preserving an explicitly blank
oauth_version; update all places that default version (references to the
oauth_version property on OAuth1AuthData and any local variable named version in
the OAuth parameter-building functions) to only substitute '1.0' when version is
strictly undefined (e.g., use version === undefined ? '1.0' : version), and
ensure the signing/parameter set logic includes oauth_version even when it's an
empty string so intentional blank values are preserved (apply the same change
where similar defaulting appears in the parameter assembly and header creation
logic).
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` RSA-SHA1 200.yml:
- Line 21: The RSA private key in the OAuth1 RSA-SHA1 fixture contains a drifted
substring (`.../+wizRjDr...`) that must be corrected to the canonical value
(`.../+sizRjDr...`); update that substring in the "OAuth1 RSA-SHA1 200.yml"
fixture (and the other RSA-SHA1 fixture) so the PEM matches the canonical test
private key exactly and keep both RSA-SHA1 fixtures identical to avoid future
divergence.
---
Nitpick comments:
In
`@packages/bruno-converters/tests/postman/postman-to-bruno/process-auth.spec.js`:
- Line 513: Add the same oauth1 assertion to the earlier unit tests so they
consistently verify unknown OAuth1 state: in the spec functions for "should
handle auth object with undefined type", "should handle type as null and auth as
null", and the tests covering lines around 450-465, 467-479, and 481-497, update
each test to include an assertion like
expect(requestObject.auth.oauth1).toBe(null) after the existing auth assertions;
locate these in the file's test blocks where requestObject is produced (the same
place as the new unknown-auth test that contains
expect(requestObject.auth.oauth1).toBe(null)) and add the identical oauth1
null-check to each corresponding it(...) block to keep behavior consistent.
In `@packages/bruno-filestore/src/formats/yml/common/auth.ts`:
- Around line 119-145: Add a concise JSDoc block above buildOAuth1Auth
explaining its responsibility (normalize BrunoAuth['oauth1'] into the AuthOAuth1
filestore contract), describing the expected input shape (config:
BrunoAuth['oauth1']) and the returned AuthOAuth1 object, and documenting the key
normalization behavior (especially how privateKey + privateKeyType produce
either {type: 'file', value: ...} or {type: 'text', value: ...}), plus notes on
optional fields like consumerKey, consumerSecret, accessToken,
accessTokenSecret, callbackUrl, verifier, signatureEncoding, timestamp, nonce,
version, realm, placement and the boolean includeBodyHash; include
examples/edge-cases (undefined config returns { type: 'oauth1' }) and any
invariants callers should rely on.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8fd6906a-e7df-4a52-836c-98e82a0695ad
📒 Files selected for processing (63)
packages/bruno-app/src/components/RequestPane/Auth/OAuth1/index.jspackages/bruno-app/src/utils/collections/index.jspackages/bruno-cli/src/runner/prepare-request.jspackages/bruno-converters/src/opencollection/common/auth.tspackages/bruno-converters/src/postman/postman-to-bruno.jspackages/bruno-converters/tests/postman/postman-to-bruno/process-auth.spec.jspackages/bruno-electron/src/ipc/network/prepare-request.jspackages/bruno-filestore/src/formats/yml/common/auth.tspackages/bruno-lang/v2/src/bruToJson.jspackages/bruno-lang/v2/src/collectionBruToJson.jspackages/bruno-lang/v2/src/jsonToBru.jspackages/bruno-lang/v2/src/jsonToCollectionBru.jspackages/bruno-lang/v2/tests/oauth1.spec.jspackages/bruno-requests/src/auth/oauth1-request-authorization.tspackages/bruno-schema-types/src/common/auth.tspackages/bruno-schema/src/collections/index.jstests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 401.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 Body 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 Body JSON 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 POST 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 Query Params 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA256 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA256 401.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA256 Body 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA512 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA512 401.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT 401.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT Body 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT Query Params 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 Body 200 formurlencoded.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 Body 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 File Key 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 Query Params 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 Variable Key 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA256 200.brutests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA512 200.brutests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 401.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Body 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Body JSON 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 POST 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Query Params 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 401.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 Body 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA512 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA512 401.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT 401.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT Body 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT Query Params 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Body 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Body formurlencoded 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 File Key 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Query Params 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 Variable Key 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA256 200.ymltests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA512 200.ymltests/auth/oauth1/oauth1-runner.spec.ts
✅ Files skipped from review due to trivial changes (36)
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 Query Params 200.bru
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT Query Params 200.yml
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 401.bru
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT 200.yml
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA512 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 Body JSON 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT 401.bru
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 POST 200.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 Body 200.yml
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 Body 200 formurlencoded.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 File Key 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 POST 200.bru
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT Body 200.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 PLAINTEXT 401.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA512 200.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Body 200.yml
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT Query Params 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA256 401.bru
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 200.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 401.yml
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 200.bru
- packages/bruno-electron/src/ipc/network/prepare-request.js
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Body JSON 200.yml
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA512 401.bru
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 401.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA256 200.yml
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA256 Body 200.bru
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 RSA-SHA1 File Key 200.yml
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA256 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 HMAC-SHA1 Body 200.bru
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA512 401.yml
- tests/auth/oauth1/fixtures/collections/yml/OAuth1 HMAC-SHA1 Query Params 200.yml
- tests/auth/oauth1/oauth1-runner.spec.ts
- packages/bruno-lang/v2/tests/oauth1.spec.js
🚧 Files skipped from review as they are similar to previous changes (6)
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 PLAINTEXT Body 200.bru
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA256 200.bru
- packages/bruno-lang/v2/src/jsonToBru.js
- tests/auth/oauth1/fixtures/collections/bru/OAuth1 RSA-SHA1 Query Params 200.bru
- packages/bruno-converters/src/postman/postman-to-bruno.js
- packages/bruno-converters/src/opencollection/common/auth.ts
Comment on lines
+385
to
+390
| const authorizer = createOAuth1Authorizer({ | ||
| consumer: { key: consumerKey, secret: consumerSecret }, | ||
| signature_method: (signatureEncoding || 'HMAC-SHA1') as SignatureMethod, | ||
| version: version || '1.0', | ||
| realm: realm || undefined, | ||
| private_key: resolvedPrivateKey |
Contributor
There was a problem hiding this comment.
Normalize consumerSecret before building the authorizer.
The shared OAuth1 model allows this field to be missing, but this path forwards it as-is while accessTokenSecret is already normalized later in the function. Blank or omitted consumer secrets will produce the wrong signing key for HMAC/PLAINTEXT requests.
💡 Minimal fix
const authorizer = createOAuth1Authorizer({
- consumer: { key: consumerKey, secret: consumerSecret },
+ consumer: { key: consumerKey, secret: consumerSecret ?? '' },
signature_method: (signatureEncoding || 'HMAC-SHA1') as SignatureMethod,
version: version || '1.0',
realm: realm || undefined,
private_key: resolvedPrivateKey
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-requests/src/auth/oauth1-request-authorization.ts` around
lines 385 - 390, Normalize consumerSecret the same way accessTokenSecret is
normalized before passing it to createOAuth1Authorizer: compute a
normalizedConsumerSecret (e.g., treat undefined/null/blank as empty string or
use the same normalization routine used for accessTokenSecret) and use that in
the authorizer's consumer.secret field so HMAC/PLAINTEXT signing keys are built
correctly; update the consumer: { key: consumerKey, secret: ... } passed to
createOAuth1Authorizer accordingly (references: consumerSecret,
accessTokenSecret, createOAuth1Authorizer, authorizer).
Comment on lines
+421
to
+426
| if (includeBodyHash && !isFormUrlEncoded) { | ||
| const bodyStr = request.data | ||
| ? (typeof request.data === 'string' ? request.data : JSON.stringify(request.data)) | ||
| : ''; | ||
| const bodyHash = computeBodyHash(bodyStr, (signatureEncoding || 'HMAC-SHA1') as SignatureMethod); | ||
| dataPairs.push(['oauth_body_hash', bodyHash]); |
Contributor
There was a problem hiding this comment.
Compute oauth_body_hash from the serialized payload bytes.
For non-form bodies this hashes JSON.stringify(request.data), which is not the wire payload for multipart params, file buffers, or streams. oauth_body_hash will be invalid for those request modes unless it is computed after serialization (or restricted to body types where the final bytes are already known).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-requests/src/auth/oauth1-request-authorization.ts` around
lines 421 - 426, The oauth_body_hash calculation in the includeBodyHash branch
currently serializes request.data via JSON.stringify (in the block around
includeBodyHash/isFormUrlEncoded and computeBodyHash), which is invalid for
multipart, file Buffers, or streams; change the logic to compute the hash from
the actual serialized request bytes after the request body is serialized (or
only compute it for known-safe body types). Specifically, move the
computeBodyHash call to run after the codepath that builds the wire payload (or
accept Buffer/Uint8Array bodies directly), detect and handle multipart/form-data
and stream/FileBuffer cases by hashing the final byte payload or skipping adding
oauth_body_hash for unsupported types, and keep using
signatureEncoding/computeBodyHash for the actual bytes so oauth_body_hash
reflects the real wire payload.
Comment on lines
+1
to
+63
| meta { | ||
| name: OAuth1 RSA-SHA512 200 | ||
| type: http | ||
| seq: 1 | ||
| } | ||
|
|
||
| get { | ||
| url: {{localhost}}/api/auth/oauth1/resource | ||
| body: none | ||
| auth: oauth1 | ||
| } | ||
|
|
||
| auth:oauth1 { | ||
| consumer_key: consumer_key_1 | ||
| consumer_secret: | ||
| access_token: access_token_1 | ||
| token_secret: token_secret_1 | ||
| callback_url: | ||
| verifier: | ||
| signature_method: RSA-SHA512 | ||
| private_key: ''' | ||
| -----BEGIN PRIVATE KEY----- | ||
| MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCf/ioOwb6uGm01 | ||
| WjL89sBOKmvMuwyGW33w+ud3eV5dWbHZbJTgCHMtjUVqGE4sVPnX8wfKgb00hVGZ | ||
| ZhbRsqquUMJL+7RHZA1G3ZcC2WPmrUvIpi13W7F/+wizRjDrutgK0oc4L7zUPhgE | ||
| +d+Qe6JHvFL376hqkD9Z6vd9jVNRDv7D2uJcG+Uc7ePM6yLzQeJILnfvSwxna89G | ||
| 6YxfbYNjf/VKAcRM6UAR8aRGm/A9Qmdwsy08iRzlM0K5lid7Ath7ADJ+D6ezUIhl | ||
| qiI6sVrZdpsFXzuvcxjgAts54XoQsLCDZudTmzMs4BftswULCQFdj6c2oye9QKsu | ||
| EoZ0CvwNAgMBAAECggEAA1kIJu2QQIhhB0rEjQfaF5309NW9JK/pag91+claW3hd | ||
| q6papc1yIN6MjfRwPlE7i8npZygL03uEAkJhRoYHOEU3AOwFZluw7hiuPkBaQiDC | ||
| Ld1RpOZlnRidoqgHV1y+LzZ0ieJwgGhu4ZEbnSRZIvMihqRHJo5aJQGGUuQ60r6w | ||
| Z5N6j7GGCV14oGck6+0k/NhYrkhpbyl+AHPZeQ20L9ZaSpl+GD3RUgvx4nOhN1Fx | ||
| agovDCKwmRPSpuuBw6Wun1hKC9MuuL89CCy2yZ+MrjQmQJ/onKZKR2fc1I5g2JRu | ||
| z7xObQq/tAXIHKM27YYY5J3NcGtsy9tLpdtFDle/wQKBgQDfBaBURNs/ccVMMXSZ | ||
| T5lOo337Rv3bGFnoLwURUZrjJNGrHqLzeZvTNt0sXQX6Jbdeb5qnC/icng+QDEod | ||
| 9hRVDz1rN+2YqG6AMvrX5dQSK1PmkwVHLles4sX7DsZaiKNYFFbXc9rt7kXBwDoE | ||
| LXYidqUIqZiOjMqCyNlfyHHnRQKBgQC3ppq7TCEWIfqLzsfOoqaKD/fDOrQLLmor | ||
| 7RvAdGa+EyVeB1G+NQO00KN6U9W+SNPz0cEUYUZQiAaAghUGkNurrS1shr5k7aTX | ||
| pXpXtaA+xSEAd6w8lTl9mAfwZMBCcsfjyjPf1RPjZ6Tj2fdHqnsllSU5843LOqZK | ||
| CBXiitdKKQKBgQCVNEloN0zLLE1HxUpxiwxQzRZqtrr9ClST/mkQhhzuW+Kd7fgs | ||
| la5HZ0we8vkdun/sARRhL6Qa+7ADugUX+Frv8SsxARDG8eBDilfBevQfV7dg6fk8 | ||
| /ucPNgQoC2Fujj1hnvHeYJcWWTN4BSeLRfLj6aZNnlD/BXgyeTbcWtjBVQKBgBhG | ||
| npd5hboePbczWzgWSftgBvk4jkoYFZK+4fc7q8UeVMcsIoMJEPdayPFHma5whAvr | ||
| wyEFhrzobiuYhlz60v7LgoChAxPmUe7rgdOMP6Vse2NLbmoHs7TFXu9I8h0WfRPA | ||
| S8EfsmRR8/rmeghwIZ0jLOuPJUQi+Y45qWLrxW+ZAoGAMXYhio9y93M0nRqjiiCR | ||
| YibnhpZxvrNiLPxNiUi/WxcWEvulpmbKxLUhiWJB1ZmRTiGYlnclQXUuRyaOQNTo | ||
| 5TVAaNzDXayWVbxhx3Lb8NUV+QNUJEJOgjq4+NYw8fUZCr7T64pGGM4DJHPuHCBo | ||
| dJv7UByPuMKBIOYpy3Z+iWs= | ||
| -----END PRIVATE KEY----- | ||
| ''' | ||
| timestamp: | ||
| nonce: | ||
| version: 1.0 | ||
| realm: | ||
| placement: header | ||
| include_body_hash: false | ||
| } | ||
|
|
||
| assert { | ||
| res.status: eq 200 | ||
| res.body.resource.name: eq oauth1-test-resource | ||
| res.body.resource.email: eq oauth1@example.com | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if there are multiple collection directories inside fixtures/collections
# to verify the plural naming is justified.
# List directories inside tests/auth/oauth1/fixtures/
echo "=== Contents of tests/auth/oauth1/fixtures/ ==="
fd --type d --max-depth 2 . tests/auth/oauth1/fixtures/ 2>/dev/null || find tests/auth/oauth1/fixtures/ -type d -maxdepth 2
# Check if there are multiple top-level directories under collections
echo ""
echo "=== Top-level items under collections (if exists) ==="
ls -la tests/auth/oauth1/fixtures/collections/ 2>/dev/null || echo "Directory not found"Repository: usebruno/bruno
Length of output: 501
Rename fixtures/collections to fixtures/collection (singular).
The fixture directory uses the plural form, but the naming convention specifies using fixtures/collection (singular) when there is a single collection. The bru/ and yml/ subdirectories are different file format representations of the same OAuth1 collection, not multiple separate collections.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/auth/oauth1/fixtures/collections/bru/OAuth1` RSA-SHA512 200.bru around
lines 1 - 63, The fixtures directory is misnamed as fixtures/collections
(plural) but should be fixtures/collection (singular); rename the directory from
fixtures/collections to fixtures/collection, move the subdirectories (e.g., bru
and yml containing files like "OAuth1 RSA-SHA512 200.bru") into the new singular
folder, and update any test references or imports that point to
fixtures/collections to the new fixtures/collection path so tests and loaders
(the OAuth1 collection fixtures) resolve correctly.
Comment on lines
+20
to
+50
| privateKey: | ||
| type: text | ||
| value: | | ||
| -----BEGIN PRIVATE KEY----- | ||
| MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCf/ioOwb6uGm01 | ||
| WjL89sBOKmvMuwyGW33w+ud3eV5dWbHZbJTgCHMtjUVqGE4sVPnX8wfKgb00hVGZ | ||
| ZhbRsqquUMJL+7RHZA1G3ZcC2WPmrUvIpi13W7F/+wizRjDrutgK0oc4L7zUPhgE | ||
| +d+Qe6JHvFL376hqkD9Z6vd9jVNRDv7D2uJcG+Uc7ePM6yLzQeJILnfvSwxna89G | ||
| 6YxfbYNjf/VKAcRM6UAR8aRGm/A9Qmdwsy08iRzlM0K5lid7Ath7ADJ+D6ezUIhl | ||
| qiI6sVrZdpsFXzuvcxjgAts54XoQsLCDZudTmzMs4BftswULCQFdj6c2oye9QKsu | ||
| EoZ0CvwNAgMBAAECggEAA1kIJu2QQIhhB0rEjQfaF5309NW9JK/pag91+claW3hd | ||
| q6papc1yIN6MjfRwPlE7i8npZygL03uEAkJhRoYHOEU3AOwFZluw7hiuPkBaQiDC | ||
| Ld1RpOZlnRidoqgHV1y+LzZ0ieJwgGhu4ZEbnSRZIvMihqRHJo5aJQGGUuQ60r6w | ||
| Z5N6j7GGCV14oGck6+0k/NhYrkhpbyl+AHPZeQ20L9ZaSpl+GD3RUgvx4nOhN1Fx | ||
| agovDCKwmRPSpuuBw6Wun1hKC9MuuL89CCy2yZ+MrjQmQJ/onKZKR2fc1I5g2JRu | ||
| z7xObQq/tAXIHKM27YYY5J3NcGtsy9tLpdtFDle/wQKBgQDfBaBURNs/ccVMMXSZ | ||
| T5lOo337Rv3bGFnoLwURUZrjJNGrHqLzeZvTNt0sXQX6Jbdeb5qnC/icng+QDEod | ||
| 9hRVDz1rN+2YqG6AMvrX5dQSK1PmkwVHLles4sX7DsZaiKNYFFbXc9rt7kXBwDoE | ||
| LXYidqUIqZiOjMqCyNlfyHHnRQKBgQC3ppq7TCEWIfqLzsfOoqaKD/fDOrQLLmor | ||
| 7RvAdGa+EyVeB1G+NQO00KN6U9W+SNPz0cEUYUZQiAaAghUGkNurrS1shr5k7aTX | ||
| pXpXtaA+xSEAd6w8lTl9mAfwZMBCcsfjyjPf1RPjZ6Tj2fdHqnsllSU5843LOqZK | ||
| CBXiitdKKQKBgQCVNEloN0zLLE1HxUpxiwxQzRZqtrr9ClST/mkQhhzuW+Kd7fgs | ||
| la5HZ0we8vkdun/sARRhL6Qa+7ADugUX+Frv8SsxARDG8eBDilfBevQfV7dg6fk8 | ||
| /ucPNgQoC2Fujj1hnvHeYJcWWTN4BSeLRfLj6aZNnlD/BXgyeTbcWtjBVQKBgBhG | ||
| npd5hboePbczWzgWSftgBvk4jkoYFZK+4fc7q8UeVMcsIoMJEPdayPFHma5whAvr | ||
| wyEFhrzobiuYhlz60v7LgoChAxPmUe7rgdOMP6Vse2NLbmoHs7TFXu9I8h0WfRPA | ||
| S8EfsmRR8/rmeghwIZ0jLOuPJUQi+Y45qWLrxW+ZAoGAMXYhio9y93M0nRqjiiCR | ||
| YibnhpZxvrNiLPxNiUi/WxcWEvulpmbKxLUhiWJB1ZmRTiGYlnclQXUuRyaOQNTo | ||
| 5TVAaNzDXayWVbxhx3Lb8NUV+QNUJEJOgjq4+NYw8fUZCr7T64pGGM4DJHPuHCBo | ||
| dJv7UByPuMKBIOYpy3Z+iWs= | ||
| -----END PRIVATE KEY----- |
Contributor
There was a problem hiding this comment.
Remove inline PEM private key from fixture content.
Line 23-Line 50 embeds a private key directly in-repo. This is being flagged by secret scanners and should be externalized (e.g., injected via test env/fixture setup) to avoid sensitive-material sprawl and alert fatigue in CI.
🧰 Tools
🪛 Checkov (3.2.508)
[medium] 22-23: Private Key
(CKV_SECRET_13)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` RSA-SHA1 Body
formurlencoded 200.yml around lines 20 - 50, The fixture currently embeds a PEM
private key inline under the YAML field privateKey -> value; remove that inline
PEM and instead reference an external secret (e.g., use an environment variable
like OAUTH1_PRIVATE_KEY or load from a secure test fixture file) and update the
test fixture loader to substitute that value at runtime; specifically, replace
the multiline value in the privateKey field with a placeholder and modify the
test setup/fixture-parsing code to inject process.env.OAUTH1_PRIVATE_KEY (or
read from a secure file) so the key is not stored in-repo.
Comment on lines
+15
to
+45
| privateKey: | ||
| type: text | ||
| value: | | ||
| -----BEGIN PRIVATE KEY----- | ||
| MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCf/ioOwb6uGm01 | ||
| WjL89sBOKmvMuwyGW33w+ud3eV5dWbHZbJTgCHMtjUVqGE4sVPnX8wfKgb00hVGZ | ||
| ZhbRsqquUMJL+7RHZA1G3ZcC2WPmrUvIpi13W7F/+wizRjDrutgK0oc4L7zUPhgE | ||
| +d+Qe6JHvFL376hqkD9Z6vd9jVNRDv7D2uJcG+Uc7ePM6yLzQeJILnfvSwxna89G | ||
| 6YxfbYNjf/VKAcRM6UAR8aRGm/A9Qmdwsy08iRzlM0K5lid7Ath7ADJ+D6ezUIhl | ||
| qiI6sVrZdpsFXzuvcxjgAts54XoQsLCDZudTmzMs4BftswULCQFdj6c2oye9QKsu | ||
| EoZ0CvwNAgMBAAECggEAA1kIJu2QQIhhB0rEjQfaF5309NW9JK/pag91+claW3hd | ||
| q6papc1yIN6MjfRwPlE7i8npZygL03uEAkJhRoYHOEU3AOwFZluw7hiuPkBaQiDC | ||
| Ld1RpOZlnRidoqgHV1y+LzZ0ieJwgGhu4ZEbnSRZIvMihqRHJo5aJQGGUuQ60r6w | ||
| Z5N6j7GGCV14oGck6+0k/NhYrkhpbyl+AHPZeQ20L9ZaSpl+GD3RUgvx4nOhN1Fx | ||
| agovDCKwmRPSpuuBw6Wun1hKC9MuuL89CCy2yZ+MrjQmQJ/onKZKR2fc1I5g2JRu | ||
| z7xObQq/tAXIHKM27YYY5J3NcGtsy9tLpdtFDle/wQKBgQDfBaBURNs/ccVMMXSZ | ||
| T5lOo337Rv3bGFnoLwURUZrjJNGrHqLzeZvTNt0sXQX6Jbdeb5qnC/icng+QDEod | ||
| 9hRVDz1rN+2YqG6AMvrX5dQSK1PmkwVHLles4sX7DsZaiKNYFFbXc9rt7kXBwDoE | ||
| LXYidqUIqZiOjMqCyNlfyHHnRQKBgQC3ppq7TCEWIfqLzsfOoqaKD/fDOrQLLmor | ||
| 7RvAdGa+EyVeB1G+NQO00KN6U9W+SNPz0cEUYUZQiAaAghUGkNurrS1shr5k7aTX | ||
| pXpXtaA+xSEAd6w8lTl9mAfwZMBCcsfjyjPf1RPjZ6Tj2fdHqnsllSU5843LOqZK | ||
| CBXiitdKKQKBgQCVNEloN0zLLE1HxUpxiwxQzRZqtrr9ClST/mkQhhzuW+Kd7fgs | ||
| la5HZ0we8vkdun/sARRhL6Qa+7ADugUX+Frv8SsxARDG8eBDilfBevQfV7dg6fk8 | ||
| /ucPNgQoC2Fujj1hnvHeYJcWWTN4BSeLRfLj6aZNnlD/BXgyeTbcWtjBVQKBgBhG | ||
| npd5hboePbczWzgWSftgBvk4jkoYFZK+4fc7q8UeVMcsIoMJEPdayPFHma5whAvr | ||
| wyEFhrzobiuYhlz60v7LgoChAxPmUe7rgdOMP6Vse2NLbmoHs7TFXu9I8h0WfRPA | ||
| S8EfsmRR8/rmeghwIZ0jLOuPJUQi+Y45qWLrxW+ZAoGAMXYhio9y93M0nRqjiiCR | ||
| YibnhpZxvrNiLPxNiUi/WxcWEvulpmbKxLUhiWJB1ZmRTiGYlnclQXUuRyaOQNTo | ||
| 5TVAaNzDXayWVbxhx3Lb8NUV+QNUJEJOgjq4+NYw8fUZCr7T64pGGM4DJHPuHCBo | ||
| dJv7UByPuMKBIOYpy3Z+iWs= | ||
| -----END PRIVATE KEY----- |
Contributor
There was a problem hiding this comment.
Avoid committing inline private keys in test fixtures.
Committing PEM key material in-repo is a security/compliance risk and also creates secret-scanner noise. Prefer injecting this via test env/fixture variables and keep only a reference in YAML.
🔧 Suggested change
privateKey:
type: text
- value: |
- -----BEGIN PRIVATE KEY-----
- MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCf/ioOwb6uGm01
- WjL89sBOKmvMuwyGW33w+ud3eV5dWbHZbJTgCHMtjUVqGE4sVPnX8wfKgb00hVGZ
- ZhbRsqquUMJL+7RHZA1G3ZcC2WPmrUvIpi13W7F/+wizRjDrutgK0oc4L7zUPhgE
- +d+Qe6JHvFL376hqkD9Z6vd9jVNRDv7D2uJcG+Uc7ePM6yLzQeJILnfvSwxna89G
- 6YxfbYNjf/VKAcRM6UAR8aRGm/A9Qmdwsy08iRzlM0K5lid7Ath7ADJ+D6ezUIhl
- qiI6sVrZdpsFXzuvcxjgAts54XoQsLCDZudTmzMs4BftswULCQFdj6c2oye9QKsu
- EoZ0CvwNAgMBAAECggEAA1kIJu2QQIhhB0rEjQfaF5309NW9JK/pag91+claW3hd
- q6papc1yIN6MjfRwPlE7i8npZygL03uEAkJhRoYHOEU3AOwFZluw7hiuPkBaQiDC
- Ld1RpOZlnRidoqgHV1y+LzZ0ieJwgGhu4ZEbnSRZIvMihqRHJo5aJQGGUuQ60r6w
- Z5N6j7GGCV14oGck6+0k/NhYrkhpbyl+AHPZeQ20L9ZaSpl+GD3RUgvx4nOhN1Fx
- agovDCKwmRPSpuuBw6Wun1hKC9MuuL89CCy2yZ+MrjQmQJ/onKZKR2fc1I5g2JRu
- z7xObQq/tAXIHKM27YYY5J3NcGtsy9tLpdtFDle/wQKBgQDfBaBURNs/ccVMMXSZ
- T5lOo337Rv3bGFnoLwURUZrjJNGrHqLzeZvTNt0sXQX6Jbdeb5qnC/icng+QDEod
- 9hRVDz1rN+2YqG6AMvrX5dQSK1PmkwVHLles4sX7DsZaiKNYFFbXc9rt7kXBwDoE
- LXYidqUIqZiOjMqCyNlfyHHnRQKBgQC3ppq7TCEWIfqLzsfOoqaKD/fDOrQLLmor
- 7RvAdGa+EyVeB1G+NQO00KN6U9W+SNPz0cEUYUZQiAaAghUGkNurrS1shr5k7aTX
- pXpXtaA+xSEAd6w8lTl9mAfwZMBCcsfjyjPf1RPjZ6Tj2fdHqnsllSU5843LOqZK
- CBXiitdKKQKBgQCVNEloN0zLLE1HxUpxiwxQzRZqtrr9ClST/mkQhhzuW+Kd7fgs
- la5HZ0we8vkdun/sARRhL6Qa+7ADugUX+Frv8SsxARDG8eBDilfBevQfV7dg6fk8
- /ucPNgQoC2Fujj1hnvHeYJcWWTN4BSeLRfLj6aZNnlD/BXgyeTbcWtjBVQKBgBhG
- npd5hboePbczWzgWSftgBvk4jkoYFZK+4fc7q8UeVMcsIoMJEPdayPFHma5whAvr
- wyEFhrzobiuYhlz60v7LgoChAxPmUe7rgdOMP6Vse2NLbmoHs7TFXu9I8h0WfRPA
- S8EfsmRR8/rmeghwIZ0jLOuPJUQi+Y45qWLrxW+ZAoGAMXYhio9y93M0nRqjiiCR
- YibnhpZxvrNiLPxNiUi/WxcWEvulpmbKxLUhiWJB1ZmRTiGYlnclQXUuRyaOQNTo
- 5TVAaNzDXayWVbxhx3Lb8NUV+QNUJEJOgjq4+NYw8fUZCr7T64pGGM4DJHPuHCBo
- dJv7UByPuMKBIOYpy3Z+iWs=
- -----END PRIVATE KEY-----
+ value: "{{oauth1_rsa_sha256_private_key}}"🧰 Tools
🪛 Betterleaks (1.1.1)
[high] 18-45: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
🪛 Checkov (3.2.508)
[medium] 17-18: Private Key
(CKV_SECRET_13)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/auth/oauth1/fixtures/collections/yml/OAuth1` RSA-SHA256 200.yml around
lines 15 - 45, The fixture embeds a real PEM under the YAML key privateKey in
"OAuth1 RSA-SHA256 200.yml"; remove the inline PEM and replace it with a
non-sensitive reference (e.g., a placeholder string or an env/fixture variable
reference) so tests load the actual private key from the test environment or a
secrets fixture at runtime; update any test helper that reads this fixture
(lookup by privateKey) to read process.env (or the designated fixture loader)
when the placeholder is present.
Add full OAuth 1.0 (RFC 5849) authentication with support for HMAC-SHA1/256/512, RSA-SHA1/256/512, and PLAINTEXT signature methods. Includes UI components, bru/yml serialization, Postman import, code generation, CLI support, and comprehensive playwright and unit tests.
…ests Avoid tripping secret scanners by using obviously fake BEGIN/END markers and non-sensitive base64 content in serialization and round-trip tests.
OAuth1 requires runtime-computed nonce, timestamp, and signature that cannot be pre-computed for a static code snippet. Return an empty array instead of emitting an Authorization header with literal <signature>, <timestamp>, <nonce> placeholders.
The oauth1 switch branch was dead code since it was not in supportedAuthModes and the useEffect would reset it to 'none' before it could render.
- tokenSecret → accessTokenSecret - signatureMethod → signatureEncoding - addParamsTo value 'queryparams' → 'query'
…ollection/types to 0.9.0
…e race condition - Rename OAuth1 signatureEncoding to signatureMethod across all packages - Fix timeline showing "No Headers/Body found" when request-sent IPC event arrives after response by retroactively updating the timeline entry - Store requestUid in timeline entries for precise matching - Correct timeline entry timestamp on retroactive update for proper sort order
- Add CLI tests that run full BRU and YML collections via bru run - Add start-test-server actions for Linux, macOS, and Windows - Move auth e2e and setup actions under auth/oauth1/ directory - Fix Windows Playwright failures caused by unescaped backslashes in collectionPath template variable
bijin-bruno
previously approved these changes
Mar 26, 2026
This was referenced Apr 2, 2026
4 of 5 tasks
2 of 4 tasks
6 tasks
4 of 6 tasks
1 task
maxenceleblanc
pushed a commit
to maxenceleblanc/curly_cats2
that referenced
this pull request
Jul 20, 2026
* feat: add OAuth 1.0 authentication support Add full OAuth 1.0 (RFC 5849) authentication with support for HMAC-SHA1/256/512, RSA-SHA1/256/512, and PLAINTEXT signature methods. Includes UI components, bru/yml serialization, Postman import, code generation, CLI support, and comprehensive playwright and unit tests. * test: replace real-looking PEM literals with fake markers in oauth1 tests Avoid tripping secret scanners by using obviously fake BEGIN/END markers and non-sensitive base64 content in serialization and round-trip tests. * fix: remove invalid OAuth1 placeholder header from code generator OAuth1 requires runtime-computed nonce, timestamp, and signature that cannot be pre-computed for a static code snippet. Return an empty array instead of emitting an Authorization header with literal <signature>, <timestamp>, <nonce> placeholders. * fix: remove unreachable oauth1 case from WSAuth component The oauth1 switch branch was dead code since it was not in supportedAuthModes and the useEffect would reset it to 'none' before it could render. * fix: remove unused collectionPath param and use path.basename for filename extraction * refactor: rename OAuth1 fields for clarity - tokenSecret → accessTokenSecret - signatureMethod → signatureEncoding - addParamsTo value 'queryparams' → 'query' * refactor: rename addParamsTo to placement in OAuth1 auth * fix: add missing oauth1: null in buildOAuth2Config and upgrade @opencollection/types to 0.9.0 * test: add oauth1 import tests and fix missing oauth1: null in auth assertions * ci: add auth playwright tests workflow for Linux, macOS, and Windows * refactor: rename signatureEncoding to signatureMethod and fix timeline race condition - Rename OAuth1 signatureEncoding to signatureMethod across all packages - Fix timeline showing "No Headers/Body found" when request-sent IPC event arrives after response by retroactively updating the timeline entry - Store requestUid in timeline entries for precise matching - Correct timeline entry timestamp on retroactive update for proper sort order * ci: add OAuth1 CLI tests and reorganize auth actions under oauth1/ - Add CLI tests that run full BRU and YML collections via bru run - Add start-test-server actions for Linux, macOS, and Windows - Move auth e2e and setup actions under auth/oauth1/ directory - Fix Windows Playwright failures caused by unescaped backslashes in collectionPath template variable * ci: reorder auth tests to run E2E tests before CLI tests * ci: start test server after E2E tests to fix port 8081 conflict
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
jira
opencollection pr: opencollection-dev/opencollection#25
Description
Add OAuth 1.0 (RFC 5849) authentication with support for HMAC-SHA1/256/512, RSA-SHA1/256/512, and PLAINTEXT signature methods.
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit