Skip to content

NPM project level API

David Fernandez requested to merge 220985-add-npm-project-level-manager-api into master

⚠ This MR has a lot of changes but don't be afraid the bulk of it is moving code around ⚠

🌲 Context

The NPM Packages registry accepts a few npm commands, these commands can be used at two different levels: project or instance.

Currently, we have this:

NPM command Available on endpoint
npm view instance
npm add instance
npm dist-tags X instance
npm publish project

This brings an issue for users that have to configure two endpoints depending of what they plan to do with the NPM Packages registry.

  • When they want to pull packages from the registry, they need to use the instance level endpoint
  • When they want to pull and push packages from/to the registry, they need to use both.

This situation can confuse users as described in #220985 (closed).

The fix the problem, we're going to make all the endpoints available to the instance and project level. The exceptions to this are:

  • The upload endpoint must be at the project level. It can't be at the instance level otherwise the backend would receive an upload request without any group or project = it doesn't know where to store the package.
  • The tarball endpoint can remain at the project level. This endpoint url is referenced by the metadata endpoint which in turn is used by the npm install command. In short, during npm install, npm will enquire about the tarball url and the registry only need to reply: "it's here". "here" can be anywhere. To keep things simple, we will keep it where it is now: at the project level.

We're going to centralize all shared endpoints in a Concern that will be included by both Grape APIs.

🛃 Permissions / Authentication error messages

On the instance level, a project is deducted by using the naming convention. This means that on the instance level, we receive a package name and we extract a Project object out of it. In other words, if we can get a Project, we guarantee that a package with such name exists.

When porting those endpoints to the project level, things are different: a project id is given. So we can have the situation where a Project is found but the package (looking for the given package name) doesn't exist. That's a new situation that we will reply with 404 Not Found.

Given that at the project level, we receive a project_id we will use the standard user_project helper. This helper will use #find_project! that will return 404 Not Found for the specific case where the Project exists but the current_user doesn't have access to it. At the instance level, this case is replied with 403 Forbidden. This introduces a discrepancy on the error response if we're at the instance or project level.

Consistency matters here, so we will mimic the find_project! behavior: return 404 Not Found if current_user can't access the Project (whether the Project exists or not).

🔬 What does this MR do?

  • Create a new concern with all shared endpoints (::Packages::NpmEndpoints)
    • Unfortunately, one of the shared endpoints has a globbing parameter, which means that this endpoint must be the last one otherwise it will match urls not meant to be handled by it. So the include statement should be at the bottom of the API class file.
  • Create a new API class for the instance level and include the above concern
  • Create a new API class for the project level and include the above concern
  • Create shared examples that will one shared example for each shared endpoint. The expectations are always the same for both levels (they are all ported from the existing specs). The only thing that changes is the url accessed: it should correspond to the right level.
  • Create specs for the instance level API class and use the shared examples to test each shared endpoint
  • Create specs for the project level API class and use the shared examples to test each shared endpoint
  • Update the documentation accordingly.
    • Users don't need to juggle with two endpoints anymore.

🖼 Screenshots (strongly suggested)

Using the instance-level endpoints

$ cat .npmrc 
@gitlab-org:registry=http://gdk.test:8000/api/v4/packages/npm/
//gdk.test:8000/api/v4/packages/npm/:_authToken=XXXXX

$ npm view @gitlab-org/bananas

@gitlab-org/bananas@1.3.7 | Proprietary | deps: none | versions: 1

dist
.tarball: http://gdk.test:8000/api/v4/projects/1/packages/npm/@gitlab-org/bananas/-/@gitlab-org/bananas-1.3.7.tgz
.shasum: 715e0dcd193cef7cb9cb35105896f69cd4bcf1ac

dist-tags:
latest: 1.3.7 

$ npm dist-tag add @gitlab-org/bananas@1.3.7 foobar
+foobar: @gitlab-org/bananas@1.3.7

$ npm dist-tag ls @gitlab-org/bananas
foobar: 1.3.7
latest: 1.3.7

