Add TicketFlags class and Creds.ticket_flags attribute

[steffen-kiess]

Note that this uses the heimdal definition of the ticket flags (where flag i is represented as 1 << i) instead of the MIT one (where flag i is represented as 1 << (31 - i)) because this seems to make more sense to me. For MIT the values are converted when reading Creds.ticket_flags.

[jborean93]

I'm a bit torn here, while this library does paper over some API differences that's more around some minor API/struct changes to expose the same data through a Python interface. This case is actually changing the results we get back to some more common standard. Without thinking about it too hard I'm probably against the idea for the following reasons

• We are trying to provide an interface into the C API to call from Python rather than a high level API
• People who are used to the API might in fact be more familiar with the MIT result vs Heimdal. The same argument can be used if the behaviour was the opposite

I'm wondering whether we should keep ticket_flags (or rename to ticket_flags_raw or some variant) and have another property or method that can give the friendly enum variant. What are your thoughts around this?

[steffen-kiess]

The reason I wrote it this way was to allow checking things like "does this ticket have the initial flag set". If only the raw value is provided, this would require the user of the library to hardcode the value of TKT_FLG_INITIAL, which would mean the code would likely break when moving between MIT and Heimdal. In many other places pykrb5 (e.g. PrincipalParseFlags) provides the value from the C library to the user, but in this case

• this would not work for unknown flags, which are more likely in this case, because this is a value which comes from the KDC, not from the user, and
• Heimdal does not even provide TKT_FLG_INITIAL, instead assuming the user uses the bit-fields in struct TicketFlags.

• People who are used to the API might in fact be more familiar with the MIT result vs Heimdal.

I mostly chose the Heimdal version here because it is closer to the RFCs, and because it would in theory allow more than 32 flags. (The on-the-wire ASN1 structure allow more than 32 flags, but both the MIT and Heimdal APIs can return only 32 flags, and changing this would break the ABI.)

But I understand the point that this might be confusing to people. Another thing is that python-gssapi also returns the raw ticket flags (in krb5_get_tkt_flags()), and an application which uses both pykrb5 and python-gssapi might get confused by that.

I'm wondering whether we should keep ticket_flags (or rename to ticket_flags_raw or some variant) and have another property or method that can give the friendly enum variant. What are your thoughts around this?

I think I like the idea.

Should I update the PR to

• provide the raw value from the library as ticket_flags_raw (or ticket_flags)
• add static method from_raw(int) -> TicketFlags and a property raw to TicketFlags
• add a property to Creds which returns TicketFlags.from_raw(self.ticket_flags_raw), called e.g. ticket_flags_enum or ticket_flags or something like that?

[jborean93]

provide the raw value from the library as ticket_flags_raw (or ticket_flags)

I think this might be the easiest option to me, change the existing ticket_flags to ticket_flags_raw that returns the raw integer and have ticket_flags return the enum.IntFlags enum. I'm not seeing much benefit by adding in extra methods to convert and even if there is a demand for it we can always add it later.

I mostly chose the Heimdal version here because it is closer to the RFCs, and because it would in theory allow more than 32 flags.

Makes sense to me, I would prefer that setup as well. I think as long as we document that ticket_flags is a human friendly representation of the raw value and to use ticket_flags_raw if you really want it is the way to go.

[steffen-kiess]

I've updated the PR to do that.

[jborean93]

Thanks for all your fantastic PRs for this library, please let me know if you have any more features you want to add anytime soon and I'll hold off on doing some integration testing for the next release.

[steffen-kiess]

Thanks for all your fantastic PRs for this library

Thanks for the quick review and merging of the changes :-)

please let me know if you have any more features you want to add anytime soon and I'll hold off on doing some integration testing for the next release.

I opened one last PR https://github.com/jborean93/pykrb5/pull/44, but other than that I'm not planning on any new features soon (the last PRs were mostly things I had in my local repository for the last 1-2 years).