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

Add custom extractor for filtering audit logs #4829

Merged
merged 1 commit into from
Oct 22, 2024
Merged

Conversation

nickgerace
Copy link
Contributor

@nickgerace nickgerace commented Oct 18, 2024

Description

This PR adds a custom extractor for filtering audit logs. Generally, it handles array notated query string parameters, which is likely the first time we needed to do so in sdf.

Comment on lines 150 to 151
let perms = Permission::new(workspace_object.clone(), "approve", user_object.clone());
let bad_perms = Permission::new(workspace_object, "approve", user_object2);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

cargo fmt

.uri
.query()
.ok_or(not_found_error("no query string found in uri"))?;
let deserialized = serde_qs::from_str(query).map_err(|err| internal_error(err))?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this a 422?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean: shouldn't this produce a 422 and not a 500? If we fail to deserialize it's because the input was invalid

Copy link
Contributor

Choose a reason for hiding this comment

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

Same with the not found. That's invalid input, not a resource not found

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah fair point. Will fix

Copy link
Contributor

Choose a reason for hiding this comment

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

(or maybe 400: Bad Request)

zacharyhamm
zacharyhamm previously approved these changes Oct 21, 2024
zacharyhamm
zacharyhamm previously approved these changes Oct 21, 2024
@nickgerace nickgerace added this pull request to the merge queue Oct 21, 2024
@nickgerace nickgerace removed this pull request from the merge queue due to a manual request Oct 21, 2024
This commit adds a custom extractor for filtering audit logs. Generally,
it handles array notated query string parameters, which is likely the
first time we needed to do so in sdf.

Signed-off-by: Nick Gerace <[email protected]>
@nickgerace nickgerace added this pull request to the merge queue Oct 21, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Oct 21, 2024
@nickgerace nickgerace added this pull request to the merge queue Oct 21, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 21, 2024
@nickgerace nickgerace added this pull request to the merge queue Oct 22, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 22, 2024
@nickgerace nickgerace added this pull request to the merge queue Oct 22, 2024
Merged via the queue into main with commit 1bdd992 Oct 22, 2024
8 checks passed
@nickgerace nickgerace deleted the nick/eng-2751 branch October 22, 2024 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants