Info: Owned groups vs Master access about verdaccio-gitlab HOT 22 CLOSED

GitLab 11.2 will support privide the option to request min_access_level, see https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/20478

from verdaccio-gitlab.

I think we should create global options, one for access with Reporter as default and one for publish with Maintainer as default.

See:

• https://docs.gitlab.com/ce/user/permissions.html#project-members-permissions
• https://docs.gitlab.com/ce/api/access_requests.html

from verdaccio-gitlab.

I agree master would make sense. However, we would need a nice api on gitlab side to get groups where the authenticated person is master in a similar way as owner=true on the group api endpoint. Otherwise, we have to do a lot of queries during the authentication phase and that's definitive not what a like to do.

from verdaccio-gitlab.

How should we approach gitlab version compatibility? We have several options here:

• We release this functionality as a new verdaccio-gitlab major version, requires gitlab 11.2+
• We make the code compatible with e.g. at least gitlab 11.x (or whichever version the owner api functionality was added in) and default to the current owner-only functionality in case we are not in 11.2+. For this we need an endpoint in gitlab to identify the api version, I'll check the documentation.
• ???

from verdaccio-gitlab.

They offer a version API https://docs.gitlab.com/ee/api/version.html but it requires authentication which is against the plugin's goals.

I believe we could leave the default behaviour as it currently is and introduce a new config which switches to this new functionality.

Then later we could release a major version upgrade that makes this the default if we want.

I wouldn't have an issue either way as either way it will require us to change something.

from verdaccio-gitlab.

Ok, I think I'll have a first working implementation by the end of today, I'll work on a feature branch and let's see what @bufferoverflow thinks of this.

from verdaccio-gitlab.

@bufferoverflow
I'm still working with the code in my own branch, but I'm faced with the following issues.

I'm working on a configuration like this:

auth:
  gitlab:
    url: http://gitlab
    authCache:
      enabled: true
      ttl: 300
    # $min_access_level or $owner
    accessControl: $min_access_level

packages:
  '@*/*':
    # scoped packages
    access: $gitlab_reporter
    publish: $gitlab_maintainer
    proxy: npmjs
    gitlab: true

  '**':
    access: $gitlab_reporter
    publish: $gitlab_maintainer
    proxy: npmjs
    gitlab: true

The accessControl: $owner would replicate the previous behaviour; the accessControl: $min_access_level implements the new functionality.

Each package definition would allow to set the access and publish rules independently, with the possible values:
$all | $authenticated | $gitlab_guest | $gitlab_reporter | $gitlab_developer | $gitlab_maintainer | $gitlab_owner

We would need to query the groups for a user twice, once for access and another for publish, since the groups will be different depending on the access or publish operations. The auth cache can be extended to support this.

@juanpicado The main issue is how to pass two different sets of groups between authenticate and allow_access / allow_publish. We only have the credentials of the user in the authenticate call, so it's the only location in which I can query gitlab and then we can set the groups of the user in the callback. These groups are then available later as real_groups in allow_access and allow_publish.

Those two sets of groups will be different on access or publish, I need a way to store the sets and reference them appropriately. I could probably use the in-memory cache, but we made that an optional functionality, so I can't depend on it being always available.

Any ideas?

from verdaccio-gitlab.

@bufferoverflow And now that I think about it a bit more in detail, I think that if we allow to configure the access level on each package definition, we potentially need to query a set of groups for each combination min_access_level - user, since all combinations might be possible. Maybe we prefer to set this min_access_level as a fixed plugin configuration for all packages, and not a per-package configuration. In that case, we only have to query max. each user groups twice.

from verdaccio-gitlab.

@dlouzan we should implement this as simple as possible. One global gitlab configuration option for publish and one for access. This requires two gitlab api calls during login and caching for access and publish groups.

auth:
  gitlab:
    url: http://gitlab
    authCache:
      enabled: true
      ttl: 300
    access: 20
    publish: 40

Other options should stay as is.

from verdaccio-gitlab.

@bufferoverflow Ok, I'm on it
@juanpicado Any idea if it's possible to implement the passing of multiple sets of groups between authenticate and allow_access / allow_publish without relying on the cache? Right now I can only pass a single string array group list.

from verdaccio-gitlab.

@bufferoverflow A functional version is uploaded

For this initial version, I'm only applying the min_access_level configuration to publish operations, access operations are still using the previous logic (authenticated users are always allowed to view packages, unauthenticated only those packages that have a verdaccio rule $all set).

The reason for this is that there's no support at the moment for passing multiple sets of permissions between the plugin methods, the current API only supports a single group set, and we need one for access and another for publish. I'm in contact with @juanpicado to see if we can find a proper solution. There's also ways to do it via the cache, but I think they're quite ugly.

Additionally, today I found two potential flaws in our current gitlab logic:

1. If we enable a rule for access e.g. access: $reporter, there's no way to download then any public package from the upstream registries. Users will never be part of a group for those packages, and verdaccio will reject their downloads. We need to think a bit about this, I think the use case would be to protect some specific groups depending on their gitlab configuration, so maybe we need to, by default allow authenticated users to download, and only reject access when a specific group exists in gitlab but is not public and doesn't match the access level configured. There's also the issue of name collisions (e.g. a public package that has the same name as a group in gitlab). This requires some careful thinking.
2. I also think we might have name collisions between user names and existing public packages or groups. Our logic at the moment is to add a group to each user that matches their own user id. Wouldn't this give rights to e.g. a user named verdaccio to the verdaccio package?

from verdaccio-gitlab.

@dlouzan I have not a clear idea about how to answer you yet, I'm working currently on new token support and some things might change. I'll let you know the outcome of it.

from verdaccio-gitlab.

