PrefectHQ / prefect

Prefect is a workflow orchestration framework for building resilient data pipelines in Python.
https://prefect.io
Apache License 2.0
15.95k stars 1.57k forks source link

BUG: MS Teams Webhook notification has incorrect obfuscation #7017

Open znicholasbrown opened 2 years ago

znicholasbrown commented 2 years ago

First check

Bug summary

The webhook input uses type="password" to obfuscate the value, despite it being visible just below

Reproduction

Create a new MS Teams webhook notification

Error

Screen Shot 2022-09-29 at 12 39 10 PM

Browers

Prefect version

2.4.3

Additional context

N/A

pleek91 commented 1 year ago

@znicholasbrown I don't think this is a ui issue. The schema from the api is what's causing this to be treated like a password.

{
  "type": "string",
  "title": "Webhook URL",
  "format": "password",
  "example": "https://your-org.webhook.office.com/webhookb2/XXX/IncomingWebhook/YYY/ZZZ",
  "writeOnly": true,
  "description": "The Teams incoming webhook URL used to send notifications."
}
zanieb commented 1 year ago

Webhook URLs are secret though right? Isn't the issue here that it's being displayed below?

marichka-offen commented 1 year ago

@znicholasbrown @pleek91 the screenshot for the issue is made on create notification page, that's why we can see the hook I assume, so we can validate one last time it is correct. On the general notifications dashboard, it's starred. I just want to confirm we want it hidden on create notification page as well

Image

marichka-offen commented 1 year ago

Also, when we go back to edit, it is starred as well. So the only time you see it, is when creating notification

Image

znicholasbrown commented 1 year ago

I think the issue here is that the field probably doesn't need to be obfuscated as a user types since that makes it difficult to identify and correct mistakes; I think it's ok if the final value is obfuscated when being displayed but not the input one. As @pleek91 mentioned though, I don't think this is a UI issue per se, unless we want to make a blanket rule that password input types are visible when interacting with the input field and obfuscated otherwise. Either way I think this is low priority