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 Delegated Credentials Issues #99

Merged
merged 2 commits into from
Feb 26, 2016
Merged

Conversation

DirectXMan12
Copy link
Member

This PR contains a fix for #95 (exposing delegated_creds as a property).

It also contains a partial fix for #96 (setting delegated_creds to None if no delegated credentials are returned by the low-level API, and only wrapping the result in a high-level Credentials API if delegated credentials are actually returned), which should have while we debate a longer-term solution.

Previously, in the high-level API, we would unconditionally set
wrap the return value of delegated_creds, which could result in
delegated_creds being set to the default credentials when no
delegated creds were returned (since the call would be
`Credentials(None)`).  This changes the logic so that delegated_creds
stays set as `None` unless delegated_creds are actually returned.

Partial solution for #96
This commit exposes `SecurityContext#delegated_creds` as a documented
property.  Previously, it existed as a field, but was undocumented.

Fixes #95
@DirectXMan12
Copy link
Member Author

cc @Simo @frozencemetery

I'm thinking we can cut 1.1.5 after b8c76a0, and cut 1.2.0 after 45c9ee1 (since exposing a previously undocumented field would probably count as a backwards-compatible feature introduction)

@frozencemetery
Copy link
Member

Okay by me to merge once it's past travis.

@DirectXMan12 DirectXMan12 merged commit 45c9ee1 into master Feb 26, 2016
@DirectXMan12 DirectXMan12 deleted the bug/delegated-creds branch February 26, 2016 16:23
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.

3 participants