Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

internal/server: Add many validations to authenticated endpoints #2273

Merged
merged 18 commits into from
Sep 13, 2021

Conversation

briancain
Copy link
Member

This pull request adds many validations to various authenticated endpoints that would panic if given an empty request body.

Fixes #2270

@briancain briancain requested a review from a team September 9, 2021 22:09
@github-actions github-actions bot added the core label Sep 9, 2021
Copy link
Contributor

@izaaklauer izaaklauer left a comment

Choose a reason for hiding this comment

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

So good!

@briancain briancain requested a review from a team September 9, 2021 22:13
Copy link
Contributor

@krantzinator krantzinator left a comment

Choose a reason for hiding this comment

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

3 issues in 1! 🚀

Copy link
Contributor

@jgwhite jgwhite left a comment

Choose a reason for hiding this comment

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

Looks air-tight to me. I tested it quickly against the web UI and the errors come through pretty nicely:

CleanShot 2021-09-13 at 12 56 00@2x

(In reality you can’t submit the form without entering something in the name field, so users shouldn’t ever get into this state, but useful to see what these validation messages look like in general)

))
}

// ValidateGetProjectRequest
Copy link
Contributor

Choose a reason for hiding this comment

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

[sand] This comment doesn’t match the function below it. But also: these comments don’t add much, could we drop them altogether?

Copy link
Member Author

@briancain briancain Sep 13, 2021

Choose a reason for hiding this comment

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

Oops, yeah probably from all the copy paste to get the bare functions around before adding the content :D I can just fix the comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

If I were to do this again I'd probably delete them but now that they are there I'll probably leave them be 😅

@briancain briancain merged commit e317fc8 into main Sep 13, 2021
@briancain briancain deleted the bug/server/validate-api-endpoints branch September 13, 2021 15:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

internal/server: Add API validations to all authenticated endpoints
4 participants