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 check allowed for user #19

Closed
wants to merge 2 commits into from
Closed

Fix check allowed for user #19

wants to merge 2 commits into from

Conversation

alex-vento
Copy link

There was an error on line #80 in warden_local.go. If it return from function on this line, we do not check roles allow/deny for requested policy

@aeneasr
Copy link
Member

aeneasr commented Jun 7, 2018

Yeah, looks like a bug! Would be great to have a breaking test case for this!

@aeneasr
Copy link
Member

aeneasr commented Jun 7, 2018

Test fail because the commit to coverall fails, we can ignore the status of goveralls -service=circle-ci -coverprofile=coverage.txt -repotoken=$COVERALLS_REPO_TOKEN for this test.

@alex-vento
Copy link
Author

It breaks the tutorial last step and all core functionality in this library. By the way, we now just start to RND this tool and wanna try to use it in our infrastructure systems, thanks :)

@aeneasr
Copy link
Member

aeneasr commented Jun 7, 2018

Hey, that's great - the PR will definitely be merged. A failing test case will ensure that:

  1. The functionality is working as intended
  2. This bug never happens again

If you need help with writing the test, let me know!

@alex-vento
Copy link
Author

@arekkas yes, i need help, because i just start to learn and understand GO, my profile is python
Have you any telegram account?

@aeneasr
Copy link
Member

aeneasr commented Jun 7, 2018

Ok, no problem. No telegram account, we'll use GitHub. I'll point you in the right directions tomorrow.

@aeneasr aeneasr mentioned this pull request Jun 10, 2018
aeneasr pushed a commit that referenced this pull request Jun 11, 2018
@aeneasr aeneasr closed this in #22 Jun 11, 2018
aeneasr pushed a commit that referenced this pull request Jun 11, 2018
@aeneasr
Copy link
Member

aeneasr commented Jun 11, 2018

Thank you for your contribution, I had a bit of spare time and quickly wrote a failing case for this and incorporated your PR.

Cheers :)

@alex-vento
Copy link
Author

Thanks, i'll be waiting for new release 👍

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.

2 participants