Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix JWT OIDC decode yet again #3466

Merged
merged 1 commit into from
Jun 17, 2023

Conversation

dbutenhof
Copy link
Member

PBENCH-1182

JWT added yet another DecodeError in our API Key validation path in some recent update. Instead of continuing to add specific errors to be converted into a specific internal exception, just handle all exceptions in the primary OIDC decode by attempting the token as an API key. If that fails, report the API key validation error, and re-raise the original OIDC error.

(Note that verify_auth will in turn just report this error and act as if no authentication token was given.)

PBENCH-1182

JWT added yet another `DecodeError` in our API Key validation path in some
recent update. Instead of continuing to add specific errors to be converted
into a specific internal exception, just handle all exceptions in the primary
OIDC decode by attempting the token as an API key. If that fails, report the
API key validation error, and re-raise the original OIDC error.

(Note that `verify_auth` will in turn just report this error and act as if no
authentication token was given.)
@dbutenhof dbutenhof added Server API Of and relating to application programming interfaces to services and functions Users Of and relating to working with users. labels Jun 15, 2023
@dbutenhof dbutenhof self-assigned this Jun 15, 2023
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good, as long as the logging is going to work as needed. (I'm sad to see the API Key validation call back inside the except block for the OIDC access token validation, but I concede that it's expedient.)

lib/pbench/server/auth/auth.py Show resolved Hide resolved
Copy link
Member Author

@dbutenhof dbutenhof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'm sad to see the API Key validation call back inside the except block for the OIDC access token validation, but I concede that it's expedient.)

I know; and I considered storing the exception in a local and retaining more or less the old structure; but I decided that wasn't worth it. (And would have been less clear when scanning the code anyway.)

lib/pbench/server/auth/auth.py Show resolved Hide resolved
@webbnh
Copy link
Member

webbnh commented Jun 16, 2023

I considered storing the exception in a local and retaining more or less the old structure; but I decided that wasn't worth it. (And would have been less clear when scanning the code anyway.)

Thank you for that. I figured that you'd reached that conclusion, and I don't disagree with it.

@dbutenhof dbutenhof merged commit 8561ce6 into distributed-system-analysis:main Jun 17, 2023
@dbutenhof dbutenhof deleted the token branch June 17, 2023 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Of and relating to application programming interfaces to services and functions Server Users Of and relating to working with users.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants