Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Custom scope authentication #98

Closed
guybedford opened this issue Sep 28, 2015 · 7 comments
Closed

Custom scope authentication #98

guybedford opened this issue Sep 28, 2015 · 7 comments

Comments

@guybedford
Copy link
Member

While we now support scopes which have their own registry settings, we still only check the auth once without any scope context. We should upgrade the auth check to apply for each lookup given the currently installing scope.

@glen-84
Copy link
Contributor

glen-84 commented Jul 7, 2016

@guybedford,

With the following .npmrc file:

@x:registry=https://registry.cnpm.example.com

Installation with npm works (npm i @x/api-client-js -SE), but installation with jspm fails:

(node:3336) Warning: a promise was rejected with a non-error: [object String]
(node:3336) Warning: a promise was rejected with a non-error: [object String]

err  Error on lookup for npm:@x/api-client-js
Invalid authentication details. Run jspm registry config npm to reconfigure.
(node:3336) Warning: a promise was rejected with a non-error: [object String]
(node:3336) Warning: a promise was rejected with a non-error: [object String]

err  Error looking up npm:@x/api-client-js.

warn Installation changes not saved.

If I remove the scope from the npmrc file (which I don't really want to do), it still fails:

     Updating registry cache...
     Downloading npm:@x/[email protected]
(node:20428) Warning: a promise was rejected with a non-error: [object String]
(node:20428) Warning: a promise was rejected with a non-error: [object String]
(node:20428) Warning: a promise was rejected with a non-error: [object String]
(node:20428) Warning: a promise was rejected with a non-error: [object String]
(node:20428) Warning: a promise was rejected with a non-error: [object String]
(node:20428) Warning: a promise was created in a handler but was not returned from it
ok   Installed peer buffer to github:jspm/nodelibs-buffer@^0.2.0-alpha (0.2.0-alpha)
(node:20428) Warning: a promise was rejected with a non-error: [object String]
(node:20428) Warning: a promise was rejected with a non-error: [object String]
(node:20428) Warning: a promise was rejected with a non-error: [object String]
(node:20428) Warning: a promise was rejected with a non-error: [object String]
(node:20428) Warning: a promise was rejected with a non-error: [object String]
(node:20428) Warning: a promise was rejected with a non-error: [object String]

err  Repo npm:buffer not found!

warn Installation changes not saved.

This means that I'm unable to install this package from a private repo, which is quite a big issue.

Is this difficult to fix? (jspm 0.17.0-beta.22)

@glen-84
Copy link
Contributor

glen-84 commented Jul 14, 2016

I was able to install my package with a temporary hack, but it would really be nice if this was implemented properly.

In npm.js, above this line:

      if (scope) {
        var npmrc = new Npmrc();
        self.auth = npmrc.getAuth(self.registryURL(scope));
      }

If I can help in some way, please let me know.

@guybedford
Copy link
Member Author

@glen-84 a proper implementation for this would be really awesome. If you're keen on working on a PR let me know. The hack sounds about the right direction to me actually, but perhaps we can inline the implementation into getRegistryURL or have getRegistryURL return both auth and url?

@glen-84
Copy link
Contributor

glen-84 commented Jul 15, 2016

I would have to get more familiar with the code, so if it's something that you could do in 5 minutes then it's probably not worth me trying to figure this all out. However, it is really important and something that I need to have working within the next couple of business days (maybe a week at most), so if you simply don't have the time then please let me know now and I'll see if I can make a plan.

@glen-84
Copy link
Contributor

glen-84 commented Jul 17, 2016

@guybedford,

I don't think that it makes sense for registryURL to set or return auth data, but my understanding of the code is limited, so I could be wrong.

I made some improvements to my code, so it now looks like this:

      var authData;
      if (scope) {
        authData = self.npmrc.getAuth(self.registryURL(scope));
      } else {
        authData = self.auth;
      }

      // This is the existing code, except that `self.auth` is replaced with `authData`.
      return asp(request)(auth.injectRequestOptions({
        uri: self.registryURL(scope) + '/' + repoPath,
        gzip: true,
        strictSSL: self.strictSSL,
        headers: lookupCache ? {
          'if-none-match': lookupCache.eTag
        } : {}
      }, doAuth && authData)).then(function(res) {

The npmrc object is made available from the constructor.

Question: If there is no scope-specific auth, should it fall back to self.auth? I'm not really sure how options.authToken and options.auth fit into this.

It would then be something like this:

      var authData;
      if (scope) {
        authData = self.npmrc.getAuth(self.registryURL(scope));
      }

      if (!authData) {
        authData = self.auth;
      }

      // etc.

@guybedford
Copy link
Member Author

@glen-84 it make make sense to have a more general getAuthAndURL style method with an object containing the URL and auth object to pass to the request.

For non-scope-specific, yes we just fallback to global auth.

options.authToken and options.auth can then be thought of as global auth to apply when this isn't detected perhaps?

@glen-84
Copy link
Contributor

glen-84 commented Jul 22, 2016

I just submitted a PR for this.

I couldn't see the benefit of having a method like that. getXAndY() seems like a code smell, but I could be wrong. Maybe something like getRegistryInfo/Details/Config could have worked, but I've kept it simple for now. If you disagree, please LMK exactly what you require.

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

No branches or pull requests

2 participants