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.

Sign up for GitHub

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

Jump to bottom

Add failure scenarios audit for check permission #2059

Merged
merged 2 commits into from
Mar 10, 2021
Merged

Add failure scenarios audit for check permission #2059

merged 2 commits into from
Mar 10, 2021

Conversation

Avi-Eyal
Copy link
Contributor

@Avi-Eyal Avi-Eyal commented Mar 4, 2021 •
edited
Loading

What does this PR do?

  • Added missing audits for failure scenarios on check permission

What ticket does this PR close?

Resolves ״Some failure scenarios don't produce audit events #1551״

Checklists

Change log

  • The CHANGELOG has been updated, or
  • This PR does not include user-facing changes and doesn't require a CHANGELOG update

Test coverage

  • This PR includes new unit and integration tests to go with the code changes, or
  • The changes in this PR do not require tests

Documentation

  • Docs (e.g. READMEs) were updated in this PR, and/or there is a follow-on issue to update docs, or
  • This PR does not require updating any documentation

API Changes

  • The OpenAPI spec has been updated to meet new API changes (or an issue has been opened), or
  • The changes in this PR do not affect the Conjur API

@Avi-Eyal Avi-Eyal requested a review from a team as a code owner March 4, 2021 15:06
@Avi-Eyal Avi-Eyal requested review from egvili and uCatu and removed request for a team March 4, 2021 15:07
codeclimate[bot]
codeclimate bot reviewed Mar 4, 2021
View reviewed changes
egvili
egvili reviewed Mar 4, 2021
View reviewed changes
@egvili
Copy link
Contributor

egvili commented Mar 4, 2021

Hi @Avi-Eyal ,
It looks pretty good!
I added some questions and comments above.
Also formal note: the ticket this PR solves is expected to be Git issue and not Jira. You can use #1987 instead

@Avi-Eyal Avi-Eyal force-pushed the ONYX-7550-check-permissins-audit branch 2 times, most recently from 35516fb to ee915f6 Compare March 7, 2021 14:57
egvili
egvili reviewed Mar 7, 2021
View reviewed changes
@Avi-Eyal Avi-Eyal force-pushed the ONYX-7550-check-permissins-audit branch from f237e40 to 3604cf9 Compare March 7, 2021 15:51
@Avi-Eyal Avi-Eyal force-pushed the ONYX-7550-check-permissins-audit branch 2 times, most recently from 7edd838 to 459a9e5 Compare March 8, 2021 13:24
@Avi-Eyal Avi-Eyal requested a review from orenbm March 8, 2021 13:56
@Avi-Eyal Avi-Eyal force-pushed the ONYX-7550-check-permissins-audit branch from 459a9e5 to 78406cf Compare March 8, 2021 15:25
@Avi-Eyal Avi-Eyal requested a review from orenbm March 9, 2021 08:36
@Avi-Eyal Avi-Eyal force-pushed the ONYX-7550-check-permissins-audit branch from 78406cf to 6560f77 Compare March 9, 2021 09:08
uCatu
uCatu suggested changes Mar 9, 2021
View reviewed changes
uCatu
uCatu suggested changes Mar 9, 2021
View reviewed changes
Copy link
Contributor

@uCatu uCatu left a comment

Choose a reason for hiding this comment

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

Changes

@Avi-Eyal Avi-Eyal force-pushed the ONYX-7550-check-permissins-audit branch from 6560f77 to a95f7ac Compare March 9, 2021 16:04
uCatu
uCatu suggested changes Mar 9, 2021
View reviewed changes
Copy link
Contributor

@uCatu uCatu left a comment

Choose a reason for hiding this comment

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

changes

egvili
egvili reviewed Mar 10, 2021
View reviewed changes
@Avi-Eyal Avi-Eyal force-pushed the ONYX-7550-check-permissins-audit branch 2 times, most recently from 0d96fa2 to 967bf40 Compare March 10, 2021 08:38
orenbm
orenbm reviewed Mar 10, 2021
View reviewed changes
@Avi-Eyal Avi-Eyal force-pushed the ONYX-7550-check-permissins-audit branch from 967bf40 to 8a0c663 Compare March 10, 2021 12:36
@Avi-Eyal Avi-Eyal force-pushed the ONYX-7550-check-permissins-audit branch from 8a0c663 to 51d87b1 Compare March 10, 2021 12:52
@Avi-Eyal Avi-Eyal requested a review from uCatu March 10, 2021 13:27
@Avi-Eyal Avi-Eyal force-pushed the ONYX-7550-check-permissins-audit branch from 51d87b1 to caf4af3 Compare March 10, 2021 14:33
@codeclimate
Copy link

codeclimate bot commented Mar 10, 2021

Code Climate has analyzed commit caf4af3 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 89.4% (0.1% change).

View more on Code Climate.

uCatu
uCatu approved these changes Mar 10, 2021
View reviewed changes
Copy link
Contributor

@uCatu uCatu left a comment

Choose a reason for hiding this comment

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

lgtm

@Avi-Eyal Avi-Eyal merged commit 4a7f0e9 into master Mar 10, 2021
@izgeri izgeri mentioned this pull request Mar 30, 2021
Closed
@Avi-Eyal Avi-Eyal deleted the ONYX-7550-check-permissins-audit branch June 1, 2021 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orenbm orenbm orenbm left review comments

@codeclimate codeclimate[bot] codeclimate[bot] left review comments

@egvili egvili egvili left review comments

@uCatu uCatu uCatu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Avi-Eyal @egvili @orenbm @uCatu