Skip to content

fix: Incorrect authorization prefix for basic auth, and undocumented env var - #454

Merged
aduh95 merged 5 commits into
nodejs:mainfrom
plumpNation:incorrect-authorization-type
Apr 12, 2024
Merged

fix: Incorrect authorization prefix for basic auth, and undocumented env var#454
aduh95 merged 5 commits into
nodejs:mainfrom
plumpNation:incorrect-authorization-type

Conversation

@plumpNation

@plumpNationplumpNation commented Apr 11, 2024

Copy link
Copy Markdown
Contributor

Changes auth type header strings to use Basic prefix for base64 encoded username:password.

In addition, it seems like an undocumented env var COREPACK_NPM_USER is being used in tests, meaning that this value will be undefined and the user will always get a 401 as the documented env var is COREPACK_NPM_USERNAME.
This bit me when I was using a test to help debug why my COREPACK_NPM_REGISTRY was being ignored when running yarn --version in real life.

@plumpNation
plumpNationforce-pushed the incorrect-authorization-type branch from 780acec to 11419a2CompareApril 11, 2024 15:30
@plumpNation
plumpNation marked this pull request as ready for review April 11, 2024 15:39
@plumpNationplumpNation changed the title Incorrect authorization headers for basic auth with username and passwordfix: Incorrect authorization prefix for basic auth, and undocumented env varApr 11, 2024
@plumpNation
plumpNationforce-pushed the incorrect-authorization-type branch from 0cabeae to 78ae0edCompareApril 12, 2024 10:02
Comment threadsources/httpUtils.ts Outdated
@plumpNation
plumpNationforce-pushed the incorrect-authorization-type branch from 78ae0ed to a6cfd0fCompareApril 12, 2024 10:28
The location of the registry should be validated against the input.origin. With the current code, it simply checks to see if DEFAULT_NPM_REGISTRY_URL exists.
I made the registries into an array to make a regression less likely.
Comment threadsources/httpUtils.ts Outdated
@aduh95
aduh95 merged commit 2d63536 into nodejs:mainApr 12, 2024
@github-actionsgithub-actionsBot mentioned this pull request Apr 12, 2024
Sign up for freeto 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.

2 participants

@plumpNation@aduh95