iterative / PyDrive2

Google Drive API Python wrapper library. Maintained fork of PyDrive.
https://docs.iterative.ai/PyDrive2
Other
565 stars 70 forks source link

Partial compatibility with Google security update 2021 #350

Closed agrenott closed 1 month ago

agrenott commented 2 months ago

Google Drive added a new "resourceKey" attribute required to access documents shared by links. This resourceKey must be passed through HTTP header, aside with the document ID. resourceKey can be retrieved from a previous list operation on containing folder. Partial implementation; only for basic methods: GetContentFile, GetContentIOBuffer and FetchMetadata.

Ref.

Few remarks:

shcheklein commented 1 month ago

thanks @agrenott ! Please check linter / tests failures. Also, I don't think we can rely on an external file in tests. We need to do some research and create via API, if not possible, let me know how I can create a file / share a link manually. Let's create it in some account that is controlled via the same account that we use for tests.

agrenott commented 1 month ago

Hi @shcheklein , I've fixed the linter issue. I'm a bit puzzled by the failing test, as it's specific to python 3.10 and doesn't seem directly related to my change. Let's see if it happens on next run.

Concerning the unit tests, sadly I can't find a way to create a new file impacted by the resourceKey requirement (neither via API nor manually in Drive UI). Googling a bit, I think it has only been used as a work-around on old files (pre-2021?) which were allocated a not-so-random ID. rclone's source code seems to point into that direction as well: https://github.com/rclone/rclone/blob/master/backend/drive/drive.go#L659

So I don't have much better idea than relying on an existing public impacted file if we want to test those relying on the E2E usage of GDrive. Alternative would be to mock GDrive for those tests.

shcheklein commented 1 month ago

thanks @agrenott !

Alternative would be to mock GDrive for those tests.

let's mock it then and put an actual file for a reference and some code to test it if needed

as it's specific to python 3.10 and doesn't seem directly related to my change. Let's see if it happens on next run.

I'll take a look and let you know. Sometimes it happens since we run too many API calls in parallel.

agrenott commented 1 month ago

Mock implemented, and marked the real tests with as "manual", hope it's enough to disable them by default.

agrenott commented 1 month ago

Hi @shcheklein, are you fine merging this PR in its current state or do you expect more changes?

shcheklein commented 1 month ago

@agrenott I haven't had time to do the final review yet. Let me get back to it soon. I'll let you know. AFAIR it should be fine, but let me do the final pass.

shcheklein commented 1 month ago

@agrenott PTAL - just a few minor comments

shcheklein commented 1 month ago

@agrenott merged and released. Thanks 🙏

agrenott commented 1 month ago

You're welcome! It's nice to be able to contribute to some project I rely upon.