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

maintain!: remove ununsed create token body (#1216) #1497

Merged
merged 2 commits into from
Apr 7, 2022

Conversation

BruceMacD
Copy link
Collaborator

  • remove unused create token request body that specified user id

Summary

The token creation endpoint took a user identifier in the request body, but this value is ignored. The token is created for the authorized party calling the endpoint.

This change means the CLI version will need to be upgraded.

Checklist

  • Wrote appropriate unit tests
  • Considered security implications of the change
  • Updated associated docs where necessary
  • Updated associated configuration where necessary
  • Change is backwards compatible if it needs to be (user can upgrade without manual steps?)
  • Nothing sensitive logged
  • Commit message conforms to Conventional Commit
  • GitHub Actions are passing
  • Considered data migrations for smooth upgrades

Related Issues

Resolves #1216

- remove unused create token request body that specified user id
Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

Nice cleanup!

I guess api.CreateTokenRequest can also be removed now? Nothing references it.

Should we allow admins to create tokens for machine users? Or would we have them login as the machine user first and create the token that way? Not for this PR, but wondering how it interacts with the removal of infra destinations add.

@mxyng
Copy link
Collaborator

mxyng commented Apr 6, 2022

We should already have support for user identities creating machine identity access keys. The removal of destinations add relies on this

@dnephin
Copy link
Contributor

dnephin commented Apr 6, 2022

Ah, sorry, I was confused! This is CreateToken, not CreateAccessKey. I got them mixed up.

@mxyng
Copy link
Collaborator

mxyng commented Apr 6, 2022

Easy thing to mix up 😞

- remove api createtokenrequest
@BruceMacD BruceMacD merged commit ac3c509 into main Apr 7, 2022
@BruceMacD BruceMacD deleted the remove-unused-token-req branch April 7, 2022 13:23
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.

Remove request body for CreateTokenRequest
3 participants