cyberark / conjur-api-python

Python client for the CyberArk Conjur API
Apache License 2.0
8 stars 4 forks source link

Add support for a generic JWT authentication #19

Closed AlonBenHorin closed 1 year ago

AlonBenHorin commented 2 years ago

Desired Outcome

Implemented Changes

Connected Issue/Story

CyberArk internal issue links: ONYX-18050, ONYX-23423, ONYX-23294

Changelog

Test coverage

Documentation

Behavior

Security

AlonBenHorin commented 2 years ago

@szh I handled the linting issues. Regarding the squash, when merging into main I'll squash the entire PR into one commit. Regarding the manual test, I created a manual test that validates given a JWT a successful authentication (and also test that it fails if suppling it after the expiration date). I didn't include this test in the commit as the JWT changes over time and it's needed to be retrieved from idaptive (out of the scope of this task)

AlonBenHorin commented 2 years ago

After a discussion with @nahumcohen and @mbenita-Cyberark we have decided that it would be better to reduce the dependency of the API in the CLI, so instead of the API to assume that the API_TOKEN is passed in the credentials_provider, the CLI would set it using a general set_api_token api.

jvanderhoof commented 2 years ago

@AlonBenHorin, although it might come across as strange, I'm going to recommend we use the JWT authenticator instead of the OIDC Authenticator. The Conjur OIDC authenticator doesn't currently support an actual OIDC workflow. It's just a glorified JWT authenticator.

We've begun work to update the OIDC Authenticator to support the full OIDC flow, which means the authenticator will accept the OIDC code returned from the OIDC login redirect and perform the backchannel validation (including validating the returned JWT).

Once we have full support for the OIDC authentication workflow, your current implementation here will need to be completely rethought. Instead, if we use the JWT authenticator, we can support both full and partial OIDC authentication flows in the future.

AlonBenHorin commented 2 years ago

@jvanderhoof, Even though we started this PR thinking we're going to implement an OIDC authentication, we realized that the better approach is using a generic JWT authenticator and so the implementation in this PR is of JWT. PS. the title was misleading so i changed it.

szh commented 2 years ago

I'm going to mark this PR as draft for now to limit the amount of notifications. Please mark it as Ready for Review when you're ready for the C&I team to review and merge it.