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: allow setting username/description during rotation #85

Merged
merged 8 commits into from
Apr 25, 2023

Conversation

TJM
Copy link
Contributor

@TJM TJM commented Apr 25, 2023

This simply allows the optional setting of a username and/or description when rotating the admin token.

It still needs docs and tests.

Basic functionality:

[tmcneely@local artifactory-secrets-plugin]$ vault read artifactory/config/admin
Key                    Value
---                    -----
access_token_sha256    d3edeabe961e3741ae0e1d8795f56e8e6fa1e562e257673c4367e58ef49f9c48
scope                  applied-permissions/admin
token_id               393984a5-2948-4fa6-bd91-49301decdbe4
url                    http://localhost:8082
use_expiring_tokens    false
username               vault-admin
version                7.55.10
[tmcneely@local artifactory-secrets-plugin]$ vault write artifactory/config/rotate username=tommy-dev-vault-admin description="Admin account for vault secrets engine"
Success! Data written to: artifactory/config/rotate
[tmcneely@local artifactory-secrets-plugin]$ vault read artifactory/config/admin
Key                    Value
---                    -----
access_token_sha256    f9c63ee191570be71fabd19ff0b3104e33ad4bc9b6970b4bbdfa05dca21c36b8
scope                  applied-permissions/admin
token_id               a94b7ea8-7588-4810-8853-80809642e002
url                    http://localhost:8082
use_expiring_tokens    false
username               tommy-dev-vault-admin
version                7.55.10
[tmcneely@local artifactory-secrets-plugin]$ curl -H "Authorization: Bearer $ACCESS_TOKEN"  http://localhost:8082/access/api/v1/tokens/a94b7ea8-7588-4810-8853-80809642e002
{
  "token_id" : "a94b7ea8-7588-4810-8853-80809642e002",
  "subject" : "jfac@01gykd5tha8tmk06x1sjmg1seb/users/tommy-dev-vault-admin",
  "issued_at" : 1682391447,
  "issuer" : "jfac@01gykd5tha8tmk06x1sjmg1seb",
  "description" : "Admin account for vault secrets engine",
  "refreshable" : false
}%

NOTE: The token description is stored in the database, but is not in the actual JWT token.

/closes #69

@TJM TJM force-pushed the feat/admin-username branch from 243a735 to 90f1d02 Compare April 25, 2023 02:59
@TJM TJM force-pushed the feat/admin-username branch from 90f1d02 to 5b6b456 Compare April 25, 2023 03:03
path_config_rotate.go Outdated Show resolved Hide resolved
path_config_rotate_test.go Outdated Show resolved Hide resolved
@alexhung alexhung added the enhancement New feature or request label Apr 25, 2023
t.Fatal(err)
}

accTestEnv.RotatePathConfigWithDetails(t)
Copy link
Member

Choose a reason for hiding this comment

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

I built the acceptance tests structure based on HashiCorp example, which is fine as a starter but probably a little simplistic. Some of the other acceptance tests have multiple func calls to setup, test, then cleanup. Ideal, I believe, is to make these funcs composable so you can mix and match as necessary.

Copy link
Member

@alexhung alexhung Apr 25, 2023

Choose a reason for hiding this comment

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

e.g.

t.Run("update", accTestEnv.UpdatePathConfig)
t.Run("read", accTestEnv.ReadPathConfig)
t.Run("rotate with details", accTestEnv.RotatePathConfigWithDetails) // New func
t.Run("read with details", accTestEnv.ReadPathConfigWithDetails) // New func

test_utils.go Outdated Show resolved Hide resolved
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.

Looking pretty good. PR still in 'draft' mode so I assume it's not ready to be reviewed/approved yet. LMK.

@TJM TJM marked this pull request as ready for review April 25, 2023 20:20
@TJM
Copy link
Contributor Author

TJM commented Apr 25, 2023

Ooops, docs (README) would be good. :)

DONE. take a look and let me know if any changes are needed.

Thanks,
Tommy

@alexhung alexhung merged commit 4d434a3 into jfrog:master Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow for "username" parameter on /config/rotate to change token username
2 participants