feat: Rotate token with JWT token validation #17
Conversation
TJM
force-pushed
the
feat/rotate-validate
branch
from
17a8139
to
19f4acd
Compare
TJM
changed the title
|
|
||
| // Create admin role for the new token | ||
| role := &artifactoryRole{ | ||
| Username: "admin", |
There was a problem hiding this comment.
what if configured user is not called admin?
There was a problem hiding this comment.
I agree with @EvertonSA. We should be able to parse the sub from the claims and re-use it?
There was a problem hiding this comment.
Hmm, Maybe it was only certain version of Artifactory that were sensitive to this, but I am pretty sure we had problems when we used anything but "admin" ... I would love to see something like vault or whatever here. Lets test this a few different ways hard-coded before changing the code to use the sub. I vaguely recall removing all that stuff and putting it back to "admin" ... but that was 5 months ago :)
There was a problem hiding this comment.
For access token, username needs to be a valid user account in the system. admin is a built-in user account so it will always work. For other value, it needs to match an existing user account. So if your system doesn't have 'vault' user account then it will fail.
There was a problem hiding this comment.
Has anyone had time to test this? (US has been in holiday)
EDIT: According to this project's own README.md, you have to use "admin" as the username. Has this changed?
There was a problem hiding this comment.
No change. So I think we are good for now. In future, we should accommodate any username .
|
|
||
| // Create admin role for the new token | ||
| role := &artifactoryRole{ | ||
| Username: "admin", |
There was a problem hiding this comment.
I agree with @EvertonSA. We should be able to parse the sub from the claims and re-use it?
I wrote this code like 5 months ago, and was hoping for some feedback ;) That is why it is in draft state :) |
- added more detail to the 7.21.1 comment - uncommented error log incase of failed rootCert retreival
There was a problem hiding this comment.
Should this section in README.md be removed? https://github.com/jfrog/artifactory-secrets-plugin/blob/master/README.md?plain=1#L22
Also fixed a few formatting issues
Updated README.md to remove that section, add a new part to show the /artifactory/config/rotate, and a few minor formatting things that VSCode (Markdown Lint) deemed necessary :) |
There was a problem hiding this comment.
I feel this looks good now.
@shrajfr12 and @mritunjaykumar Please review as well.
|
thanks for merging colleagues, I will test and also provide feedbacks |
|
tested exactly like described and it works fine! |
Rotate Artifactory admin token so that only vault knows the token.
Supersedes #16
Closes #14