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

auth: a new option for configuring TTL of jwt tokens #8302

Merged
merged 2 commits into from Feb 28, 2018

Conversation

mitake
Copy link
Contributor

@mitake mitake commented Jul 25, 2017

This commit adds a new option of --auth-token, ttl, for configuring
TTL of jwt tokens. It can be specified like this:

--auth-token jwt,pub-key=<pub key path>,priv-key=<priv key path>,sign-method=<sign method>,ttl=5m

In the above case, TTL will be 5 minutes.

/cc @fanminshi

@mitake
Copy link
Contributor Author

mitake commented Jul 25, 2017

/cc @vedant15 this PR adds the functionality for configuring TTL of jwt tokens. Unlike simple tokens, jwt tokens' TTL won't be extended by RPCs so clients need to retry after the expiration.

@heyitsanthony
Copy link
Contributor

@mitake will the client automatically acquire a new JWT token if the current one expires?

Also, this probably needs a test case; I can't find where exp is being checked against the current time (jwt-go seems to be capable of verifying exp conforms with the RFC, but it doesn't check the time)...

@mitake
Copy link
Contributor Author

mitake commented Jul 27, 2017

@heyitsanthony yes, clientv3.newAuthRetryWrapper() does the retry. And sure I'll add a test case.

jwt-go checks the expiration with this flow Parser.Parse() -> Parser.ParseWithClaims() -> Claims.Valid() (in the case of etcd, MapClaims.Valid()) -> MapClaims.VerifyExpireAt() with a return value of TimeFunc(). The TimeFunc() is time.Now() in default.

@gyuho
Copy link
Contributor

gyuho commented Oct 2, 2017

@mitake Do we want to include this in 3.3? Otherwise, we can move this to 3.4. Thanks.

@mitake
Copy link
Contributor Author

mitake commented Oct 3, 2017

@gyuho I think it is not an emergent stuff so moving to 3.4 is ok.

@gyuho gyuho added this to the v3.4.0 milestone Oct 3, 2017
@mitake
Copy link
Contributor Author

mitake commented Oct 11, 2017

@gyuho updated for an e2e test case, could you take a look?

This commit adds a new option of --auth-token, ttl, for configuring
TTL of jwt tokens. It can be specified like this:
```
--auth-token jwt,pub-key=<pub key path>,priv-key=<priv key path>,sign-method=<sign method>,ttl=5m
```

In the above case, TTL will be 5 minutes.
@mitake
Copy link
Contributor Author

mitake commented Feb 27, 2018

rebased on the latest master

@gyuho
Copy link
Contributor

gyuho commented Feb 27, 2018

So, if no TTL given, does this defaults to 5-min? Is there any specific reason?

@mitake
Copy link
Contributor Author

mitake commented Feb 27, 2018

No specific reason for now. If you think we should change, please let me know.

@gyuho
Copy link
Contributor

gyuho commented Feb 27, 2018

Would it be breaking change, if previous behavior is auth token never expires? I am ok with either way.

Maybe highlight this change more loud around configuration.md and etcdmain/help.go?

@mitake
Copy link
Contributor Author

mitake commented Feb 27, 2018

It isn't a breaking change. clientv3 automatically refresh the expired token so existing etcd users won't be aware about this change. It is ensured in the test https://github.com/coreos/etcd/pull/8302/files#diff-4e20b6b68ada25d636aa265f49dcfa19

@gyuho
Copy link
Contributor

gyuho commented Feb 27, 2018

Oh, it will be just assigned a new token. Makes sense.

Copy link
Contributor

@gyuho gyuho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm on test passes.

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

Successfully merging this pull request may close these issues.

None yet

3 participants