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

feat: parse and re-use admin token username #34

Merged
merged 1 commit into from
Feb 22, 2023

Conversation

TJM
Copy link
Contributor

@TJM TJM commented Feb 21, 2023

This will parse the username from the current token and re use it for the new one during rotation.

It will default to "admin" if the subject parsing doesn't work.

Copy link
Member

@alexhung alexhung left a comment

Choose a reason for hiding this comment

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

LGTM. This reminds me that we need to add more tests 😄

@TJM
Copy link
Contributor Author

TJM commented Feb 21, 2023

I did test this against our Artifactory 7.38 instance here. For all that is worth. Automated tests would be a "good thing" (tm)

Copy link

@shrajfr12 shrajfr12 left a comment

Choose a reason for hiding this comment

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

lgtm

@TJM
Copy link
Contributor Author

TJM commented Feb 21, 2023

This allows you to name your access token something reasonable like vault-artifactory-admin ... I am not sure which version of artifactory required the username to specifically be admin, but on a reasonably recent version, that is no longer the case, and can make debugging issues much easier.

@alexhung
Copy link
Member

@TJM Don't quote me on this, but I think it's the switch to scoped token that remove the restriction of admin username. However, this doesn't mean you can use any username as it still needs to match with a valid username in Artifactory.

Copy link

@Turhsus Turhsus left a comment

Choose a reason for hiding this comment

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

LGTM, nice catch

@alexhung alexhung merged commit 1adb380 into jfrog:master Feb 22, 2023
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

Successfully merging this pull request may close these issues.

None yet

4 participants