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

Jump to bottom

feat: respect registry-scoped certfile and keyfile options #125

Merged
merged 1 commit into from
Jul 18, 2022
Merged

feat: respect registry-scoped certfile and keyfile options #125

merged 1 commit into from
Jul 18, 2022

Conversation

jenseng
Copy link
Contributor

@jenseng jenseng commented Jul 12, 2022

Add support for registry-scoped certfile and keyfile options, e.g.

{
  "//my.registry.example/npm/:certfile": "~/.secret/stuff.crt",
  "//my.registry.example/npm/:keyfile": "~/.secret/stuff.key"
}

Since these are registry-specific, they will override top-level cert and key options (if set).

Like the top-level cafile option, these registry-scoped options are silently ignored if invalid.

References

Related to npm/rfcs#591
Closes #118

@jenseng jenseng requested a review from a team as a code owner July 12, 2022 23:19
jenseng added a commit to jenseng/config that referenced this pull request Jul 12, 2022
@jenseng
feat: detect registry-scoped certfile and keyfile options
eb34e7a 
RFC: npm/rfcs#591

See also: npm/npm-registry-fetch#125

By itself this change doesn't do much, but it enables us to resolve
npm/cli#4765 and surface these options anywhere
else they may be needed.
@jenseng jenseng mentioned this pull request Jul 12, 2022
Merged
jenseng added a commit to jenseng/cli that referenced this pull request Jul 12, 2022
@jenseng
feat: accept registry-scoped certfile and keyfile as credentials
1b0b9ea 
Closes npm#4765
RFC: npm/rfcs#591

While this doesn't directly allow top-level cert/key as credentials (per the
original issue), it's a more targeted/secure approach that accomplishes the
same end-result; the new options are scoped to a specific registry, and the
actual cert/key contents are much less likely to be exposed. See the RFC for
more context.

Depends on:
* npm/npm-registry-fetch#125
* npm/config#69
@jenseng jenseng mentioned this pull request Jul 12, 2022
Merged
@jenseng
Copy link
Contributor Author

jenseng commented Jul 12, 2022

A notable difference from the top-level cafile option is that we resolve these much later (i.e. here in getAuth, rather than way back in config/nopt land).

If y'all prefer I can push things back that way, e.g. getCredentialsByUri could resolve them into scoped cert/key options. Though it may result in a little more complexity, since we don't actually send the results of getCredentialsByUri down into npm-registry-fetch anywhere afaict, we just use it for ENEEDAUTH checks and the like. Though that feels a bit backwards if the goal is to eventually remove ENEEDAUTH checks from the CLI (per #38)

@wraithgar wraithgar self-assigned this Jul 13, 2022
wraithgar pushed a commit to npm/config that referenced this pull request Jul 18, 2022
@jenseng
feat: detect registry-scoped certfile and keyfile options (#69)
e58a4f1 
RFC: npm/rfcs#591

See also: npm/npm-registry-fetch#125

By itself this change doesn't do much, but it enables us to resolve
npm/cli#4765 and surface these options anywhere
else they may be needed.
wraithgar
wraithgar reviewed Jul 18, 2022
View reviewed changes
lib/auth.js Show resolved Hide resolved
@jenseng
feat: respect registry-scoped certfile and keyfile options
f5f0d39 
Closes npm#118
RFC: npm/rfcs#591

Add support for registry-scoped certfile and keyfile options, e.g.

```
{
  "//my.registry.example/npm/:certfile": "~/.secret/stuff.crt",
  "//my.registry.example/npm/:keyfile": "~/.secret/stuff.key"
}
```

Since these are registry-specific, they will override top-level cert and
key options (if set).

Like the top-level `cafile` option, these registry-scoped options are
silently ignored if invalid.
@jenseng jenseng force-pushed the registry-scoped-keyfile-and-certfile branch from 49517d1 to f5f0d39 Compare July 18, 2022 18:24
@wraithgar wraithgar merged commit 42d605c into npm:main Jul 18, 2022
@github-actions github-actions bot mentioned this pull request Jul 18, 2022
Merged
jenseng added a commit to jenseng/cli that referenced this pull request Jul 18, 2022
@jenseng
feat: accept registry-scoped certfile and keyfile as credentials
47ecd22 
Closes npm#4765
RFC: npm/rfcs#591

While this doesn't directly allow top-level cert/key as credentials (per the
original issue), it's a more targeted/secure approach that accomplishes the
same end-result; the new options are scoped to a specific registry, and the
actual cert/key contents are much less likely to be exposed. See the RFC for
more context.

Depends on:
* npm/npm-registry-fetch#125
* npm/config#69
jenseng added a commit to jenseng/cli that referenced this pull request Jul 18, 2022
@jenseng
feat: accept registry-scoped certfile and keyfile as credentials
64ef34f 
Closes npm#4765
RFC: npm/rfcs#591

While this doesn't directly allow top-level cert/key as credentials (per the
original issue), it's a more targeted/secure approach that accomplishes the
same end-result; the new options are scoped to a specific registry, and the
actual cert/key contents are much less likely to be exposed. See the RFC for
more context.

Depends on:
* npm/npm-registry-fetch#125
* npm/config#69
jenseng added a commit to jenseng/cli that referenced this pull request Jul 18, 2022
@jenseng
feat: accept registry-scoped certfile and keyfile as credentials
f110f65 
Closes npm#4765
RFC: npm/rfcs#591

While this doesn't directly allow top-level cert/key as credentials (per the
original issue), it's a more targeted/secure approach that accomplishes the
same end-result; the new options are scoped to a specific registry, and the
actual cert/key contents are much less likely to be exposed. See the RFC for
more context.

Depends on:
* npm/npm-registry-fetch#125
* npm/config#69
jenseng added a commit to jenseng/cli that referenced this pull request Jul 19, 2022
@jenseng
feat: accept registry-scoped certfile and keyfile as credentials
eb1ea9c 
Closes npm#4765
RFC: npm/rfcs#591

While this doesn't directly allow top-level cert/key as credentials (per the
original issue), it's a more targeted/secure approach that accomplishes the
same end-result; the new options are scoped to a specific registry, and the
actual cert/key contents are much less likely to be exposed. See the RFC for
more context.

Depends on:
* npm/npm-registry-fetch#125
* npm/config#69
jenseng added a commit to jenseng/cli that referenced this pull request Jul 19, 2022
@jenseng
feat: accept registry-scoped certfile and keyfile as credentials
9771bf6 
Closes npm#4765
RFC: npm/rfcs#591

While this doesn't directly allow top-level cert/key as credentials (per the
original issue), it's a more targeted/secure approach that accomplishes the
same end-result; the new options are scoped to a specific registry, and the
actual cert/key contents are much less likely to be exposed. See the RFC for
more context.

Depends on:
* npm/npm-registry-fetch#125
* npm/config#69
fritzy pushed a commit to npm/cli that referenced this pull request Jul 20, 2022
@jenseng
feat: accept registry-scoped certfile and keyfile as credentials (#5160)
5ef53ee 
Closes #4765
RFC: npm/rfcs#591

While this doesn't directly allow top-level cert/key as credentials (per the
original issue), it's a more targeted/secure approach that accomplishes the
same end-result; the new options are scoped to a specific registry, and the
actual cert/key contents are much less likely to be exposed. See the RFC for
more context.

Depends on:
* npm/npm-registry-fetch#125
* npm/config#69
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wraithgar wraithgar wraithgar approved these changes

Assignees

@wraithgar wraithgar

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEATURE] registry-scoped certfile and keyfile options
2 participants
@jenseng @wraithgar