Skip to content
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

Allows self-signed certs on OPDS feed requests in dev #874

Conversation

iisisrael
Copy link
Contributor

Not sure how kosher / best practice this is, but resolves issue #860 .

@iisisrael iisisrael force-pushed the allow-self-signed-opds-dev-requests branch from ed4d10e to b50d1a2 Compare December 5, 2019 00:08
@iisisrael
Copy link
Contributor Author

Squashed, which removes the original changes.

@danielweck
Copy link
Member

danielweck commented Dec 6, 2019

It has just occurred to me that we should probably use this instead:

const request = require( 'request' ).defaults({rejectUnauthorized: _NODE_ENV === "development"});

https://github.com/request/request#requestdefaultsoptions

However, unfortunately this is not a global default that would apply to all instances of request in a given Javascript bundle / process (e.g. the Electron main process), which would be ideal because there are other uses of request in Readium2 dependencies/components (e.g. LCP LSD register / renew / return interactions, etc.).

So instead this just creates a wrapper to a specific request instance that will use the given defaults. For this to work more globally in Thorium's own codebase, we must expose the instance as a const, somewhere. For this to work with underlying Readium2 components, the APIs would need to accept a custom request instance to be used instead of a regular import * as request from "request" (IIRC).

@danielweck
Copy link
Member

New file: src/utils/request.ts

import * as rawRequest from "request";
export const request = rawRequest.defaults({ rejectUnauthorized: _NODE_ENV === "development" });

...then in src/main/services/downloader.ts and src/common/utils/http.ts replace import * as request from "request"; with import { request } from "readium-desktop/utils/request";

What do you think?

@danielweck
Copy link
Member

I'm not sure how this would work with request-promise-native which is used in some cases instead of request inside the Readium2 LCP/LSD code.
https://github.com/request/request-promise-native

@danielweck
Copy link
Member

Closing this PR, as rejectUnauthorized is now enabled in DEV mode for httpGet() (wrapper for request() which calls Node fetch()), as well as the special case request.post() (OPDS OAuth). Note that the LCP/LSD internal request() calls (request lib, not a Thorium-specific function wrapper) do not enable rejectUnauthorized (see comment above #874 (comment) ).

@danielweck danielweck closed this Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants