Added cookie support to CLI requests #2820
Conversation
|
Hey @matthewdickinson ! Thanks for taking time to work on this PR. Just tested it. Implementation is working as expected. Ref: https://github.com/usebruno/bruno/blob/main/packages/bruno-electron/src/utils/cookies.js |
|
Fully agree with @helloanoop However, Please keep in mind that there are still bugs in our current cookie implementation in the app, just like #2682, #2950 or #1332 so I suppose while migrating the current app implementation to CLI, you might first want to address those possibly or as least beware that those functions are currently not working as expected and therefore also will not when migrated to CLI. |
|
I will see what I can do to change the implementation to be closer to the GUI's implementation. However, I only have time to look at this every few weeks, but I might not get back to it until the end of the month. Thanks! |
matthewdickinson
force-pushed
the
feature/cli-cookies
branch
from
02982ba
to
57ccfb0
Compare
matthewdickinson
force-pushed
the
feature/cli-cookies
branch
from
57ccfb0
to
b82a2c3
Compare
There was a problem hiding this comment.
@helloanoop @nikischin Would you be willing to look at this again? I switched it to use the (exact) same logic as the Electron app.
It seems like the util could be centralized, but I left a question for you about that.
There was a problem hiding this comment.
This is (almost) an exact copy of the util in the electron app, but I'm not sure where the mainainers would like to centralize this. bruno-common seems like it would make sense, but right now it doesn't seem to have any dependencies this this would require.
| const saveCookies = (url, headers) => { | ||
| let setCookieHeaders = []; | ||
| if (headers['set-cookie']) { | ||
| setCookieHeaders = Array.isArray(headers['set-cookie']) | ||
| ? headers['set-cookie'] | ||
| : [headers['set-cookie']]; | ||
| for (let setCookieHeader of setCookieHeaders) { | ||
| if (typeof setCookieHeader === 'string' && setCookieHeader.length) { | ||
| addCookieToJar(setCookieHeader, url); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I coped this from packages/bruno-electron/src/ipc/network/index.js since it seemed like it could be centralized to this util once they both used the same util.
Hi @matthewdickinson, thank you so much, I really appreciate that! I would be willing to test the cli, however, I haven't used the cli before and also I have no idea how to run the cli version of this branch and not the one installed with the latest version. I would definitely benefit from this PR though, as if I want to use the cli in the future, I will definitely need this feature. Hope it gets merged soon! @lohxt1 is assigned for review :) |
Description
Right now, Bruno CLI doesn't support cookies. Many of our collections include an "Auth" request that we can use to get a auth cookie that will be used on subsequent requests. However, without cookies, we can't run the same collections (e.g. treat them as a test suite) without adding complicated pre-request scripts to fake the cookie first.
This adds a
--use-cookiesflag that (if set totrue) creates a cookie store for this invocation ofbru. Hence, no existing use-cases should be impacted, but this feature would be available for those who wish to use it.This also should work in both standard and Proxy (HTTP or SOCKS) configurations.
Contribution Checklist: