search

n4bb12 / verdaccio-github-oauth-ui

📦🔐 GitHub OAuth plugin for Verdaccio
https://verdaccio.org
MIT License
74 stars 45 forks source link

WIP Users that don't belong to the specified organization shouldn't see the list of packages. #32

Closed fatmirsukaliq closed 5 years ago

fatmirsukaliq commented 5 years ago

Hi Abraham,

Great work on the library! I believe I found a bug and also a quick fix for it.

Explanation of the bug Our Verdaccio config has access & publish properties set to $authenticated. When the user is not logged in and they visit our registry, the registry won't show any packages and that is the way I expected it to be.

But when a user that doesn't belong to our organization logs in, they can see the list of the packages, but on the server log I can see that it logs Access denied: user username is not a member of organization..

Therefore, I found out that when we don't return null but we return an error message, this problem goes away.

For the record I'm using Verdaccio 3.13.1 and the latest version of your library.

n4bb12 commented 5 years ago

Hi @fatmirsukaliq, thanks for reporting this.

According to the docs https://verdaccio.org/docs/en/3.8.6/dev-plugins#callback, the right way to fail the authentication is to call:

callback(null, false)

To my understanding, the callback is only supposed to be called with an error message, when an error occurs that cannot be recovered from.

Since verdaccio can load mutliple authentication plugins at once, when one plugin denies authentication, another plugin might still grant authentication. For instance one could not have access via GitHub, but still have access via password.

Calling the callback with an error message breaks the plugin loop, while calling it with (null, false) denies authentication on the current plugin and verdaccio then hands over to the next plugin.

Please see this related bug report https://github.com/n4bb12/verdaccio-github-oauth-ui/issues/17.

So my guess is that you might be loading an additional authentication plugin which is granting access.

n4bb12 commented 5 years ago

Could you please share the full console output?

fatmirsukaliq commented 5 years ago

