feat(base-driver): Use markSensitive to mask password input field value
#21118
Conversation
c628fa6 to
075ce05
Compare
893f909 to
8cfc4f0
Compare
|
Would using Settings API be nice for leveraging this feature for all clients? |
Interesting. Using a setting would require a call before (to enable) and after (to disable) the setting API, so there would be three requests if we also count the My main goal is to mask secrets on demand since, for example, in our case, we only require this feature for each test when entering the user's account password. We do not want masking to apply when filling in all other inputs. Therefore, a header makes it concise to a single request and easier to manage. I would see it as a setting if all the requests of the entire test case needed masking, but this is not necessary for now. |
|
@dprevost-LMI I've tried to collect all findings we've made so far in #21123 |
|
@dprevost-LMI We have recently switched the master branch, since we do not maintain Appium 2 anymore and only want to contribute to Appium 3. Could you please recreate this PR with master? |
🤔 I can, it's unfortunate; I didn't know that otherwise I would have pushed myslef a bit 😅 Moreover, the other part you did was already merged, anyway not sure we can do a release just for that 😄 |
41681fe to
92680d9
Compare
markSensitive to mask password input field valuemarkSensitive to mask password input field value
|
Thanks a lot for your help and patience @mykola-mokhnach |
- Dependant [Appium PR](appium/appium#21118) got merge and be available only in v3 - Fix missing main title in page + missing end tag for info sections - Readjust some documentation - Add Best Practice section
…wing PR merge (#14547) * Fix doc glitches + update Appium required version following PR merge - Dependant [Appium PR](appium/appium#21118) got merge and be available only in v3 - Fix missing main title in page + missing end tag for info sections - Readjust some documentation - Add Best Practice section * Make log example more visible * Update website/docs/Security.md --------- Co-authored-by: Christian Bromann <git@bromann.dev>
Proposed changes
As shown in the image above, when entering a value in a field like a password, appium leaks the value at the debug and info level. Since the above can also happen in a cloud provider like BrowserStack, leaking it even at the debug log level can compromise an actual production user account.
Since this approach would be the simplest and quickest, I would like to explicitly exclude it before going into full implementation of the dynamic regular expression to filter the logs as specified here
By using
markSensitiveto leverage the headerX-Appium-Is-Sensitivefrom the clever solution of PR #21123. We can now mask the password value and safely enter the passwords in input fields without worrying about leaks in logs (when no plugins are used 😉 ).Pros:
Cons:
markSensitivefunction.Remaining tasks:
These are draft changes, and if the idea is accepted, I will implement all required unit tests
Types of changes
What types of changes does you 8000 r code introduce to Appium?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...