Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Bug]: Api key query param fails to mask secret when query param value has & special chars in it #33742

Closed
1 task done
rishabhrathod01 opened this issue May 27, 2024 · 0 comments · Fixed by #33720
Closed
1 task done

[Bug]: Api key query param fails to mask secret when query param value has & special chars in it #33742

rishabhrathod01 opened this issue May 27, 2024 · 0 comments · Fixed by #33720
Assignees
@rishabhrathod01
Labels
BE Coders Pod Issues related to users writing code to fetch and update data Bug Something isn't working Integrations Pod General Issues related to the Integrations Pod that don't fit into other tags. Integrations Pod Issues related to a specific integration Medium Issues that frustrate users due to poor UX Needs Triaging Needs attention from maintainers to triage QA Pod Issues under the QA Pod QA Needs QA attention Release

Comments

@rishabhrathod01
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Description

Api key query param fails to mask secret when query param value has & special chars in it

Steps To Reproduce

  1. Go to application
  2. Create an authenticated API datasource with API key as authentication type and addTo queryParam
  3. Enter label="secret" and value="some&abc=123" and save the datasource
  4. Create an API using the datasource and run it.
  5. Check logs and you would notice masking issue in the url.

Public Sample App

No response

Environment

Release

Severity

Medium (Frustrating UX)

Issue video log

No response

Version

release
@rishabhrathod01 rishabhrathod01 added Bug Something isn't working Needs Triaging Needs attention from maintainers to triage labels May 27, 2024
@Nikhil-Nandagopal Nikhil-Nandagopal added Medium Issues that frustrate users due to poor UX Release labels May 27, 2024
@rishabhrathod01 rishabhrathod01 mentioned this issue May 27, 2024
Merged
2 tasks
@rishabhrathod01 rishabhrathod01 added the Integrations Pod General Issues related to the Integrations Pod that don't fit into other tags. label May 27, 2024
@github-actions github-actions bot added the Integrations Pod Issues related to a specific integration label May 27, 2024
rishabhrathod01 added a commit that referenced this issue May 27, 2024
@rishabhrathod01
fix: encode queryParam value and fix parsing logic for queryParams (#…
43bd032 
…33720)

## Description

- Add encoding for Api key secret value before storing it as query param
to avoid incorrect parsing.
- Remove hardcoded "api_key" check to mask headers.
- Used standard URL parsing APIs instead of custom
- Added unit test to cover the case 

Fixes #33742
flaws in the implementation
[here](#33528)

## Automation

/test datasource

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!IMPORTANT]
> 🟣 🟣 🟣 Your tests are running.
> Tests running at:
<https://github.com/appsmithorg/appsmith/actions/runs/9256269257>
> Commit: 602b0f5
> Workflow: `PR Automation test suite`
> Tags: `@tag.Datasource`

<!-- end of auto-generated comment: Cypress test results  -->






## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No
@appsmith-bot appsmith-bot added the QA Needs QA attention label May 27, 2024
@github-actions github-actions bot added BE Coders Pod Issues related to users writing code to fetch and update data QA Pod Issues under the QA Pod labels May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rishabhrathod01 rishabhrathod01

Labels
BE Coders Pod Issues related to users writing code to fetch and update data Bug Something isn't working Integrations Pod General Issues related to the Integrations Pod that don't fit into other tags. Integrations Pod Issues related to a specific integration Medium Issues that frustrate users due to poor UX Needs Triaging Needs attention from maintainers to triage QA Pod Issues under the QA Pod QA Needs QA attention Release
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: encode queryParam value and fix parsing logic for queryParams appsmithorg/appsmith
4 participants
@Nikhil-Nandagopal @pranavkanade @rishabhrathod01 @appsmith-bot