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.

Sign up for GitHub

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

feat(subject_alt_names): Allow multiple subject_alt_names #34

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

feat(subject_alt_names): Allow multiple subject_alt_names #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8463211
feat(subject_alt_names): Allow multiple subject_alt_names
Jun 15, 2019
1e7527c
Change alt names
Jun 15, 2019
db54c8a
Add flatMap
Jun 15, 2019
f8c5bae
Fix typescript errors, add flatMap support in typescript
Jun 15, 2019
c06e68f
Switch public api
Jun 15, 2019
a50d7b9
Fix the domain vs domains change
Jun 20, 2019
1c77d6c
Fix typescript stuff
Jun 20, 2019
08038ba
Try this?
Jun 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions openssl-configurations/domain-certificate-signing-requests.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,4 @@ subjectAltName = @subject_alt_names
subjectKeyIdentifier = hash

[ subject_alt_names ]
DNS.1 = *.<%= domain %>
DNS.2 = <%= domain %>
<%= subjectAltNames %>
3 changes: 1 addition & 2 deletions openssl-configurations/domain-certificates.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,4 @@ extendedKeyUsage = serverAuth
subjectAltName = @subject_alt_names

[ subject_alt_names ]
DNS.1 = *.<%= domain %>
DNS.2 = <%= domain %>
<%= subjectAltNames %>
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"homepage": "https://github.com/davewasmer/devcert#readme",
"devDependencies": {
"standard-version": "^4.3.0",
"typescript": "^2.7.0"
"typescript": "^2.9.2"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you find that you needed to update typescript and tslib for your changes to work? If not, I'd like to save dep updates for separate PRs.

Copy link
Author

Choose a reason for hiding this comment

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

@zetlen yes, at the time I needed to update typescript for flatMap to work

},
"dependencies": {
"@types/configstore": "^2.1.1",
Expand All @@ -52,7 +52,7 @@
"rimraf": "^2.6.2",
"sudo-prompt": "^8.2.0",
"tmp": "^0.0.33",
"tslib": "^1.8.1"
"tslib": "^1.10.0"
},
"optionalDependencies": {}
}
5 changes: 3 additions & 2 deletions src/certificates.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ const debug = createDebug('devcert:certificates');
* individual domain certificates are signed by the devcert root CA (which was
* added to the OS/browser trust stores), they are trusted.
*/
export default async function generateDomainCertificate(domain: string): Promise<void> {
export default async function generateDomainCertificate(domains: string | string[]): Promise<void> {
const domain = Array.isArray(domains) ? domains[0] : domains;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is an example of the kind of code you can simplify if you cast a request for a single domain into an array of length 1 earlier in the process.

Copy link
Author

Choose a reason for hiding this comment

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

Good idea!

mkdirp(pathForDomain(domain));

debug(`Generating private key for ${ domain }`);
Expand Down Expand Up @@ -43,4 +44,4 @@ export function generateKey(filename: string): void {
debug(`generateKey: ${ filename }`);
openssl(`genrsa -out "${ filename }" 2048`);
chmod(filename, 400);
}
}
25 changes: 20 additions & 5 deletions src/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,29 +16,44 @@ export const configDir = applicationConfigPath('devcert');
export const configPath: (...pathSegments: string[]) => string = path.join.bind(path, configDir);

export const domainsDir = configPath('domains');
export const pathForDomain: (domain: string, ...pathSegments: string[]) => string = path.join.bind(path, domainsDir)
export const pathForDomain: (domain: string | string[], ...pathSegments: string[]) => string = path.join.bind(path, domainsDir)

export const caVersionFile = configPath('devcert-ca-version');
export const opensslSerialFilePath = configPath('certificate-authority', 'serial');
export const opensslDatabaseFilePath = configPath('certificate-authority', 'index.txt');
export const caSelfSignConfig = path.join(__dirname, '../openssl-configurations/certificate-authority-self-signing.conf');

export function withDomainSigningRequestConfig(domain: string, cb: (filepath: string) => void) {
function generatesubjectAltNames(domains: string[]) {
return domains
.flatMap(domain => [domain, `*.${domain}`])
.map((domain, index) => `DNS.${index + 1} = ${domain}`)
.join("\r\n");
}

export function withDomainSigningRequestConfig(domains: string | string[], cb: (filepath: string) => void) {
const domainList = typeof domains === "string" ? [domains] : domains;
const domain = domainList[0];
const subjectAltNames = generatesubjectAltNames(domainList);

let tmpFile = mktmp();
let source = readFile(path.join(__dirname, '../openssl-configurations/domain-certificate-signing-requests.conf'), 'utf-8');
let template = makeTemplate(source);
let result = template({ domain });
let result = template({ domain, subjectAltNames });
writeFile(tmpFile, eol.auto(result));
cb(tmpFile);
rm(tmpFile);
}

export function withDomainCertificateConfig(domain: string, cb: (filepath: string) => void) {
export function withDomainCertificateConfig(domains: string | string[], cb: (filepath: string) => void) {
const domainList = typeof domains === "string" ? [domains] : domains;
const domain = domainList[0];
const subjectAltNames = generatesubjectAltNames(domainList);

let tmpFile = mktmp();
let source = readFile(path.join(__dirname, '../openssl-configurations/domain-certificates.conf'), 'utf-8');
let template = makeTemplate(source);
let result = template({
domain,
subjectAltNames,
serialFile: opensslSerialFilePath,
databaseFile: opensslDatabaseFilePath,
domainDir: pathForDomain(domain)
Expand Down
19 changes: 13 additions & 6 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ export interface Options {
* are Buffers with the contents of the certificate private key and certificate
* file, respectively
*/
export async function certificateFor(domain: string, options: Options = {}) {
debug(`Certificate requested for ${ domain }. Skipping certutil install: ${ Boolean(options.skipCertutilInstall) }. Skipping hosts file: ${ Boolean(options.skipHostsFile) }`);
export async function certificateFor(domains: string | string[], options: Options = {}) {
const domain = Array.isArray(domains) ? domains[0] : domains;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I appreciate the backwards-compatibility efforts here. I recommend that you simplify the following code by making domains always an array and then using domains only for the rest of the code. Then you can remove the conditionals for when it's one domain.

-export async function certificateFor(domains: string | string[], options: Options = {}) {
-  const domain = Array.isArray(domains) ? domains[0] : domains;
+export async function certificateFor(requestedDomains: string | string[], options: Options = {}) {
+  const domains = Array.isArray(requestedDomains) ? requestedDomains : [requestedDomains]

If you do that, you can rename all the other private identifiers to plurals so the code is easier to read, so pathForDomain becomes pathForDomains et cetera. This makes the types easier to follow!!

Copy link
Author

@jamsea jamsea Mar 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Cool @zetlen , I thought that too I just wanted to save the backward compatibility. If we're not bothered about backward compatibility (it's really not that big of a deal if we do a major version bump) I'll change this

debug(`Certificate requested for ${ domains }. Skipping certutil install: ${ Boolean(options.skipCertutilInstall) }. Skipping hosts file: ${ Boolean(options.skipHostsFile) }`);

if (options.ui) {
Object.assign(UI, options.ui);
Expand All @@ -59,12 +60,18 @@ export async function certificateFor(domain: string, options: Options = {}) {
}

if (!exists(pathForDomain(domain, `certificate.crt`))) {
debug(`Can't find certificate file for ${ domain }, so it must be the first request for ${ domain }. Generating and caching ...`);
await generateDomainCertificate(domain);
debug(`Can't find certificate file for ${ domains }, so it must be the first request for ${ domains }. Generating and caching ...`);
await generateDomainCertificate(domains);
}

if (!options.skipHostsFile) {
await currentPlatform.addDomainToHostFileIfMissing(domain);
if (Array.isArray(domains)) {
domains.forEach(async (domain) => {
await currentPlatform.addDomainToHostFileIfMissing(domain);
})
} else {
await currentPlatform.addDomainToHostFileIfMissing(domain);
}
}

debug(`Returning domain certificate`);
Expand All @@ -84,4 +91,4 @@ export function configuredDomains() {

export function removeDomain(domain: string) {
return rimraf.sync(pathForDomain(domain));
}
}
3 changes: 3 additions & 0 deletions tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
"compileOnSave": false,
"compilerOptions": {
"declaration": true,
"lib": [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same question as the dependency updates: is it possible to break this into another PR?

"esnext"
],
Copy link
Contributor

Choose a reason for hiding this comment

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

While running the PR branch, it seems as though this pull request requires upgrades to Node environment, with the usage of functions like flatMap requiring Node 11+ unless this line is removed from tsconfig

Copy link
Contributor

Choose a reason for hiding this comment

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

@camsjams if it were me, I'd forego the use of flatMap() altogether. A single reduce() (as opposed to flatMap().map()) would solve the problem, I think, and would be better supported.

Copy link
Author

Choose a reason for hiding this comment

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

Yup, I upgraded it because of flatMap at the time.

"module": "commonjs",
"moduleResolution": "node",
"target": "ES2016",
Expand Down
14 changes: 8 additions & 6 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1230,17 +1230,19 @@ trim-off-newlines@^1.0.0:
version "1.0.1"
resolved "https://registry.yarnpkg.com/trim-off-newlines/-/trim-off-newlines-1.0.1.tgz#9f9ba9d9efa8764c387698bcbfeb2c848f11adb3"

tslib@^1.8.1:
version "1.8.1"
resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.8.1.tgz#6946af2d1d651a7b1863b531d6e5afa41aa44eac"
tslib@^1.10.0:
version "1.10.0"
resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.10.0.tgz#c3c19f95973fb0a62973fb09d90d961ee43e5c8a"
integrity sha512-qOebF53frne81cf0S9B41ByenJ3/IuH8yJKngAX35CmiZySA0khhkovshKK+jGCaMnVomla7gVlIcc3EvKPbTQ==

typedarray@^0.0.6:
version "0.0.6"
resolved "https://registry.yarnpkg.com/typedarray/-/typedarray-0.0.6.tgz#867ac74e3864187b1d3d47d996a78ec5c8830777"

typescript@^2.7.0:
version "2.7.2"
resolved "https://registry.yarnpkg.com/typescript/-/typescript-2.7.2.tgz#2d615a1ef4aee4f574425cdff7026edf81919836"
typescript@^2.9.2:
version "2.9.2"
resolved "https://registry.yarnpkg.com/typescript/-/typescript-2.9.2.tgz#1cbf61d05d6b96269244eb6a3bce4bd914e0f00c"
integrity sha512-Gr4p6nFNaoufRIY4NMdpQRNmgxVIGMs4Fcu/ujdYk3nAZqk7supzBE9idmvfZIlH/Cuj//dvi+019qEue9lV0w==

uglify-js@^2.6:
version "2.8.18"
Expand Down