Skip to content
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

[Bug] TypeError: (groups || []).concat is not a function , secure ldap is failing TLS randomly #33

Closed
kopax opened this issue May 31, 2018 · 10 comments

Comments

@kopax
Copy link
Contributor

kopax commented May 31, 2018

We are using:

We have GItLab-CI runner that run npm install --registry https://our.registry.com for the project.

I can connect repetitively multiple time to the LDAP, until:

In node 10.1.0

http <-- 200, user: me(172.18.0.1 via 172.16.14.10), req: 'GET /node-int64', bytes: 0/2481
 http --> 304, req: 'GET https://registry.npmjs.org/multicast-dns-service-types' (streaming)
 http --> 304, req: 'GET https://registry.npmjs.org/multicast-dns-service-types', bytes: 0/0
 debug--- connected after 1 attempt(s)
 debug--- connected after 1 attempt(s)
 debug--- connected after 1 attempt(s)
 debug--- failed to connect after 1 attempts
 fatal--- uncaught exception, please report this
Error: Client network socket disconnected before secure TLS connection was established
    at TLSSocket.onConnectEnd (_tls_wrap.js:1092:19)
    at Object.onceWrapper (events.js:273:13)
    at TLSSocket.emit (events.js:187:15)
    at endReadableNT (_stream_readable.js:1086:12)
    at process._tickCallback (internal/process/next_tick.js:63:19)

In node 8.11.2

 info --> making request: 'GET https://registry.npmjs.org/array-unique'
 debug--- connected after 1 attempt(s)
 debug--- connected after 1 attempt(s)
 fatal--- uncaught exception, please report this
TypeError: (groups || []).concat is not a function
    at authenticatedUser (/usr/local/lib/node_modules/verdaccio/build/lib/auth.js:372:32)
    at /usr/local/lib/node_modules/verdaccio/build/lib/auth.js:105:26
    at sendResult (/usr/local/lib/node_modules/verdaccio-ldap/node_modules/ldapjs/lib/client/client.js:1395:12)
    at messageCallback (/usr/local/lib/node_modules/verdaccio-ldap/node_modules/ldapjs/lib/client/client.js:1421:16)
    at /usr/local/lib/node_modules/verdaccio-ldap/node_modules/ldapjs/lib/client/client.js:1282:14
    at Array.forEach (<anonymous>)
    at Client._onClose (/usr/local/lib/node_modules/verdaccio-ldap/node_modules/ldapjs/lib/client/client.js:1272:19)
    at Object.onceWrapper (events.js:272:13)
    at TLSSocket.emit (events.js:180:13)
    at _handle.close (net.js:541:12)
    at TCP.done [as _onclose] (_tls_wrap.js:379:7)

We can connect repititively to the LDAP but then this error happen and prevent us totally from using verdaccio.

We are not using any proxy between verdaccio and the ldap, why is this happening?

Is there a way to prevent such bug ?

@kopax kopax changed the title [Bug] What are verdaccio requirements? verdaccio and verdaccio-ldap can't support a CI process [Bug] TypeError: (groups || []).concat is not a function , secure ldap is failing TLS randomly Jun 1, 2018
@kopax
Copy link
Contributor Author

kopax commented Jun 1, 2018

I have added some console.log on group to know why it was not an array and I have found this:

// When it work, group is an Array of group except arr[0] which has the firstname and lastname of the LDAP users
[
  "John Doe",
  "npm_users"
]
 debug-=- updating package @semantic-release/gitlab info
 debug-=- updating package @semantic-release/npm info
 debug-=- updating package babel-loader info
// This is what makes the TypeError, is it also because of the TLS error ?
{
  "messageID": "2",
  "controls": [],
  "status": "unbind",
  "matchedDN": "",
  "errorMessage": "",
  "referrals": [],
  "connection": null
}

We can get to this result only with node 8 or earlier, with node 10 it is about a TLS error.

I really don't know how to correct this, can we re-authenticate in case of failure?

@kopax
Copy link
Contributor Author

kopax commented Jun 3, 2018

There might be a way, without db to use application memory on first login to keep the user authenticated. bufferoverflow/verdaccio-gitlab#13 (comment)

@juanpicado
Copy link
Contributor

juanpicado commented Jun 5, 2018

This commit verdaccio/verdaccio@a62688f will help to avoid such use case happens again.

@kopax
Copy link
Contributor Author

kopax commented Jun 6, 2018

Very nice @juanpicado, not a fix but at least a message to trace.

@juanpicado
Copy link
Contributor

yw, you can try with [email protected]. Shipped already.

@kopax
Copy link
Contributor Author

kopax commented Jun 6, 2018

Well, the ldap error is not solved so we removed the ldap plugin and we do not have this error.

I'll modify my ldap when I'll have enough time to take care of the ldap/redis issue.

@GlennMatthys
Copy link
Contributor

This is the root issue: #31 which has been fixed in PR #32 but there has not been a new release yet. You can apply the changes yourself to your local files, it's only a small change.

@kopax
Copy link
Contributor Author

kopax commented Jun 7, 2018

Thanks, but that doesn't solve the spamming that should never happen.

What about this : idangozlan/verdaccio-bitbucket#10 (comment) ?

can we try the solution with this plugin? I don't think it support multiple instance thought it will work but it will still login more than once.

There won't be as much spamming if you implement the getCache, and people could even pass their own getCache function so they can cache from wherever they want.

@eshion
Copy link

eshion commented Jun 8, 2018

I encountered a similar issue:

warn --- basic authentication is deprecated, please use JWT instead
warn --- basic authentication is deprecated, please use JWT instead
warn --- basic authentication is deprecated, please use JWT instead
warn --- basic authentication is deprecated, please use JWT instead
warn --- basic authentication is deprecated, please use JWT instead
warn --- basic authentication is deprecated, please use JWT instead
warn --- basic authentication is deprecated, please use JWT instead
warn --- basic authentication is deprecated, please use JWT instead
fatal--- uncaught exception, please report this
TypeError: user groups is different than an array
at /usr/local/lib/node_modules/verdaccio/build/lib/auth.js:106:19
at sendResult (/usr/local/lib/node_modules/verdaccio-ldap/node_modules/ldapjs/lib/client/client.js:1395:12)
at messageCallback (/usr/local/lib/node_modules/verdaccio-ldap/node_modules/ldapjs/lib/client/client.js:1421:16)
at /usr/local/lib/node_modules/verdaccio-ldap/node_modules/ldapjs/lib/client/client.js:1282:14
at Array.forEach ()
at Client._onClose (/usr/local/lib/node_modules/verdaccio-ldap/node_modules/ldapjs/lib/client/client.js:1272:19)
at Object.onceWrapper (events.js:316:30)
at emitOne (events.js:115:13)
at TLSSocket.emit (events.js:210:7)
at _handle.close (net.js:545:12)
at TCP.done [as _onclose] (_tls_wrap.js:356:7)

Alexandre-io added a commit that referenced this issue Jul 24, 2018
feat(authCache): added auth cache to fix #37 #39 #35 #33
@Alexandre-io
Copy link
Owner

Should be fixed since 2.1.0

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

No branches or pull requests

5 participants