-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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.
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
docs: add danger note to auth guide #35409
base: main
Are you sure you want to change the base?
Conversation
@@ -13,6 +13,11 @@ Regardless of the authentication strategy you choose, you are likely to store au | |||
|
|||
We recommend to create `playwright/.auth` directory and add it to your `.gitignore`. Your authentication routine will produce authenticated browser state and save it to a file in this `playwright/.auth` directory. Later on, tests will reuse this state and start already authenticated. | |||
|
|||
:::danger | |||
The browser state file may contain sensitive cookies and headers that could be used to impersonate you or your test account. | |||
We strongly discourage checking them into private or public repositories. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We strongly discourage checking them into private or public repositories. | |
We strongly discourage checking them into private or public repositories and adding it to your `.gitignore` file. |
@@ -13,6 +13,11 @@ Regardless of the authentication strategy you choose, you are likely to store au | |||
|
|||
We recommend to create `playwright/.auth` directory and add it to your `.gitignore`. Your authentication routine will produce authenticated browser state and save it to a file in this `playwright/.auth` directory. Later on, tests will reuse this state and start already authenticated. | |||
|
|||
:::danger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would love the auth guide to not have a BIG RED WARNING on the first page!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's big red warning worthy, IMO. I don't suggest it lightly and do appreciate not wanting to decorate the docs with a bunch of warnings. However, the consequence of checking these auth files in under many circumstances is full account takeover, and there's evidence many folks are not aware of the risks of checking these in.
That being said, perhaps there's something we could implement to make it safe(r)-by-default.
Some alternative ideas I had (with various degrees of effectiveness vs. implementationd difficulty vs. DX):
-
Add
playwright/.auth
to the Create Playwright template, and hope folks heed the recommendation to put this files in the.auth/
directory. -
Have some sort of explicit safelist for headers that the user could configure with everything off-by-default if — and only if — playwright detects the file is not in
.gitignore
. -
Provide users a specific API for marking sensitive state files and ensure PW puts them in a location outside the tree1:
{ name: 'firefox', use: { ...devices['Desktop Firefox'], // Use prepared auth state. storageState: { sensitive: true, 'playwright/.auth/user.json' }, }, dependencies: ['setup'], },
1: It's important they are put outside the working directory/tree since some folks will decide to by default upload the entire plawright/*
directory as a build artifact and it would be bad if these files were slurped up into it.
wdyt? My vote would be something like #3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dgozman - or perhaps it could be as simple as:
- Do not add the big red warning,
- But update the example docs to have the user store the files in a temp dir:
{
name: 'firefox',
use: {
...devices['Desktop Firefox'],
// Use prepared auth state.
storageState: `${mkdirtemp()}/playwright/.auth/user.json`,
},
dependencies: ['setup'],
},
],
No description provided.