Skip to content

Conversation

@jimchamp
Copy link
Collaborator

@jimchamp jimchamp commented Jan 8, 2026

Closes #11628

Updates the username used for authentication in the copydocs script. The username now matches that of the local /people/openlibrary account.

Makes the following changes to the OpenLibrary API class:

  • login methods now authenticate via the local HTML login handler
  • The login request no longer follows redirects

Technical

A recent update to the JSON login handler removed the ability to authenticate with username and password, and using S3 credentials triggers an S3 service call in the audit_accounts function. Until and unless these calls are mocked in local development environments, the HTML login handler must be used to authenticate locally.

Successful authentication requests to the HTML login handler always redirect. These redirected responses were missing the Set-Cookie header, which is needed to make authenticated calls with the OpenLibrary client.

Important

The OpenLibrary client is used by manage-imports.py. Jobs using this script should be closely monitored for authentication errors after this is deployed.

Testing

Run copydocs locally, and expect books to be imported from openlibrary.org.

Screenshot

Stakeholders

Copilot AI review requested due to automatic review settings January 8, 2026 01:41
@github-actions github-actions bot added the Priority: 1 Do this week, receiving emails, time sensitive, . [managed] label Jan 8, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes authentication errors in the copydocs.py script by updating the login mechanism to use the HTML login handler instead of the JSON handler, and updates the username to match the local development account.

Key changes:

  • Modified the OpenLibrary.login() method to authenticate via the HTML login handler with query parameters instead of JSON data
  • Added allow_redirects parameter to the _request() method to prevent following redirects during login
  • Updated the hardcoded username in copydocs.py from "admin" to "[email protected]"

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
openlibrary/api.py Modified login method to use HTML handler with query params and prevent redirect following to capture Set-Cookie header
scripts/copydocs.py Updated hardcoded login username to match the local development account email
Comments suppressed due to low confidence (1)

openlibrary/api.py:101

  • The login credentials should be sent as form data in the request body using the data parameter instead of the params parameter. Using params sends credentials as URL query parameters which is unconventional for POST requests and could expose sensitive data in server logs. HTML forms submit data in the request body, and the data parameter in the requests library is the standard way to send form-encoded data.

        section = section or self.base_url.split('://')[-1]


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jimchamp jimchamp marked this pull request as draft January 8, 2026 02:31
@mekarpeles mekarpeles self-assigned this Jan 8, 2026
@jimchamp
Copy link
Collaborator Author

jimchamp commented Jan 8, 2026

Noting that the OpenLibrary API is used by two other scripts:

manage-imports.py uses OpenLibrary for all import_* commands:

  • import_ocaids
  • import_batch
  • import_item
  • import_all

The add-new-scans and add-items commands are unaffected.

pull-templates.py is no longer in used (as far as I can tell).

@jimchamp jimchamp marked this pull request as ready for review January 8, 2026 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: 1 Do this week, receiving emails, time sensitive, . [managed]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

copydocs erroring with 403 forbiden

2 participants