-
Notifications
You must be signed in to change notification settings - Fork 30
Throw if using secret key #465
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
base: master
Are you sure you want to change the base?
Throw if using secret key #465
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds validation to prevent the use of secret keys during Radar SDK initialization. The change ensures that only publishable keys (containing "pk") are accepted, while secret keys (containing "sk") will throw an exception to maintain security best practices.
- Adds secret key validation logic to throw
NSInvalidArgumentException
when secret keys are used - Implements comprehensive test coverage for the new validation behavior
- Maintains existing functionality for valid publishable keys
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
RadarSDK/Radar.m | Adds secret key detection and exception throwing logic in the initialization method |
RadarSDKTests/RadarSDKTests.m | Adds three test cases to verify exception behavior and valid key acceptance |
} | ||
} | ||
|
||
- (void)test_Radar_initialize_acceptsValidPublishableKey { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this case is covered by test_Radar_initialize
, no?
} | ||
|
||
- (void)test_Radar_initialize_throwsCorrectExceptionTypeForSecretKey { | ||
NSString *secretKey = @"prj_test_sk_0000000000000000000000000000000000000000"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and this test would cover test_Radar_initialize_throwsExceptionForSecretKey
I think it would cover all cases with just this test that verify it throws and throws the right error. But it's probably fine to have more tests.
No description provided.