Bugfix/IPv6 compatibility for AuthTktAuthenticationPolicy #837
Conversation
Add a note that using the include_ip functionality with IPv6 is not a good idea due to the users network expiring IPv6 addresses quickly. See preferred lifetime/valid lifetime for routers doing SLAAC in IPv6 for more information.
Add an extra test that sets environ['REMOTE_ADDR'] to an IPv6 address "::1".
Add some simple tests that test IPv6 compatibility with the AuthTicket class and the parse_ticket function.
This is a quick fix for adding IPv6 compatibility so that if the AuthTktAuthenticationPolicy is used with include_ip enabled then it won't throw an error.
|
@tshepang thanks for the fixes on the grammar. Fixed those issues, squashed the commits and it looks like Github automatically updated this pull request! |
|
koolnes |
|
So call me slow ... but just realized that auth_tkt is actually a standard of sorts, and not something that was simply added to Pyramid/Paste... Anyway there is currently no support for IPv6 in mod_auth_tkt, and that will itself segfault when the IP isn't an IPv4 address. Some patches have been proposed on the mailing list for mod_auth_tkt, but none have been accepted yet from what I can find. Not sure what the best course of action is to fix the situation. At the moment the IPv4 setup is still compatible with mod_auth_tkt, so that will continue to function, and my patch fixes IPv6 as well, but with the knowledge that most likely it is going to have to be changed in the future to be compatible with mod_auth_tkt. Would like to hear from someone on the Pyramid team what the suggested way to move forward is. @tseaver? @goodwillcoding? |
|
Is it crazy to ignore the |
|
I don't think that simply disabling it is a good idea. If the developer has specifically turned on the feature it should definitely be enabled. Although, as I stated in this patch: https://github.com/bertjwregeer/pyramid/commit/918c9d9dd632d346909d2429647758352d753a42 I don't think it is a good idea because of the way that IPv6 addresses are provided to most users (SLAAC) |
|
@mmerickel since you are looking through old pull requests, any updates on how to proceed with this one? |
|
Figured since I asked you to look at it again, I should do some looking and see if mod_auth_tkt has moved forward, they have not. Here is one patch I found though: Which packs a 0 for the IPv4 address if the address is IPv6 and appends the IPv6 address to the timestamp. If we want to be compatible with that patch, it is a very simple change. Although I haven't seen any movement towards adopting that patch, nor any work on mod_auth_tkt in general... |
|
Thanks! Sorry this took so long to handle. |
I've added tests for IPv6 compatibility in the AuthTktAuthenticationPolicy when
include_ipis set to True. I have also updated the documentation to add an extra note that it is not suggested on IPv6, and fixed the actual issue incalculate_digest()so that if it is called with an IPv6 address it doesn't try to callencode_ip_timestamp().This came about directly because of issue #831 and would effectively close that issue.
If there any style or code issues that I need to fix, please let me know and I will update my brach and submit an updated pull request.