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

fix: reject API admin tokens when importing features #4016

Merged
merged 4 commits into from
Jun 20, 2023

Conversation

thomasheartman
Copy link
Contributor

@thomasheartman thomasheartman commented Jun 20, 2023

This PR fixes an issue where trying to use an admin token to import features via the API resulted in a 500 (due to missing properties).

The solution is to catch when the user is using and admin token in the controller, throw a 400, and tell them to use personal access tokens or service accounts.

The PR includes a new test file for this specific use case. We don't really test these cases many other places, so it seemed the logical choice.

@vercel
Copy link

vercel bot commented Jun 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
unleash-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 20, 2023 1:00pm
unleash-monorepo-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 20, 2023 1:00pm

Copy link
Contributor

@kwasniew kwasniew left a comment

Choose a reason for hiding this comment

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

Nice handled! Minor comment: I'd remove code that's not necessary to test the scenario so that the test reader is not distracted by irrelevant details.

@thomasheartman
Copy link
Contributor Author

Yeah, great catch. I've shaved off a bunch of lines for that 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants