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

Provide Credentials.valid property to inspect validity without catching exceptions #80

Open
msimacek opened this issue Aug 21, 2015 · 12 comments

Comments

@msimacek
Copy link

When determinig whether given credentials are valid, I currently need to inquire them and catch an exception. Like this:

try:
    cred.lifetime
    # valid
except ExpiredCredentialsError:
    # invalid

It would be nice if the Credentials object itself had a valid property that would provide this information.

@frozencemetery
Copy link
Member

I don't understand the use case here. There is already a handle to the object that you can check for this information. Why does it need to be a method specifically (it's already an @property method)?

@msimacek
Copy link
Author

What do you mean by handle? What do you suggest to do to check whether given credentials are valid? Maybe I'm just missing something. I didn't say it has to be a method, I said property in this issue. What I need to do is 1. acquire credentials 2. check whether they're valid. Actually, it would be sufficient if the Credentials constructor raised ExpiredCredentialsError when the acquired creds are expired (which doesn't always seem to be the case)

@frozencemetery
Copy link
Member

Michael Šimáček notifications@github.com writes:

What do you mean by handle? What do you suggest to do to check whether
given credentials are valid?

Didn't you just post lines that check if a credential is valid?

Maybe I'm just missing something. I didn't say it has to be a method,
I said property in this issue.

Why does property vs. method make a difference here? Either way it's a
call to the underlying GSSAPI layer.

What I need to do is 1. acquire credentials 2. check whether they're
valid. Actually, it would be sufficient if the Credentials constructor
raised ExpiredCredentialsError when the acquired creds are expired
(which doesn't always seem to be the case)

I'm confused here... are you asking for a helper method that does what
those four lines you posted do?

@simo5
Copy link
Contributor

simo5 commented Aug 21, 2015

On Fri, 2015-08-21 at 11:26 -0700, Robbie Harwood wrote:

Michael Šimáček notifications@github.com writes:

What do you mean by handle? What do you suggest to do to check whether
given credentials are valid?

Didn't you just post lines that check if a credential is valid?

Maybe I'm just missing something. I didn't say it has to be a method,
I said property in this issue.

Why does property vs. method make a difference here? Either way it's a
call to the underlying GSSAPI layer.

What I need to do is 1. acquire credentials 2. check whether they're
valid. Actually, it would be sufficient if the Credentials constructor
raised ExpiredCredentialsError when the acquired creds are expired
(which doesn't always seem to be the case)

I'm confused here... are you asking for a helper method that does what
those four lines you posted do?

One line beats 4 :-)

If it is a common thing we should probably provide a property that does
it for users in a high level API.

Simo.

Simo Sorce * Red Hat, Inc * New York

@frozencemetery frozencemetery added this to the 1.2.0 milestone Aug 21, 2015
@msimacek
Copy link
Author

The problem is not that it's four lines. The problem is that the second line is unreadable - it looks like an expression with no effect that relies on implementation detail. It looks confusing to both people and static analyzers, such as pylint. As I mentioned, maybe it would be better to just do the check for expiration in Credentials.init. Or does acquiring expired credentials have any use-case?

@simo5
Copy link
Contributor

simo5 commented Aug 21, 2015

You may be acquiring credentials just to know who the principal is and it is not hugely important if credentials are expired or not because you may then go on and use gss_acquire_cred_with_password by using the expired credentials name as the name of the creds you want to acquire with the password.

@simo5
Copy link
Contributor

simo5 commented Aug 21, 2015

Perhaps we should not throw exception when inquiring though, and just return lifetime of 0.
We should throw the exception if the user tries to use expired credentials though.

@msimacek
Copy link
Author

That would be actually even better. It's more flexible and more intuitive for new users of the library - people generally don't expect properties to have visible side-effects. So unless backwards compatibility is an issue, I would prefer this to the helper property/method solution.

@DirectXMan12
Copy link
Member

As I mentioned, maybe it would be better to just do the check for expiration in Credentials.__init__.

According to RFC 2744, this can actually happen (check out http://pythonhosted.org/gssapi/gssapi.html#gssapi.creds.Credentials). Can you confirm that you're successfully acquiring expired credentials, and then checking the lifetime throws an exception?

the problem is that the second line is unreadable - it looks like an expression with no effect that relies on implementation detail

agreed

So unless backwards compatibility is an issue, I would prefer this to the helper property/method solution.

Backwards compatibility is an issue -- if we did something like this, it would have to be part of v2.0.0, and not the v1.y.z series.

@msimacek
Copy link
Author

Can you confirm that you're successfully acquiring expired credentials, and then checking the lifetime throws an exception?

export KRB5CCNAME=FILE:/tmp/ccache
kinit -l 1
sleep 1 # let it expire
python -c 'import gssapi as g;g.Credentials().lifetime' # this fails with ExpiredCredentialsError
python -c 'import gssapi as g;g.Credentials()' # but this doesn't

@DirectXMan12
Copy link
Member

hmm... I would not be terribly opposed to adding a Credentials#expired property However, I'd like to know the context of this request -- is there something preventing you from doing this?

creds = gssapi.Credentials()
try:
    use_my_creds(creds)
except gssapi.exceptions.ExpiredCredentialsError:
    print('Ah man, your creds are expired!')

Unless it shells out to krb5 or tries to use acquire_cred_with_password, there's not much recourse for a GSSAPI application if the credentials are expired...

@msimacek
Copy link
Author

The use case is when python-gssapi is only used to acquire/query the credentials, but they are then used by something else - another library or another application that uses the ccache.

@DirectXMan12 DirectXMan12 modified the milestones: 1.3.0, 1.2.0 Mar 1, 2016
@frozencemetery frozencemetery modified the milestones: 1.3.0, 1.4.0 Dec 4, 2017
@frozencemetery frozencemetery modified the milestones: 1.4.0, 1.5.0 Feb 16, 2018
@frozencemetery frozencemetery removed this from the 1.5.0 milestone Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants