search

getodk / briefcase

ODK Briefcase is a Java application for fetching and pushing forms and their contents. It helps make billions of data points from ODK portable. Contribute and make the world a better place! ✨💼✨
https://docs.getodk.org/briefcase-intro
Other
60 stars 154 forks source link

Issue #801 Restore cookies to interact with Aggregate #805

Closed ggalmazor closed 5 years ago

ggalmazor commented 5 years ago

Closes #801

What has been done to verify that this works as intended?

Tested to push to Aggregate v2 (sandbox), Aggregate v1.17 in GAE, and Central (sandbox)

When pushing to Aggregate, I made sure to disable anonymous credentials to force Briefcase to use credentials and authenticate into the server.

Why is this the best possible solution? Were any other approaches considered?

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

In v1.16.3 we changed the cookie spec to IGNORE_COOKIES to work around an issue with multiple auth methods sent to Central servers. Doing so broke auth with Aggregate servers.

Since the cookie spec is set at an HTTP client level inside the executor, we can't create it each time we need to send a request because that would prevent requests to the same host to reuse connections. Having two executors (one with cookies, one without) is not an option either because it would leak too much of the internal workings.

Due to this, now we let the Request object tell the HTTP adapter when it needs to use cookies or not. Instead of ignoring cookies, we clear the cookie store before sending the request, which is not the same concept but achieves the same effect.

Does this change require updates to documentation? If so, please file an issue at https://github.com/opendatakit/docs/issues/new and include the link below.

Nope.

kkrawczyk123 commented 5 years ago

Testes with success! Confirmed: manual and cmd push on Ubuntu, MacOS and Windows with Aggregate 2.0, 1.7, 1.4 and Central.

@opendatakit-bot unlabel "needs testing" @opendatakit-bot label "behavior verified"