update google_auth.py - Not an RSA key (#3388)

[AAGRusso]

There is a bug when the IDP server sends a non-RSA key in the openid-configuration. When this key is encountered, it returns the error "Not a RSA key", and the login fails.

This PR fixes the issue by properly handling non-RSA keys in the JWKS validation process, allowing the authentication flow to continue smoothly with supported key types.

Fixes #3388

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[jaegeral]

Hey @AAGRusso could you have a look at https://cla.developers.google.com/clas and check your CLA status? Without it unfortunately we are not able to accept a contribution.

Cheers

[jaegeral]

/gemini review

[gemini-code-assist]

Code Review

The pull request addresses an important bug where the application fails to handle non-RSA keys in the JWKS validation process. The fix involves adding a try-except block to catch exceptions when processing keys, which allows the authentication flow to continue smoothly with supported key types. Overall, the change seems reasonable, but I have a few suggestions to improve the error handling and logging.

Summary of Findings

• Error Logging: The current implementation uses print for error logging, which is not suitable for production environments. It should be replaced with the logging module to provide better control over log levels and destinations.

Merge Readiness

The pull request addresses a critical bug and the fix seems reasonable. However, the error logging should be improved before merging. I recommend replacing the print statement with the logging module. I am unable to approve this pull request, and recommend that others review and approve this code before merging.

[gemini-code-assist]

The print statement used for logging the error is not ideal for production code. It would be better to use the logging module to log the error with appropriate severity (e.g., logging.error). This allows for better control over log levels and destinations.¹

Consider including the exception type in the log message for better debugging.²

Style Guide References

Suggested change

	except Exception as e:
	print(f"[ERROR] Error processing key ID {key_dict['kid']}: {e}")
	import logging
	try:
	public_key = jwt.algorithms.RSAAlgorithm.from_jwk(json.dumps(key_dict))
	_new_keys_dict[key_dict["kid"]] = public_key
	except Exception as e:
	logging.error(f"Error processing key ID {key_dict['kid']}: {type(e).__name__} - {e}")

Footnotes

1. Use the logging module for logging messages. This includes setting the appropriate log level (e.g., INFO, WARNING, ERROR) and formatting the log messages consistently. (link) ↩

2. Use exceptions for error handling and provide informative error messages. (link) ↩

[AAGRusso]

Hi, I already signed Need to do something more? El vie, 16 may 2025, 13:25, Alexander J ***@***.***> escribió:

…

*jaegeral* left a comment (google/timesketch#3408) <#3408 (comment)> /gemini review — Reply to this email directly, view it on GitHub <#3408 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASG7KL2DDTG4BB45FXTEBD26XDJHAVCNFSM6AAAAAB5BINHASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQOBWGQ2DMMRXGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[gemini-code-assist]

Hi @AAGRusso, thanks for letting us know that you've signed the CLA.

The /gemini review command was invoked, but the automated checks, including the CLA check, need to pass before the review can proceed. Once the CLA status is updated and the checks are green, the review will run.

Let us know if you have any other questions!

[jaegeral]

Ah it states your contribution in that PR: e536a6c is with a different mail. I will check

[jaegeral]

Hi, I already signed Need to do something more? El vie, 16 may 2025, 13:25, Alexander J @.> escribió:
…
jaegeral left a comment (google/timesketch#3408) <#3408 (comment)> /gemini review — Reply to this email directly, view it on GitHub <#3408 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASG7KL2DDTG4BB45FXTEBD26XDJHAVCNFSM6AAAAAB5BINHASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQOBWGQ2DMMRXGI . You are receiving this because you were mentioned.Message ID: @.>

jm*****@la****.e*can you confirm that this is you?

 

[gemini-code-assist]

Thanks for the update, @jaegeral. I see the discussion regarding the CLA status and the email address associated with the commit.

As mentioned previously, the /gemini review command is pending the successful completion of the automated checks, including the CLA check. Once the CLA issue is resolved and the checks are green, I will be able to proceed with the code review.

Let me know if there's anything else I can assist with once the checks pass!

[AAGRusso]

Yes, sorry, its my company mail. El vie, 16 may 2025, 14:42, Alexander J ***@***.***> escribió:

…

*jaegeral* left a comment (google/timesketch#3408) <#3408 (comment)> Hi, I already signed Need to do something more? El vie, 16 may 2025, 13:25, Alexander J *@*. *> escribió: … <#m_-7146449561852793424_> jaegeral left a comment (google/timesketch#3408 <#3408>) <#3408 (comment) <#3408 (comment)>> /gemini review — Reply to this email directly, view it on GitHub <#3408 (comment) <#3408 (comment)>>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASG7KL2DDTG4BB45FXTEBD26XDJHAVCNFSM6AAAAAB5BINHASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQOBWGQ2DMMRXGI <https://github.com/notifications/unsubscribe-auth/AASG7KL2DDTG4BB45FXTEBD26XDJHAVCNFSM6AAAAAB5BINHASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQOBWGQ2DMMRXGI> . You are receiving this because you were mentioned.Message ID: @.*> ***@***.*******.e*can you confirm that this is you? — Reply to this email directly, view it on GitHub <#3408 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASG7KLTHZ6ZOJWCLYK4BVD26XMNHAVCNFSM6AAAAAB5BINHASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQOBWGYYTKNRRGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[gemini-code-assist]

Thanks for confirming the email address, @AAGRusso. It sounds like the team is working with you to get the CLA sorted out based on that information.

As I mentioned before, the /gemini review command is waiting for the automated checks, including the CLA check, to pass. Once that's resolved and the checks turn green, I'll be able to proceed with the code review.

Please continue working with the maintainers to get the CLA issue resolved. Let me know if you have any questions once the checks are passing!

[AAGRusso]

Hello, I added jm****@LA***.s as primary address to github account.

[jaegeral]

So from what I understand, the only thing added is a try catch with a very broad catch, not sure how that fixes an error.

Please correct the print() to be a logging statement, and I would like to see a unit test for the case you explain if possible?

And is there a possibility to make the broad except more specific?

[jaegeral]

@AAGRusso friendly nag