Add backend for Hashicorp Vault

[candlerb]

Proposed changes

This is a new backend for the OIDC Provider in Hashicorp Vault 1.9+

Types of changes

Please check the type of change your PR introduces:

• Release (new release request)
• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Code style update (PEP8, lint, formatting, renaming, etc)
• Refactoring (no functional changes, no api changes)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Build related changes (build process, tests runner, etc)
• Other (please describe):

Checklist

Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you're unsure about any of them, don't hesitate to ask. We're here to
help! This is simply a reminder of what we are going to look for before merging
your code.

• Lint and unit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works

(Not sure what linting you want, but flake8 passes)

Other information

Tested with Netbox

NOTE 1: I had to override the base OpenIdConnectAuth class to do HTTP Basic Auth when talking to the token endpoint. Maybe it would be better if functionality were moved into the base class itself, since it can make use of the information exposed in the OIDC discovery response:

      'token_endpoint_auth_methods_supported': [ 'client_secret_basic' ],

NOTE 2: I turned OIDC_ENDPOINT into a property, since this is something the user has to configure to point to their own Vault server. Maybe it would be better to turn this into a 'setting' on the OpenIdConnectAuth base class.

NOTE 3: With those two changes, I would have been able to instantiate the OpenIdConnectAuth class directly - except that it would still need to have a 'name' property. If you set 'name' to 'oidc' here, then it would be usable as a generic OpenID Connect backend. (I made one other change to pick up the 'groups' claim, which is non-standard but commonly implemented)

[candlerb]

There is a weird interaction between tests.

pytest test_vault.py and pytest test_open_id_connect.py both pass when run separately. But if I run them together, one test fails:

$ pytest test_open_id_connect.py test_vault.py
============================================================ test session starts =============================================================
platform darwin -- Python 3.10.2, pytest-7.0.1, pluggy-1.0.0
rootdir: /Users/brian/git/social-core
plugins: cov-3.0.0
collected 16 items

test_open_id_connect.py ........                                                                                                       [ 50%]
test_vault.py F.......                                                                                                                 [100%]

================================================================== FAILURES ==================================================================
________________________________________________ VaultOpenIdConnectTest.test_everything_works ________________________________________________

self = <social_core.tests.backends.test_vault.VaultOpenIdConnectTest testMethod=test_everything_works>

    def test_everything_works(self):
>       self.do_login()

test_vault.py:41:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
base.py:63: in do_login
    user = self.do_start()
oauth.py:86: in do_start
    return self.backend.complete()
../../backends/base.py:40: in complete
    return self.auth_complete(*args, **kwargs)
../../utils.py:247: in wrapper
    return func(*args, **kwargs)
../../backends/oauth.py:382: in auth_complete
    response = self.request_access_token(
../../backends/open_id_connect.py:212: in request_access_token
    self.id_token = self.validate_and_return_id_token(
../../backends/open_id_connect.py:202: in validate_and_return_id_token
    self.validate_claims(claims)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <social_core.backends.vault.VaultOpenIdConnect object at 0x1116db400>
id_token = {'at_hash': 'w6uP8Tcg6K2QR905Rms8iQ', 'aud': 'a-key', 'azp': 'a-key', 'exp': 1645179196, ...}

    def validate_claims(self, id_token):
        utc_timestamp = timegm(datetime.datetime.utcnow().utctimetuple())

        if 'nbf' in id_token and utc_timestamp < id_token['nbf']:
            raise AuthTokenError(self, 'Incorrect id_token: nbf')

        # Verify the token was issued in the last 10 minutes
        iat_leeway = self.setting('ID_TOKEN_MAX_AGE', self.ID_TOKEN_MAX_AGE)
        if utc_timestamp > id_token['iat'] + iat_leeway:
            raise AuthTokenError(self, 'Incorrect id_token: iat')

        # Validate the nonce to ensure the request was not modified
        nonce = id_token.get('nonce')
        if not nonce:
            raise AuthTokenError(self, 'Incorrect id_token: nonce')

        nonce_obj = self.get_nonce(nonce)
        if nonce_obj:
            self.remove_nonce(nonce_obj.id)
        else:
>           raise AuthTokenError(self, 'Incorrect id_token: nonce')
E           social_core.exceptions.AuthTokenError: Token error: Incorrect id_token: nonce

../../backends/open_id_connect.py:143: AuthTokenError
========================================================== short test summary info ===========================================================
FAILED test_vault.py::VaultOpenIdConnectTest::test_everything_works - social_core.exceptions.AuthTokenError: Token error: Incorrect id_toke...
======================================================== 1 failed, 15 passed in 8.24s ========================================================

There is some global state that I can't find - any ideas?

[candlerb]

I found it. The problem is in test_open_id_connect.py

class OpenIdConnectTestMixin:
    ...
    access_token_kwargs = {}

This class-level object is shared between all objects which inherit OpenIdConnectTestMixin. And later:

    def pre_complete_callback(self, start_url):
        ...
        self.access_token_kwargs.setdefault('nonce', nonce)

If the access token has 'nonce' set by a previous test, then it doesn't get updated :-(

I could just make it set nonce unconditionally, and that makes the tests pass, but I don't want to change it without understanding why setdefault() was used here in the first place.

[candlerb]

I've made a more conservative change (I think) which creates a new dict in each subclass. Could you re-run the tests please?

[codecov]

Codecov Report

Merging #668 (b7b2041) into master (36f25c4) will increase coverage by 0.08%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #668      +/-   ##
==========================================
+ Coverage   76.95%   77.03%   +0.08%     
==========================================
  Files         317      319       +2     
  Lines        9647     9682      +35     
  Branches     1034     1036       +2     
==========================================
+ Hits         7424     7459      +35     
  Misses       2071     2071              
  Partials      152      152              

Flag	Coverage Δ
unittests	77.03% <100.00%> (+0.08%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
social_core/backends/vault.py	100.00% <100.00%> (ø)
social_core/tests/backends/test_open_id_connect.py	98.95% <100.00%> (+0.02%)	⬆️
social_core/tests/backends/test_vault.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 36f25c4...b7b2041. Read the comment docs.

[candlerb]

Thank you: I see that tests pass now.

I have also submitted an alternative, but somewhat more invasive PR, as #669. That PR implements changes to the base OpenIdConnectAuth class to address "NOTE 1/2/3" mentioned in the top of this PR: it changes OpenIdConnectAuth to work as a "generic" backend which can be configured to point to a standard OIDC server without having to subclass it.

I hope one or other approach of these PRs is acceptable, but if not, please let me know what you'd like to see instead.

[nijel]

I think #669 is a better approach, closing this one.