Skip to content

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

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.

Sign up for GitHub

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

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

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e536a6c
update google_auth.py - Not an RSA key (#3388)
AAGRusso May 13, 2025
ccfbe1e
Merge branch 'master' into master
AAGRusso May 14, 2025
ede79e5
Merge branch 'master' into master
jaegeral May 16, 2025
f872cde
Merge branch 'master' into master
jaegeral May 16, 2025
1882ee2
Merge branch 'master' into master
AAGRusso May 16, 2025
18a9a79
Merge branch 'master' into master
AAGRusso May 19, 2025
cf3550b
Merge branch 'master' into master
jaegeral May 20, 2025
72c5ae2
Merge branch 'master' into master
jaegeral Jun 3, 2025
ae7896c
Merge branch 'master' into master
jaegeral Aug 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions timesketch/lib/google_auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,11 @@ def get_public_key_for_jwt(encoded_jwt: str, url: str):
if "keys" in keys_json:
_new_keys_dict = {}
for key_dict in keys_json["keys"]:
public_key = jwt.algorithms.RSAAlgorithm.from_jwk(json.dumps(key_dict))
_new_keys_dict[key_dict["kid"]] = public_key
try:
Copy link
Collaborator

@jaegeral jaegeral May 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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?

public_key = jwt.algorithms.RSAAlgorithm.from_jwk(json.dumps(key_dict))
_new_keys_dict[key_dict["kid"]] = public_key
except Exception as e:
print(f"[ERROR] Error processing key ID {key_dict['kid']}: {e}")
Comment on lines +316 to +317
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

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.1

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

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) ↩

key_cache = _new_keys_dict
else:
key_cache = keys_json
Expand Down
Loading