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

Improve token auth #168

Closed
Meeeeow opened this Issue Apr 22, 2017 · 21 comments

Comments

Projects
8 participants
@Meeeeow
Copy link
Member

Meeeeow commented Apr 22, 2017

My reason:

Current implement make token expire after 24 hours, it's impossible to use with CI or automatic deploy.
I think we can let user choose token expire time or never expire with reset feature.

Use (JWT may better than origin code, any idea? @juanpicado

@juanpicado

This comment has been minimized.

Copy link
Member

juanpicado commented Apr 22, 2017

Yeah, sound great, but I have some thoughts here.

  • I guess that will add a new dependency? Am I wrong?
  • Does make sense put the auth plugin as an external dependency? (eg: https://github.com/verdaccio/verdaccio-plugin-auth-htpasswd) In some point has to disappear from the source code. I've been thinking on that for a while
  • How would looks like the yaml configuration file?
  • I think will help to avoid workarounds as here #139

What do you think?

@Meeeeow

This comment has been minimized.

Copy link
Member Author

Meeeeow commented Apr 22, 2017

Something was wrong, current implement have issue_token method but never called, just encrypt user name and password, send it to client, so it will not expire, but this may not the best way.

  1. Right, origin dependency 'jju' will be replaced by 'jsonwebtoken'
  2. That's possible solution, i'll look about plugin
  3. I'm not sure
    • if publish as plugin, may possible configure it in yaml file and use a 'lift time' token everywhere
    • if rewrite auth middleware, user can generate or reset this special token from web ui and use it on CI, but login token still expired after some days for safe
  4. Yeah, that token just encrypted user name and password, may be safe, but may make user fell uncomfortable
@juanpicado

This comment has been minimized.

Copy link
Member

juanpicado commented Apr 22, 2017

Yeah, I saw Auth is kinda mess.

  1. Any change to the auth method will invalidate all htpasswd users and piss off more than one user, it's needs some fallback.
  2. Cool.
  3. a) The secret key currently is stored within .sinopia-db.json and I'm not 100% sure if the token can be stored in the config file.
    b) I like the UI that npm use to invalidate tokens, currently there is no way to invalidate the token.
  4. I'm still thinking about it.

@juanpicado juanpicado added this to the Future milestone Apr 22, 2017

@Meeeeow

This comment has been minimized.

Copy link
Member Author

Meeeeow commented Apr 23, 2017

Maybe we can ship it as plugin with verdaccio, call it like verdaccio-auth3 or whatever, but not enable as default before major upgrade?

If user have not enabled this plugin, just disable the 'special token' feature in web ui (special token mean the token which will not expire, maybe a string like current secret key)

@idangozlan

This comment has been minimized.

Copy link
Contributor

idangozlan commented Jun 26, 2017

Is there anything new with that?

@idangozlan

This comment has been minimized.

Copy link
Contributor

idangozlan commented Jun 26, 2017

Ok so after some debugging sessions, I see that the token is not invalidate by time, it's just an encrypted user:password, so that means that it will behave the same way as the NPM original registry, which invalid the token when the user that issued that token is changing the password.

@Meeeeow

This comment has been minimized.

Copy link
Member Author

Meeeeow commented Jun 26, 2017

@idangozlan We are working on this, token for webui will expire after 24h in next minor release, npm token will not change before next major release, but it will release as a standalone plugin

@juanpicado juanpicado modified the milestones: 3.0.0, Future Jun 26, 2017

@juanpicado juanpicado referenced this issue Oct 22, 2017

Merged

Release 3.x #376

14 of 15 tasks complete

@juanpicado juanpicado added the auth label Jan 11, 2018

@tvb

This comment has been minimized.

Copy link

tvb commented Jan 17, 2018

@Meeeeow can you confirm any local generated password hash works in CI/CD if you copy it?

So copy this from local

$ cat ~/.npmrc.bak
@myscope:registry=http://mydomain.com:5000/
//mydomain.com:5000/:_password=s1hbmxpa2UtY3ViYW4w3323csjcewucuieiciw039nf2tbGl0aGl1bS11bnRydXRoLWNvbXBvdW5kLi42ifn2u294bfmVsb25nLXNoZWxs
//mydomain.com:5000/:username=myuser
//mydomain.com:5000/:email=email@mydomain.com
//mydomain.com:5000/:always-auth=false

And use this in CI/CD .npmrc

@Meeeeow

This comment has been minimized.

Copy link
Member Author

Meeeeow commented Jan 19, 2018

@tvb Yes, it will never expire before you change password

@juanpicado juanpicado self-assigned this Mar 19, 2018

@juanpicado juanpicado added this to To do in Roadmap Apr 12, 2018

@juanpicado juanpicado modified the milestones: 3.0.0, Future, 3.0.x May 30, 2018

@juanpicado

This comment has been minimized.

Copy link
Member

juanpicado commented Jun 3, 2018

This is the current status:

My plan is generate all tokens using jsonwebtoken based on HS256 instead crypto.createCipher AES192.

Reasons:

  • Easy to expire tokens by demand.
  • Variety of algorithms to be used
  • It's built-in asynchronous (I might use generators here to get some boots)
  • Consistency with tokens generated in the WebUI

We have to offer always backward compatibility, thus, AES192 should remain by default, but also, should offer such deprecation process while users move to new tokens based on jwt. The downside here is soon or later all user will have to upgrade their tokens.

Since we do not have any sort configuration specified for security I suggest the following based on what I've seen in the source code.

security:
   token:
      web: 24h # by default
      api: never # by defaukt
   algorithm: HS256 # by default

