Skip to content

Set of fixes for better npmrc parameters support #12284

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

Merged
merged 3 commits into from
Sep 26, 2018
Merged

Set of fixes for better npmrc parameters support #12284

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9e1f32a
feat(@schematics/update): add per-project .npmrc file support
smnbbrv Sep 15, 2018
7246555
fix(@schematics/update): process always-auth and _auth correctly, bet…
smnbbrv Sep 15, 2018
85ee603
fix(@schematics/update): taking whitespaces around "=" in .npmrc into…
smnbbrv Sep 19, 2018
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
49 changes: 39 additions & 10 deletions packages/schematics/update/update/npm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,21 @@ function _readNpmRc(): Observable<{ [key: string]: string }> {
// TODO: have a way to read options without using fs directly.
const path = require('path');
const fs = require('fs');
const perProjectNpmrc = path.resolve('.npmrc');

let npmrc = '';
if (process.platform === 'win32') {
if (process.env.LOCALAPPDATA) {
npmrc = fs.readFileSync(path.join(process.env.LOCALAPPDATA, '.npmrc')).toString('utf-8');
}

if (fs.existsSync(perProjectNpmrc)) {
npmrc = fs.readFileSync(perProjectNpmrc).toString('utf-8');
} else {
Copy link

@aguacongas aguacongas Sep 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

You should parse both .npmrc files because the repository url can be in the per project file and credentials in the user file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True.

However if we are too picky here we enter the pain zone. First, then we need to support the global npmrc + userconfig flag / env variable as well. Second, we need to properly resolve the collisions between _authToken and _auth (because both could be present and we need to pick). Finally, the current approach is reading npmrc every time the package info is getting resolved which is not a standard npm behavior. Etc.

I'm not yet talking about having two different ways of getting NPM parameters.

The whole approach is not perfect and this PR does not pretend to be a fully working solution; at least it would be way more helpful than the current approach. Probably one needs to deeply rethink the concept.

Copy link

@aguacongas aguacongas Sep 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I agree. However, IMHO, user file should be read first then because people having open source projects will propably share the project .npmrc without credentials

Copy link
Contributor Author

@smnbbrv smnbbrv Sep 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

The private repos are nearly never used in the open source, hence no auth is required. Also if you change the order of resolving those files then you go in clear conflict with Npm itself

Copy link

Choose a reason for hiding this comment

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

#10624 is regarding an issue with fontawesome pro, so I guess it's used in open source. and npm don't need authentication nor .npmrc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://fontawesome.com/pro - 60$ per license for 5 seats. What open source projects are you talking about?

Copy link

Choose a reason for hiding this comment

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

angular-fontawesome for sample
Open source does not means free, anybody can share code on github but use a private repo with tokens they obvioulsly don't want to share.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I have more arguments, but the discussion went away from the issue. What we discuss is only the cases when there is a project-specific .npmrc which is not containing _auth or _authToken, but there is one in some of the .npmrc above right?

Copy link

Choose a reason for hiding this comment

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

Exactly, in that case, your PR doesn't fix the issue I guess

if (process.env.HOME) {
npmrc = fs.readFileSync(path.join(process.env.HOME, '.npmrc')).toString('utf-8');
if (process.platform === 'win32') {
if (process.env.LOCALAPPDATA) {
npmrc = fs.readFileSync(path.join(process.env.LOCALAPPDATA, '.npmrc')).toString('utf-8');
}
} else {
if (process.env.HOME) {
npmrc = fs.readFileSync(path.join(process.env.HOME, '.npmrc')).toString('utf-8');
}
}
}

Expand All @@ -50,7 +56,7 @@ function _readNpmRc(): Observable<{ [key: string]: string }> {

allOptionsArr.forEach(x => {
const [key, ...value] = x.split('=');
allOptions[key] = value.join('=');
allOptions[key.trim()] = value.join('=').trim();
});

subject.next(allOptions);
Expand Down Expand Up @@ -192,7 +198,8 @@ export function getNpmPackageJson(
getNpmConfigOption('_authToken', registryKey),
getNpmConfigOption('username', registryKey, true),
getNpmConfigOption('password', registryKey, true),
getNpmConfigOption('alwaysAuth', registryKey, true),
getNpmConfigOption('email', registryKey, true),
getNpmConfigOption('always-auth', registryKey, true),
).pipe(
toArray(),
concatMap(options => {
Expand All @@ -205,6 +212,7 @@ export function getNpmPackageJson(
authToken,
username,
password,
email,
alwaysAuth,
] = options;

Expand All @@ -216,17 +224,38 @@ export function getNpmPackageJson(
token?: string,
alwaysAuth?: boolean;
username?: string;
password?: string
password?: string;
email?: string;
} = {};

if (alwaysAuth !== undefined) {
auth.alwaysAuth = alwaysAuth === 'false' ? false : !!alwaysAuth;
}

if (email) {
auth.email = email;
}

if (authToken) {
auth.token = authToken;
} else if (token) {
auth.token = token;
try {
// attempt to parse "username:password" from base64 token
// to enable Artifactory / Nexus-like repositories support
const delimiter = ':';
const parsedToken = Buffer.from(token, 'base64').toString('ascii');
const [extractedUsername, ...passwordArr] = parsedToken.split(delimiter);
const extractedPassword = passwordArr.join(delimiter);

if (extractedUsername && extractedPassword) {
auth.username = extractedUsername;
auth.password = extractedPassword;
} else {
throw new Error('Unable to extract username and password from _auth token');
}
} catch (ex) {
auth.token = token;
}
} else if (username) {
auth.username = username;
auth.password = password;
Expand Down