(addendum: I think the latest commits on master have broken the test suite; at the very least, it no longer works on my rawhide machine. My changes pass prior to rebase, of course.)

Pagure doesn't make it visible what the previous commit was you were using before you rebased. But I am guessing you missed the recent change on master which requires a local Postgres to run the tests. Without seeing what the actual failure is I can't help. But no, the tests are not currently broken.

Also, we have an existing issue about this: #55. I would rather get the Flask-Kerberos library fixed/enhanced as necessary, and switch (back) to using that, so that we don't have to keep carrying and enhancing this code here in our application. We have a basically identical copy of the same thing in Greenwave as well.

flask-kerberos isn't packaged for Fedora, so I'm not sure why that's preferable. It also uses python-kerberos, which makes the problem worse for me by introducing a new dependency on that into the distro. "Fixing" it to use python-gssapi (which probably entails forking it to make flask-gssapi) isn't something I'm thrilled about either, because unless upstream (or someone else) agrees to, it's another thing I have to maintain.

Regarding the tests: are there setup instructions somewhere? I can't find any, and it seems pretty clear that I don't have the database up - everything dies in connect with E OperationalError: (psycopg2.OperationalError) could not connect to server: No such file or directory

There are instructions for setting up Postgres in README.md. It's nothing special, the tests just want to create/drop databases so they assume superuser on a local Postgres.

https://pagure.io/waiverdb/c/3e56879bbc781f776af3a4e138401f803e20df08

There is also a dev guide in the docs, although I just noticed that it has fallen out of sync with the README. I will fix that.

The reason to prefer Flask-Kerberos is so that this code for handling Kerberos authentication only lives in one place, we wouldn't have to maintain it here and in Greenwave and in the various other projects our team has. And so that changes like this, porting from one set of Kerberos bindings to another, which has no real bearing on our application itself, can be done in that library instead of in every single application.

Why the big push to get rid of python-kerberos anyway? It is working fine right now. And is actively maintained by Apple with a Python 3 port. Does switching everything to GSSAPI bring any actual benefit? That might be why you are finding upstream projects reluctant to take these invasive patches which rewrite their entire Kerberos handling code for no observable benefit...

Thanks, not sure how I missed the stuff in the readme... anyway, all tests are green.

I haven't been finding reluctance; this is the first project that's given pushback about it :) Most seem to be enthusiastic because the interface is simpler and more idiomatic (it's actually object oriented!), brings in support for NTLM and other mechanisms without additional effort, and isn't beholden to Apple's bizarre notions of releasing (python-kerberos is ripped out of their calendar server tree).

python-kerberos, python-krbV, and python-requests-krb5 were marked deprecated in RHEL-7: https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7-beta/html/7.5_release_notes/chap-red_hat_enterprise_linux-7.5_release_notes-deprecated_functionality and therefore "will likely not be supported in future major releases of this product and is not recommended for new deployments".

The "reluctance" I meant was with requests-kerberos and flask-kerberos.

I personally find neither the old code with python-kerberos nor the new code python-gssapi to be simple or easy to understand... and unfortunately we have no easy way to test this against a real KDC either. But anyway, if you say it works, then sure. :+1:

Sorry, this is conflicting because we just merged some changes for switching to Python 3. I think the conflict will just be around dependencies in the .spec. Would you mind rebasing again?

If you want tests against a real KDC, we have a library for that: https://github.com/pythongssapi/k5test (it takes a couple seconds at most to set up a test KDC for testing)

Anyway, rebased.

Thanks for the pointer about k5test, it looks very useful.

Flake8 warns that this variable is never used... Is it a bug?

There are a bunch of other miscellaneous flake8 complaints about unused imports etc too.

@rharwood, ping?

Fwiw, just replace it with gssapi.Name("HTTP@%s" % hostname, gssapi.NameType.hostbased_service)

Fixed imports.

A brief aside on the name: GSSAPI automatically uses an appropriate name from the keytab/ccache. However, it looks from the code like there's desire to run this in an environment where there might be multiple entries in the keytab (KERBEROS_HTTP_HOST), so I've made the override work. If this isn't desired, then that code can go away.

Thanks for the update @rharwood.

I think we added the KERBEROS_HTTP_HOST setting because we anticipated deploying on a VM with a hostname like waiverdb.host.prod.eng.bos.redhat.com but users would access the app over a CNAME like waiverdb.engineering.redhat.com. But it turns out now we are deploying in Openshift instead, and all that is defeated on the client side by dns_canonicalize_hostname anyway.

And as you point out, with GSSAPI we don't need to actually know the hostname in advance, now we can just fill in all the possible hostnames in the keytab and let the library pick the right one. So we could probably just drop the KERBEROS_HTTP_HOST setting... But since we have it now, and we obey it, I guess we should keep it as is. We can revisit later down the track.

The unit tests are failing for me in TestGSSAPIAuthentication.test_authorized though...

Traceback (most recent call last):
  File "/home/dcallagh/work/waiverdb/waiverdb/auth.py", line 24, in process_gssapi_request
    creds = gssapi.Credentials(name=service_name, usage="accept")
  File "/usr/lib64/python3.6/site-packages/gssapi/creds.py", line 64, in __new__
    store=store)
  File "/usr/lib64/python3.6/site-packages/gssapi/creds.py", line 137, in acquire
    mechs, usage)
  File "gssapi/raw/creds.pyx", line 158, in gssapi.raw.creds.acquire_cred
gssapi.raw.misc.GSSError: Major (851968): Unspecified GSS failure.  Minor code may provide more information, Minor (2): Key table file '/etc/foo.keytab' not found
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/home/dcallagh/work/waiverdb/tests/test_auth.py", line 37, in test_authorized
    content_type='application/json', headers=headers)
[...]
  File "/home/dcallagh/work/waiverdb/waiverdb/api_v1.py", line 212, in post
    user, headers = waiverdb.auth.get_user(request)
  File "/home/dcallagh/work/waiverdb/waiverdb/auth.py", line 75, in get_user
    user, token = process_gssapi_request(base64.b64decode(token))
  File "/home/dcallagh/work/waiverdb/waiverdb/auth.py", line 46, in process_gssapi_request
    (stage, e.gen_msg()))
AttributeError: 'GSSError' object has no attribute 'gen_msg'

The gen_msg bit appears to be just a typo, should be gen_message. So I'm kind of glad we hit that exception handler. :-)

The actual failure seems to be just because it's missing a mock for gssapi.Credentials.

Thanks, not sure why I wasn't hitting that. (Also rebased.)

Thanks, LGTM