A couple of problems still are on the table:

  • In order to invalidate tokens, as npmjs does, it allows the user change the password, this is not done yet on our side (https://github.com/verdaccio/verdaccio/issues/401).
  • It is also necessary to highlight create tokens might be really handy for many users #541
  • Invalidate tokens #724
  • Secret is being stored in the storage and there is no way to provide custom secret key

I'm not an expert in cryptography, so, if you feel strong in this area do not hesitate to contribute to this discussion.

@johan-smits

This comment has been minimized.

Copy link

johan-smits commented Jul 6, 2018

@Meeeeow and @tvb how can you create a password hash?
And does this already work the the beta docker?

@juanpicado juanpicado removed this from the 3.x.x milestone Jul 25, 2018

@juanpicado juanpicado removed this from In progress in Roadmap Jul 25, 2018

@juanpicado juanpicado added this to In Progress in Verdaccio 4 Jul 25, 2018

@juanpicado

This comment has been minimized.

Copy link
Member

juanpicado commented Jul 25, 2018

@johan-smits it does not work on beta, it will be part of next major upgrade.

@johan-smits

This comment has been minimized.

Copy link

johan-smits commented Jul 27, 2018

@juanpicado thnx for the reply.
A question what is the best practice for deploying with a CD (like Travis/Jenkins) and have protection enabled?

@juanpicado

This comment has been minimized.

Copy link
Member

juanpicado commented Jul 27, 2018

cc: @sergiohgz knows more about it 👍

@sergiohgz

This comment has been minimized.

Copy link
Contributor

sergiohgz commented Jul 31, 2018

@johan-smits I don't understand what you mean with protection enabled, please explain it or we can talk in Discord 😉

@johan-smits

This comment has been minimized.

Copy link

johan-smits commented Jul 31, 2018

@sergiohgz with protection enabled I ment authentication enabled.
We have fixed the issue by using: npm/npm-registry-client before the publish to "login" and retrieve a token.

@sergiohgz

This comment has been minimized.

Copy link
Contributor

sergiohgz commented Jul 31, 2018

@johan-smits Actually what I have is a token generated from login a user against my Verdaccio instance and saving that as environment variable across our Jenkins cluster. Your solution seems to be so great, but it don't match in my company architecture, I will try at home 😉

@bufferoverflow

This comment has been minimized.

Copy link
Member

bufferoverflow commented Jul 31, 2018

Just some ideas...

I think that an Open ID connect core / jwt standard compliant infrastructure could help here to solve that properly, using a standard as docker does whould really help here. The npm registry is not in line with any standard. Applying concepts as used within Docker, see links below, could simplify that and increase interoperability significantly.

Further readings

@juanpicado juanpicado added this to the 4.0.0 milestone Jul 31, 2018

juanpicado added a commit that referenced this issue Aug 12, 2018

juanpicado added a commit that referenced this issue Aug 21, 2018

feat: add support for jwt on api (#896)
* feat: add support for jwt on api

* test: add unit test for sign token with jwt

add multiple scenarios with configuration file

* chore: add JWT verification on middleware

* chore: restore headless

* chore: restore middleware header validation

* refactor: fix login whether user exists

* refactor: JWT is signed asynchronously

* refactor: better structure and new naming convention

* test: add unit test for token signature

* test: add unit test for creating user with JWT enabled

#168

* docs: add security section jwt

* refactor: renable  web auth middleware

* test(auth): add legacy disabled scenario

* chore: update gitignore

* chore: add some es6 sugar

* feat: enable JWT token signature for new installations

* chore: add yaml files to git

I forgot add this before 😷

* chore: trace log on auth

in case we want more output
@juanpicado

This comment has been minimized.

Copy link
Member

juanpicado commented Aug 21, 2018

Shipped on Verdaccio 4.

docker pull verdaccio/verdaccio:4.x-next

@juanpicado juanpicado closed this Aug 21, 2018

Verdaccio 4 automation moved this from In Progress to Done Aug 21, 2018

priscilawebdev added a commit that referenced this issue Sep 16, 2018

feat: add support for jwt on api (#896)
* feat: add support for jwt on api

* test: add unit test for sign token with jwt

add multiple scenarios with configuration file

* chore: add JWT verification on middleware

* chore: restore headless

* chore: restore middleware header validation

* refactor: fix login whether user exists

* refactor: JWT is signed asynchronously

* refactor: better structure and new naming convention

* test: add unit test for token signature

* test: add unit test for creating user with JWT enabled

#168

* docs: add security section jwt

* refactor: renable  web auth middleware

* test(auth): add legacy disabled scenario

* chore: update gitignore

* chore: add some es6 sugar

* feat: enable JWT token signature for new installations

* chore: add yaml files to git

I forgot add this before 😷

* chore: trace log on auth

in case we want more output

priscilawebdev added a commit that referenced this issue Sep 19, 2018

feat: add support for jwt on api (#896)
* feat: add support for jwt on api

* test: add unit test for sign token with jwt

add multiple scenarios with configuration file

* chore: add JWT verification on middleware

* chore: restore headless

* chore: restore middleware header validation

* refactor: fix login whether user exists

* refactor: JWT is signed asynchronously

* refactor: better structure and new naming convention

* test: add unit test for token signature

* test: add unit test for creating user with JWT enabled

#168

* docs: add security section jwt

* refactor: renable  web auth middleware

* test(auth): add legacy disabled scenario

* chore: update gitignore

* chore: add some es6 sugar

* feat: enable JWT token signature for new installations

* chore: add yaml files to git

I forgot add this before 😷

* chore: trace log on auth

in case we want more output
@cojack

This comment has been minimized.

Copy link

cojack commented Oct 10, 2018

@juanpicado where is documentation about what type of expiresIn configruation file can handle? Where is documentation of what "security" section in this yaml file stands for?

@juanpicado

This comment has been minimized.

Copy link
Member

juanpicado commented Oct 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.