Successfully built c05d66495d40 Successfully tagged mys3verdaccio_verdaccio:latest Recreating verdaccio-s3-plugin ... done Recreating mys3verdaccio_aws-resources_1 ... done Attaching to mys3verdaccio_aws-resources_1, verdaccio-s3-plugin aws-resources_1 | make_bucket: verdaccios3 mys3verdaccio_aws-resources_1 exited with code 0 verdaccio-s3-plugin | warn --- config file - /verdaccio/conf/config.yaml verdaccio-s3-plugin | debug-=- s3: configuration: { verdaccio-s3-plugin | "bucket": "verdaccios3", verdaccio-s3-plugin | "region": "eu-east-1", verdaccio-s3-plugin | "endpoint": "", verdaccio-s3-plugin | "accessKeyId": "", verdaccio-s3-plugin | "secretAccessKey": "", verdaccio-s3-plugin | "keyPrefix": "docker-test-prefix/" verdaccio-s3-plugin | } verdaccio-s3-plugin | warn --- Plugin successfully loaded: aws-s3-storage verdaccio-s3-plugin | debug-=- s3: [_getData] bucket: verdaccios3 prefix: docker-test-prefix/ verdaccio-s3-plugin | trace-=- s3: [_getData] get database object verdaccio-s3-plugin | trace-=- s3: [_getData] get data {"list":["@/image-uploader","rollup-starter-app"],"secret":"0e2cc59bfbd112419fc367e86f0106bca03ff769c3913f51e1b0fe890023a8e8"} verdaccio-s3-plugin | trace-=- s3: [_getData] already exist verdaccio-s3-plugin | debug-=- s3: [_sync] bucket: verdaccios3 prefix: docker-test-prefix/ verdaccio-s3-plugin | debug-=- s3: [_sync] sucess verdaccio-s3-plugin | [github-oauth-ui] Proxy config: null verdaccio-s3-plugin | warn --- Plugin successfully loaded: github-oauth-ui verdaccio-s3-plugin | [github-oauth-ui] Proxy config: null verdaccio-s3-plugin | warn --- Plugin successfully loaded: github-oauth-ui verdaccio-s3-plugin | debug-=- s3: [get] verdaccio-s3-plugin | trace-=- s3: [_getData] already exist verdaccio-s3-plugin | debug-=- s3: [get] verdaccio-s3-plugin | trace-=- s3: [_getData] already exist verdaccio-s3-plugin | debug-=- s3: [getPackageStorage] @/image-uploader verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] packageName @/image-uploader verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] endpoint https://s3.us-east-1.amazonaws.com verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] region eu-east-1 verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] s3ForcePathStyle undefined verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] accessKeyId verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] secretAccessKey verdaccio-s3-plugin | debug-=- s3: [S3PackageManager readPackage init] name @/image-uploader/@/image-uploader verdaccio-s3-plugin | debug-=- s3: [S3PackageManager _getData init] verdaccio-s3-plugin | debug-=- s3: [getPackageStorage] @/image-uploader verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] packageName @/image-uploader verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] endpoint https://s3.us-east-1.amazonaws.com verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] region eu-east-1 verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] s3ForcePathStyle undefined verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] accessKeyId verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] secretAccessKey verdaccio-s3-plugin | debug-=- s3: [S3PackageManager readPackage init] name @/image-uploader/@/image-uploader verdaccio-s3-plugin | debug-=- s3: [S3PackageManager _getData init] verdaccio-s3-plugin | warn --- http address - http://0.0.0.0:4873/ - verdaccio/3.13.1 verdaccio-s3-plugin | trace-=- s3: [S3PackageManager _getData body] @/image-uploader verdaccio-s3-plugin | trace-=- s3: [S3PackageManager readPackage] packageName: @/image-uploader / data @data verdaccio-s3-plugin | debug-=- s3: [getPackageStorage] rollup-starter-app verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] packageName rollup-starter-app verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] endpoint https://s3.us-east-1.amazonaws.com verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] region eu-east-1 verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] s3ForcePathStyle undefined verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] accessKeyId verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] secretAccessKey verdaccio-s3-plugin | debug-=- s3: [S3PackageManager readPackage init] name rollup-starter-app/rollup-starter-app verdaccio-s3-plugin | debug-=- s3: [S3PackageManager _getData init] verdaccio-s3-plugin | trace-=- s3: [S3PackageManager _getData body] @/image-uploader verdaccio-s3-plugin | trace-=- s3: [S3PackageManager readPackage] packageName: @/image-uploader / data @data verdaccio-s3-plugin | debug-=- s3: [getPackageStorage] rollup-starter-app verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] packageName rollup-starter-app verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] endpoint https://s3.us-east-1.amazonaws.com verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] region eu-east-1 verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] s3ForcePathStyle undefined verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] accessKeyId verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] secretAccessKey verdaccio-s3-plugin | debug-=- s3: [S3PackageManager readPackage init] name rollup-starter-app/rollup-starter-app verdaccio-s3-plugin | debug-=- s3: [S3PackageManager _getData init] verdaccio-s3-plugin | trace-=- s3: [S3PackageManager _getData body] rollup-starter-app verdaccio-s3-plugin | trace-=- s3: [S3PackageManager readPackage] packageName: rollup-starter-app / data @data verdaccio-s3-plugin | trace-=- s3: [S3PackageManager _getData body] rollup-starter-app verdaccio-s3-plugin | trace-=- s3: [S3PackageManager readPackage] packageName: rollup-starter-app / data @data verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/vendors.fd8981c8de5ef6fbf3ea.js' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/vendors.fd8981c8de5ef6fbf3ea.js', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/github-oauth-ui/login-button.js' verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/0.style.19c5f80b8dd9cfde5c93.css' verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /', bytes: 0/0 verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/0.style.19c5f80b8dd9cfde5c93.css', bytes: 0/0 verdaccio-s3-plugin | http <-- 200, user: null(199.116.115.131), req: 'GET /-/static/github-oauth-ui/login-button.js', bytes: 0/1401 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/github-oauth-ui/styles.css' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/github-oauth-ui/styles.css', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/2.style.9af79f87cda575da973d.css' verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/manifest.fd8981c8de5ef6fbf3ea.js' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/2.style.9af79f87cda575da973d.css', bytes: 0/0 verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/manifest.fd8981c8de5ef6fbf3ea.js', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/main.fd8981c8de5ef6fbf3ea.js' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/main.fd8981c8de5ef6fbf3ea.js', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/1.fd8981c8de5ef6fbf3ea.js' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/1.fd8981c8de5ef6fbf3ea.js', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/6.fd8981c8de5ef6fbf3ea.js' verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/4.style.79143ae4f619220b955a.css' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/6.fd8981c8de5ef6fbf3ea.js', bytes: 0/0 verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/4.style.79143ae4f619220b955a.css', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/4.fd8981c8de5ef6fbf3ea.js' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/4.fd8981c8de5ef6fbf3ea.js', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/verdaccio/logo' verdaccio-s3-plugin | http <-- 304, user: thes0n1x(199.116.115.131), req: 'GET /-/verdaccio/logo', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/verdaccio/packages' verdaccio-s3-plugin | debug-=- s3: [get] verdaccio-s3-plugin | trace-=- s3: [_getData] already exist verdaccio-s3-plugin | debug-=- s3: [getPackageStorage] @/image-uploader verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] packageName @/image-uploader verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] endpoint https://s3.us-east-1.amazonaws.com verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] region eu-east-1 verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] s3ForcePathStyle undefined verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] accessKeyId verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] secretAccessKey verdaccio-s3-plugin | debug-=- s3: [S3PackageManager readPackage init] name @/image-uploader/@/image-uploader verdaccio-s3-plugin | debug-=- s3: [S3PackageManager _getData init] verdaccio-s3-plugin | http <-- 200, user: thes0n1x(199.116.115.131), req: 'GET /-/verdaccio/packages', bytes: 0/0 verdaccio-s3-plugin | trace-=- s3: [S3PackageManager _getData body] @/image-uploader verdaccio-s3-plugin | trace-=- s3: [S3PackageManager readPackage] packageName: @/image-uploader / data @data verdaccio-s3-plugin | debug-=- s3: [getPackageStorage] rollup-starter-app verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] packageName rollup-starter-app verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] endpoint https://s3.us-east-1.amazonaws.com verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] region eu-east-1 verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] s3ForcePathStyle undefined verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] accessKeyId verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] secretAccessKey verdaccio-s3-plugin | debug-=- s3: [S3PackageManager readPackage init] name rollup-starter-app/rollup-starter-app verdaccio-s3-plugin | debug-=- s3: [S3PackageManager _getData init] verdaccio-s3-plugin | trace-=- s3: [S3PackageManager _getData body] rollup-starter-app verdaccio-s3-plugin | trace-=- s3: [S3PackageManager readPackage] packageName: rollup-starter-app / data @data verdaccio-s3-plugin | [github-oauth-ui] Access denied: user "thes0n1x" is not a member of "" verdaccio-s3-plugin | [github-oauth-ui] Access denied: user "thes0n1x" is not a member of "" verdaccio-s3-plugin | http <-- 200, user: thes0n1x(199.116.115.131), req: 'GET /-/verdaccio/packages', bytes: 0/1027 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/github-oauth-ui/login-button.js' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/github-oauth-ui/login-button.js', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/0.style.19c5f80b8dd9cfde5c93.css' verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/vendors.fd8981c8de5ef6fbf3ea.js' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/0.style.19c5f80b8dd9cfde5c93.css', bytes: 0/0 verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/vendors.fd8981c8de5ef6fbf3ea.js', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/2.style.9af79f87cda575da973d.css' verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/github-oauth-ui/styles.css' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/2.style.9af79f87cda575da973d.css', bytes: 0/0 verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/github-oauth-ui/styles.css', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/manifest.fd8981c8de5ef6fbf3ea.js' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/manifest.fd8981c8de5ef6fbf3ea.js', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/main.fd8981c8de5ef6fbf3ea.js' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/main.fd8981c8de5ef6fbf3ea.js', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/6.fd8981c8de5ef6fbf3ea.js' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/6.fd8981c8de5ef6fbf3ea.js', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/4.style.79143ae4f619220b955a.css' verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/1.fd8981c8de5ef6fbf3ea.js' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/4.style.79143ae4f619220b955a.css', bytes: 0/0 verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/1.fd8981c8de5ef6fbf3ea.js', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/4.fd8981c8de5ef6fbf3ea.js' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/4.fd8981c8de5ef6fbf3ea.js', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/verdaccio/logo' verdaccio-s3-plugin | http <-- 304, user: thes0n1x(199.116.115.131), req: 'GET /-/verdaccio/logo', bytes: 0/0 verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/verdaccio/packages' verdaccio-s3-plugin | debug-=- s3: [get] verdaccio-s3-plugin | trace-=- s3: [_getData] already exist verdaccio-s3-plugin | debug-=- s3: [getPackageStorage] @/image-uploader verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] packageName @/image-uploader verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] endpoint https://s3.us-east-1.amazonaws.com verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] region eu-east-1 verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] s3ForcePathStyle undefined verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] accessKeyId verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] secretAccessKey verdaccio-s3-plugin | debug-=- s3: [S3PackageManager readPackage init] name @/image-uploader/@/image-uploader verdaccio-s3-plugin | debug-=- s3: [S3PackageManager _getData init] verdaccio-s3-plugin | info <-- 199.116.115.131 requested 'GET /-/static/logo.png' verdaccio-s3-plugin | http <-- 304, user: null(199.116.115.131), req: 'GET /-/static/logo.png', bytes: 0/0 verdaccio-s3-plugin | trace-=- s3: [S3PackageManager _getData body] @/image-uploader verdaccio-s3-plugin | trace-=- s3: [S3PackageManager readPackage] packageName: @/image-uploader / data @data verdaccio-s3-plugin | debug-=- s3: [getPackageStorage] rollup-starter-app verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] packageName rollup-starter-app verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] endpoint https://s3.us-east-1.amazonaws.com verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] region eu-east-1 verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] s3ForcePathStyle undefined verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] accessKeyId verdaccio-s3-plugin | trace-=- s3: [S3PackageManager constructor] secretAccessKey verdaccio-s3-plugin | debug-=- s3: [S3PackageManager readPackage init] name rollup-starter-app/rollup-starter-app verdaccio-s3-plugin | debug-=- s3: [S3PackageManager _getData init] verdaccio-s3-plugin | trace-=- s3: [S3PackageManager _getData body] rollup-starter-app verdaccio-s3-plugin | trace-=- s3: [S3PackageManager readPackage] packageName: rollup-starter-app / data @data verdaccio-s3-plugin | [github-oauth-ui] Access denied: user "thes0n1x" is not a member of "" verdaccio-s3-plugin | [github-oauth-ui] Access denied: user "thes0n1x" is not a member of "" verdaccio-s3-plugin | http <-- 200, user: thes0n1x(199.116.115.131), req: 'GET /-/verdaccio/packages', bytes: 0/1027

fatmirsukaliq commented 5 years ago

Hi @n4bb12 , thank you for the quick response. Please see above for the log. I'm also using the aws-s3-storage plugin as well.

n4bb12 commented 5 years ago

@fatmirsukaliq thanks for posting this PR. I was able to reproduce your issue but I think it needs to be fixed in a different place.

GitHub users were able to see packages because they were allowed to log in. It seems the UI has a separate login from the API. Package access was prevented as expected.

I fixed this by preventing UI login if the user is not an org member. https://github.com/n4bb12/verdaccio-github-oauth-ui/commit/d3355dab4e8f9714e64939215c706f7389fe69e7 The fix has been released in 1.10.0.