$ npm dist-tag rm @gitlab-org/bananas foobar
-foobar: @gitlab-org/bananas@1.3.7

$ npm dist-tag ls @gitlab-org/bananas       
latest: 1.3.7

$ npm add @gitlab-org/hello123
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN sandbox@1.0.0 No description
npm WARN sandbox@1.0.0 No repository field.

+ @gitlab-org/hello123@1.3.8
added 1 package from 1 contributor and audited 1 package in 1.02s
found 0 vulnerabilities

$ npm version patch
v1.0.1

$ npm publish
npm notice 
npm notice 📦  sandbox@1.0.1
npm notice === Tarball Contents === 
npm notice 265B package.json
npm notice === Tarball Details === 
npm notice name:          sandbox                                 
npm notice version:       1.0.1                                   
npm notice package size:  276 B                                   
npm notice unpacked size: 265 B                                   
npm notice shasum:        b0652324b64f2eafa3d39c7b5948ffcbc35562df
npm notice integrity:     sha512-Ew03obOEDbF/M[...]0vZ/qtZKqODQg==
npm notice total files:   1                                       
npm notice 
npm ERR! code E403
npm ERR! 403 403 Forbidden - PUT https://registry.npmjs.org/sandbox - You do not have permission to publish "sandbox". Are you logged in as the correct user?
npm ERR! 403 In most cases, you or one of your dependencies are requesting
npm ERR! 403 a package version that is forbidden by your security policy.

Notice that the npm publish command doesn't work. That's expected, we can't push to the packages registry using the instance-level endpoints.

USing the project-level endpoints

$ cat .npmrc 
@gitlab-org:registry=http://gdk.test:8000/api/v4/projects/1/packages/npm/
//gdk.test:8000/api/v4/projects/1/packages/npm/:_authToken=XXXXX

$ npm view @gitlab-org/hello123

@gitlab-org/hello123@1.3.8 | Proprietary | deps: none | versions: 2

dist
.tarball: http://gdk.test:8000/api/v4/projects/1/packages/npm/@gitlab-org/hello123/-/@gitlab-org/hello123-1.3.8.tgz
.shasum: 3ba3567ba52ac06a004686125c8c864deb37778e

dist-tags:
latest: 1.3.8  

$ npm dist-tags ls @gitlab-org/hello123
latest: 1.3.8

$ npm dist-tags add @gitlab-org/hello123@1.3.8 bananas
+bananas: @gitlab-org/hello123@1.3.8

$ npm dist-tags ls @gitlab-org/hello123               
bananas: 1.3.8
latest: 1.3.8

$ npm dist-tags rm @gitlab-org/hello123 bananas
-bananas: @gitlab-org/hello123@1.3.8

$ npm dist-tags ls @gitlab-org/hello123        
latest: 1.3.8

$ npm add @gitlab-org/bananas
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN @gitlab-org/hello123@1.3.8 No repository field.

+ @gitlab-org/bananas@1.3.7
added 1 package from 1 contributor and audited 1 package in 1.045s
found 0 vulnerabilities

$ npm version patch
v1.3.9

$ npm publish
npm notice 
npm notice 📦  @gitlab-org/hello123@1.3.9
npm notice === Tarball Contents === 
npm notice 351B package.json
npm notice === Tarball Details === 
npm notice name:          @gitlab-org/hello123                    
npm notice version:       1.3.9                                   
npm notice package size:  328 B                                   
npm notice unpacked size: 351 B                                   
npm notice shasum:        3369906f8dc280c9ba98de4646b159b855299d75
npm notice integrity:     sha512-9YihOL44huicF[...]8YfsqBCniz55A==
npm notice total files:   1                                       
npm notice 
+ @gitlab-org/hello123@1.3.9

🔮 Follow ups

  • The specs try to go trough combinations of the user role (anonymous, developer, guest) and the project visibility (public, private, internal) to assert that the access is properly restricted. This is done using dedicated context each time which makes the whole test suite hard to read. A possible follow-up would be to use a table based specs.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by David Fernandez

Merge request reports