Repository navigation
fix: return verified boolean in verify_signature, prevent timing attacks, and ensure thread-safe headers - #108
Open
cobanfurkanx wants to merge 1 commit into
Conversation
…cks, and ensure thread-safe headers - Fix verify_signature to return verified boolean rather than None, enabling callers to properly check payment notification validity. - Use hmac.compare_digest for constant-time comparison in verify_signature to guard against timing attacks. - Make calculate_hmac_sha256_signature resilient against TypeErrors when params contain numbers or None. - Prevent multi-threading race conditions in concurrent web frameworks (FastAPI/Django) by returning per-request copies rather than mutating class-level header dict in-place. - Sanitize base_url host in connect() to handle both scheme-less and https:// prefixes. - Add comprehensive unit test suite in tests/ covering signature verification, thread-safety, and helpers. - Update GitHub Actions workflow to run automated unit tests across Python 3.9-3.13 matrix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses several critical reliability, security, and concurrency issues in
IyzipayResource:verify_signaturereturnedNone: The method computedverified = signature == calculated_signatureand printed it to stdout, but never returned the boolean, causing any caller performingif not payment.verify_signature(...)to always fail even on valid signatures.hmac.compare_digestto protect against HMAC timing side-channel attacks.TypeErrorincalculate_hmac_sha256_signature: Whenparamscontained numbers (paidPrice,price,installment) orNone(conversationId),':'.join(params)raisedTypeError: sequence item X: expected str instance. It now safely converts values to string or empty string.header(Addresses Bugs I experienced & Concept (FastAPI, MongoDB, React) #91):IyzipayResource.headerwas a shared class variable mutated in-place during each request (self.header.update({'x-iyzi-rnd': ...})). In concurrent environments (e.g. FastAPI / Django / Flask with threaded workers), concurrent requests could overwrite headers mid-flight, causingGeçersiz imzaauthentication failures. Headers are now copied per-request.base_urlHost Sanitization: Gracefully handles both scheme-less hostnames (sandbox-api.iyzipay.com) and standard URLs (https://sandbox-api.iyzipay.com), preventingInvalidURL: nonnumeric porterrors.tests/and updated.github/workflows/github_pull_request.ymlto run automated test discovery across the matrix (Python 3.9–3.13).Verification
All 14 unit tests pass cleanly:
$ python -m unittest discover -s tests -v Ran 14 tests in 0.028s OK