Closed 0nebody closed 4 years ago
This is a non-trivial change, and I want to say thanks right off the bat.
You'll understand if it takes me some time to getting around to reviewing it, right? I can see getting this in on Friday (2020-06-26) or next week Monday (2020-06-29), but probably not before. I'll start getting you some unit tests before then, though.
Hey, I'm sorry I haven't gotten to this yet. I'm still getting the IdP set up to create the SAML Responses so I can give you better tests.
Merging #53 into master will decrease coverage by
1.73%
. The diff coverage is67.85%
.
@@ Coverage Diff @@
## master #53 +/- ##
==========================================
- Coverage 91.09% 89.36% -1.74%
==========================================
Files 2 2
Lines 348 376 +28
==========================================
+ Hits 317 336 +19
- Misses 31 40 +9
Impacted Files | Coverage Δ | |
---|---|---|
samlauthenticator/samlauthenticator.py | 89.33% <67.85%> (-1.74%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update cd97085...150dfa9. Read the comment docs.
Cool, thanks for getting to my comments so quickly! If you want to merge my PR into your branch here, I think that we should be able to get this merged by the end of the week.
Wait - my changes don’t account for your updates based on the review comments. I’ll make some changes and get back to you.
Ok, I should have fixed my tests at this point. If you want to squash-and-merge my changes into your branch, I think that I would be more than happy to get these changes into the project.
First pass at adding access restriction by roles / groups that are returned in the SAML response.
Signed-off-by: Mitchel Haring mitchel.haring@gmail.com