After googling a bit and doing some diagrams, I think a workable approach would be to define an npm package-name-friendly prefix in the verdaccio-gitlab config and then mandate all custom, non-public registry packages to be prefixed with it. If the prefix is well chosen, the probability of collisions is minimal and it also has the advantage of making very clear which packages belong to the private registry.

Something like:
prefix: my.org.com

And then names like:

• my.org.com_mypackage
• @my.org.com_mygroup/mypackage

I will do some more thinking and check some scenarios in paper.

from verdaccio-gitlab.

@dlouzan We would never be able to upgrade if you forced this level of breaking change.

Why is this an issue? Why would you want to enforce this level of prefixes?

It's not the job of a plugin/verdacio to figure out if the package was internal or private, Its the job to match the configuration and apply the access rules.

Verdacio allows for easily overriding public packages if need be, Those, however, should follow the same authentication policy as defined in the config.

from verdaccio-gitlab.

We never do prefix. Clever people have their scope registered upstream on npm registry already.

Just map the publish level to a configurable value with owner as default would be a first feature that I like to add. The log can provide a error message if someone with older GitLab has configured other then owner.

from verdaccio-gitlab.

@bufferoverflow That functionality is basically what is currently in the branch I'm working on, but the default for publish is $maintainer. Easy to change.

But I need some clarification because I don't understand how the current model can work:

• Verdaccio provides by default basically three access levels: anonymous, authenticated and specific-users. Those are statically mapped in the configuration file.
• There's a mismatch between the user/group/project/package models of gitlab and npm. They need to be mapped somehow, and the current approach is to make an authenticated user part of the following npm groups for publishing:
  • the authenticated user id itself
  • any gitlab groups on which the user has at least auth.gitlab.publish level, by default right now $maintainer
• This approach makes it so, that at least two types of collisions can happen:
  1. Gitlab user ids and npm package names and package scopes
     if my user id is by chance the same as any public package or scope, then I'm allowed to publish that package in verdaccio with a completely different purpose. This potentially impacts any other user of the registry that happens to use the same package
  2. Gitlab groups and npm package names and package scopes
     same as above, but for groups on which a user has auth.gitlab.publish rights

At least in our environment we are speaking about thousands of users and hundreds of groups, dynamically managed in gitlab. I eludes me how this can be effectively used. There's hundreds of thousands of packages in npmjs.com, I don't know how many orgs. Collisions will happen.

@drubin How is this managed in your env?

from verdaccio-gitlab.

I have just uploaded some refactorings and changed the default access level for publish to $owner. The behaviour in case of not providing any new configuration should be exactly the same as in the master version.

Ignore for the moment the auth.gitlab.access configuration: although it's parsed, the query is sent to gitlab and the two sets of groups are saved in the cache, the values are not used since there's no way in the verdaccio plugin api to pass two sets of groups. Only the publish set is considered.

@bufferoverflow @drubin It would be nice if you could try to run it locally on your end for some tests. I still do not see how this solves the issues I outlined in the previous comment, though.

from verdaccio-gitlab.

@dlouzan great, please create an MR with the changes. The amount of packages that people are publishing to their private repo is limited so I think we can live with those limitations for the moment.

from verdaccio-gitlab.

@juanpicado Any idea if it's possible to implement the passing of multiple sets of groups between authenticating and allow_access / allow_publish without relying on the cache? Right now I can only pass a single string array group list.

@dlouzan I'm working in the new JWT implementation. Currently, with the AES encryption, the token is a combination of

  ? auth.aesEncrypt(new Buffer(req.body.name + ':' + req.body.password)).toString('base64')

the user name and the password (which I think is not a good idea). However, with JWT I can add more information to the token payload, which I'm thinking to add the groups, so on each request, we unpack the token I can provide groups and more information.

Does that might help with your issue?

from verdaccio-gitlab.

Thanks @juanpicado for the details, if adding group info, I recommend to provide individual group.access and group.publish lists. Similar to what docker does

https://auth.docker.io/token?service=registry.docker.io&scope=repository:samalba/my-app:pull,push

with pull, push.

from verdaccio-gitlab.

@bufferoverflow I'll keep on mind :) The idea is to keep backward compatibility and add a new setting to enable JWT by demand and since is new stuff we are on time to do whatever we want.

from verdaccio-gitlab.

The verdaccio plugin model uses these two signatures for access / publish:

allow_access(user: RemoteUser, _package: PackageAccess, cb: Callback)
allow_publish(user: RemoteUser, _package: PackageAccess, cb: Callback)

In the authenticate phase, we can pass a single group set in the callback:

return cb(null, userGroups);

And the passed userGroups will be available in the allow_access and allow_publish calls in the user.real_groups parameter.

In an ideal world:

1. in the authentication phase, we transform the userGroups callback parameter from an array to an object indexed by access type, returning several sets of groups, e.g. userGroups.access and userGroups.publish
2. we then get this info in the allow_access and allow_publish parameter user.real_groups. Either by providing as of now a single array that matches the type of the call, or by also transforming this to an object indexed by the access type.

Since we want to maintain plugin compatibility, we need to find a way of doing this without breaking older plugins. One would be I think:

• add an extra, optional parameter to the authenticate callback that accepts another list array
• if no extra parameter is provided, behave as of now and pass the single userGroup in the callback to both allow_access and allow_publish as user.real_groups
• if the extra group is provided, then allow_access gets the first group as parameter user.real_groups, and allow_publish gets the second extra group.
• this assumes there's only access and publish, maybe I'm forgetting any other

I think I'll be opening an issue in verdaccio for discussing this, probably other members / plugin developers would have a say on this, I personally have only the gitlab view.

from verdaccio-gitlab.