Skip to content

Support legacy authentication #10

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

Merged
merged 1 commit into from
Nov 30, 2017
Merged

Support legacy authentication #10

merged 1 commit into from
Nov 30, 2017

Conversation

pvdlg
Copy link
Member

@pvdlg pvdlg commented Nov 30, 2017

We are supporting legacy npm authentication as we use npm-registry-couchapp for integration tests which doesn't support token authentication.
But it wasn't intended to be used in production.

It seems there is several private npm registry solution that supports only legacy authentication.
See semantic-release/semantic-release#500 and #6.

This PR improve our support of the legacy auth and mention it in the documentation.

Fix semantic-release/semantic-release#500

@pvdlg pvdlg requested a review from gr2m November 30, 2017 06:44
README.md Outdated

The registry and dist-tag can be configured in the `package.json` and will take precedence on the configuration in `.npmrc`:
Both the [token](https://docs.npmjs.com/getting-started/working_with_tokens) and the legacy (`username`, `password` and `email`) authentication are supported. It is recommended to use the [token](https://docs.npmjs.com/getting-started/working_with_tokens) authentication. The legacy authentication is supported as some npm private registry do not suport the token authentication.
Copy link
Member

Choose a reason for hiding this comment

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

maybe instead of

some npm private registry

it would be better to list the one we know? I think it’s only Artifactory right now? Maybe sth like this?

The legacy authentication is supported as the alternative npm registry Artifactory only supports that form of authentication at this point.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

how about tthis

The legacy authentication is supported as the alternative npm registries Artifactory and npm-registry-couchapp only supports that form of authentication at this point.

Copy link
Member Author

Choose a reason for hiding this comment

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

I made the change

Copy link
Member

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

CI failed, but PR looks good 👍

@pvdlg
Copy link
Member Author

pvdlg commented Nov 30, 2017

I keep getting a lot of Couldn't start npm-docker-couchdb after 2 min.
It happened to me 3 or 4 times. Each time on Node 9. While the Node 8 build was passing...

That's going to be a tricky one to figure out...

@gr2m
Copy link
Member

gr2m commented Nov 30, 2017

yeah debugging CI is the worst :(

@pvdlg
Copy link
Member Author

pvdlg commented Nov 30, 2017

I check the availability of the the registry here.
Basically I wait for /registry/_design/app to return a 200...So not sure what could go wrong...

@pvdlg pvdlg merged commit 5fb0b09 into master Nov 30, 2017
@pvdlg pvdlg deleted the legacy-auth branch November 30, 2017 17:01
@SimenB
Copy link

SimenB commented Nov 30, 2017

I'm trying to make this work, perfect timing :D

Have you tested this against artifactory? I can't seem to make it work properly, but I don't get why it's failing. Tried semantic-release --debug but that didn't help much. AFAICT I'm on the latest of everything as of 10 minutes ago (including 2.2.0 of this module).

image

Variables part:

Setting environment variables from repository settings
$ export NPM_TOKEN=[secure]
$ export ARTIFACTORY_CONTEXT=https://artifacts.schibsted.io/artifactory
$ export ARTIFACTORY_NPM_SECRET=[secure]
$ export [secure]=[secure]
$ export ARTIFACTORY_USER=[secure]
$ export AWS_ACCOUNT_NAME=infra-pro
$ export SPT_ENGPROD_REPORT_KEY=[secure]
$ export SPT_ENGPROD_REPORT_SECRET=[secure]

Setting environment variables from .travis.yml
$ export GITHUB_URL=https://github.schibsted.io
$ export GITHUB_PREFIX=https://github.schibsted.io/api/v3
$ export NPM_EMAIL=$ARTIFACTORY_USER
$ export NPM_OLD_TOKEN=$ARTIFACTORY_NPM_SECRET
$ export GH_TOKEN=[secure]

Any ideas about what I should try to debug this?

For all I know the token really isn't valid, I haven't used it before.

But it would be great if --debug added some information that it actually tried to use legacy instead of new one, etc.

(Artifactory version 4.16.0)

EDIT: I just noticed a NPM_TOKEN from the repo settings when I pasted it here... Lemme try to remove my own token stuff

EDIT2: Didn't help...

@pvdlg
Copy link
Member Author

pvdlg commented Nov 30, 2017

The logs mention Wrote NPM_TOKEN to .npmrc which mean you have an environment variable called NPM_TOKEN and therefore the plugin uses the token authentication.

From the README:

Use either NPM_TOKEN for token authentication or NPM_USERNAME, NPM_PASSWORD and NPM_EMAIL for legacy authentication

@SimenB
Copy link

SimenB commented Nov 30, 2017

Yeah, I just noticed that... But that is set globally on Travis for us (tried removing my own ""), so I'd be surprised if it was wrong. I can ask the guys who put it there, but is there anything I can do myself to debug more?

Or a way I can force the legacy version?

@SimenB
Copy link

SimenB commented Nov 30, 2017

npm/lib/verify.js

Lines 9 to 13 in 5fb0b09

try {
await execa('npm', ['whoami', '--registry', registry]);
} catch (err) {
throw new SemanticReleaseError('Invalid npm token.', 'EINVALIDNPMTOKEN');
}

For all I know I get a 404 or something, some more information than "no matter what fails with whoami it's auth failure" would be awesome in this case.

Or maybe I'm missing some other variable so it doesn't use the correct registry

@pvdlg
Copy link
Member Author

pvdlg commented Nov 30, 2017

It seems you want to configure the legacy auth. But you have an NPM_TOKEN which tells the plugin to use the token auth.

If you want to use the legacy auth configure it according to the doc with NPM_USERNAME , NPM_PASSWORD and NPM_EMAIL, without NPM_TOKEN.

@SimenB
Copy link

SimenB commented Nov 30, 2017

A - export NPM_TOKEN="" seems to have done it! And not calling it NPM_OLD_TOKEN, but NPM_PASSWORD. I'm now getting an error from calling github, but that's not related to this repo.

Thanks for the help!

@pvdlg
Copy link
Member Author

pvdlg commented Nov 30, 2017

Probably because of your GITHUB_PREFIX configuration. It should be a prefix and not the full URL.

@SimenB
Copy link

SimenB commented Nov 30, 2017

Yup, I noticed, thanks! Opened up semantic-release/github#10 for it.

Now I'm stuck at semantic-release/semantic-release#540, but I feel like I'm getting closer at least.

Thanks for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants