Skip to content

fix: do not allow operator token from being deleted#26418

Merged
praveen-influx merged 4 commits intomainfrom
praveen/fix-delete-operator-token
May 15, 2025
Merged

fix: do not allow operator token from being deleted#26418
praveen-influx merged 4 commits intomainfrom
praveen/fix-delete-operator-token

Conversation

@praveen-influx
Copy link
Copy Markdown
Contributor

closes: https://github.com/influxdata/influxdb_pro/issues/819

Tests

❯ cargo run -- delete token --token-name _admin --token $TOKEN
     Running `target/debug/influxdb3 delete token --token-name _admin --token apiv3_79XMCTDB-hNiwOtTwQ-rwLCnvz9BgBWKPC8Gvq5AOPEcqfQtzi9g6yXNp_0xQsHI4nDQpB_wWMw8wzXKNypwQQ`
Are you sure you want to delete "_admin"? Enter 'yes' to confirm
yes
Cannot delete operator token, to regenerate an operator token, use `influxdb3 create token --admin --regenerate --token $TOKEN`

@praveen-influx praveen-influx requested a review from a team May 14, 2025 12:24
@praveen-influx praveen-influx force-pushed the praveen/fix-delete-operator-token branch from 6d32af6 to 9b60be9 Compare May 14, 2025 13:27
@peterbarnett03
Copy link
Copy Markdown
Contributor

Is there a reason we even allow the prompt on deleting the _admin if we know it's not going to be allowed?

It looks like the delete_token functionality is more nested than the high-level where the CLI works, so not too concerned if it's a decision for code simplicity, and not pulling the const name higher up; but also something that could be simpler.

Copy link
Copy Markdown
Contributor

@hiltontj hiltontj left a comment

Choose a reason for hiding this comment

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

I had one nit on a HTTP response code, but not sure if I'm right on that. I will approve for now.

let json: CreateTokenWithPermissionsResponse = result.json().await.unwrap();
info!(?json, "test: result running the token delete");
assert_eq!(json.id, 2);
assert_eq!(result.status(), StatusCode::INTERNAL_SERVER_ERROR);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could this potentially be a 409 CONFLICT error?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes - it can, I'll change that too.

Copy link
Copy Markdown
Contributor

@peterbarnett03 peterbarnett03 left a comment

Choose a reason for hiding this comment

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

Just a change to wording.

&& message == "cannot delete operator token"
{
println!(
"Cannot delete operator token, to regenerate an operator token, use `influxdb3 create token --admin --regenerate --token $TOKEN`"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The operator token "_admin" is required cannot be deleted. To regenerate an operator token, use influxdb3 create token --admin --regenerate --token $TOKEN.

@praveen-influx
Copy link
Copy Markdown
Contributor Author

Is there a reason we even allow the prompt on deleting the _admin if we know it's not going to be allowed?

@peterbarnett03 - not really, I can easily pull this up. We don't even need to go to the server to check. I'll make that change too.

Copy link
Copy Markdown
Contributor

@hiltontj hiltontj left a comment

Choose a reason for hiding this comment

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

Looks good from my end.

Copy link
Copy Markdown
Contributor

@peterbarnett03 peterbarnett03 left a comment

Choose a reason for hiding this comment

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

LGTM. I added the missing word "and" (my fault from above) and a clarifying colon to separate the command from the info.

@praveen-influx praveen-influx merged commit b404e84 into main May 15, 2025
12 checks passed
@praveen-influx praveen-influx deleted the praveen/fix-delete-operator-token branch May 15, 2025 08:10
praveen-influx added a commit that referenced this pull request May 15, 2025
* fix: do not allow operator token from being deleted

closes: influxdata/influxdb_pro#819

* refactor: address PR feedback

* fix: add a word and clarifying colon

* fix: failing test

---------

Co-authored-by: Peter Barnett <peter.barnett03@gmail.com>

commit hash in main: b404e84
praveen-influx added a commit that referenced this pull request May 15, 2025
* fix: do not allow operator token from being deleted

closes: influxdata/influxdb_pro#819

* refactor: address PR feedback

* fix: add a word and clarifying colon

* fix: failing test

---------

Co-authored-by: Peter Barnett <peter.barnett03@gmail.com>

commit hash in main: b404e84
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.

3 participants