Fix editing connection with sensitive extra field #52403
Conversation
|
Not a critical fix, maybe land it for 3.1? |
|
@amoghrajesh Updating the connection with sensitive value is a problem and result in failures since it will change the values. Would be good to have fix. |
I'd propose to back-port, else in 3.0.x it messes-up the secrets if you edit and save via connections form. |
|
I tested locally and the standard fields are still published redacted... and then when savong three stars are stored as password. UPDATE: Uuups had an issue to switch to correct branch - after making the review "right" all is working as expected... |
|
Reference to slack discussion: https://apache-airflow.slack.com/archives/C0809U4S1Q9/p1751108215738019 |
Backport failed to create: v3-0-test. View the failure log Run details
You can attempt to backport this manually by running: cherry_picker 8ef792d v3-0-testThis should apply the commit to the v3-0-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continue |
…#52403) * Handle unchanges json * Remove the redact from connections * Fix the static checks (cherry picked from commit 8ef792d) Co-authored-by: Shubham Raj <[email protected]>
#52445) * Handle unchanges json * Remove the redact from connections * Fix the static checks (cherry picked from commit 8ef792d) Co-authored-by: Shubham Raj <[email protected]>
|
Thanks! Yeah I think it should be ok with show unredacted value for people to edit, it is a really bad experience (atleast for me) to find that in the "airflow connections export", change it in the UI, if im experimenting especially. If someone is changing stuff from the UI, they are authenticated at the very least. |
| @@ -94,6 +94,14 @@ const ConnectionForm = ({ | |||
| mutateConnection(data); | |||
| }; | |||
|
|
|||
| const hasChanges = () => { | |||
There was a problem hiding this comment.
Nit. functions should start with a verb.
But we don't even need a function here.
const hasChanges = isDirty || (JSON.stringify(JSON.parse(extra)) !== JSON.stringify(JSON.parse(initialConnection.extra)))
There was a problem hiding this comment.
And whats the point of parsing first?
There was a problem hiding this comment.
Actually yes, in current state func shouldn't be, In starting I was doing multiple things and debugging, later should have changed it.
Reason for parsing, A user can add something from cli which would not be in UI formatted way, it should not be treated as different value.
closes: #52301
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.