-
Notifications
You must be signed in to change notification settings - Fork 23
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
SDKS-1747 / SDKS-2022 - DeviceBinding/Verifier - Do not merge to develop #184
Conversation
keyCallBack = keyAware.generateKeys(context, it) | ||
} | ||
} | ||
catch (e: Exception) { |
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.
TooGenericExceptionCaught: Caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.
When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
setDeviceName("jan device") | ||
setJws(jws) | ||
Listener.onSuccess(listener, null) | ||
} catch (e: Exception) { |
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.
TooGenericExceptionCaught: Caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.
When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
jsonObject.put("userId", userId) | ||
jsonObject.put("kid", uuid) | ||
} catch (e: JSONException) { | ||
throw RuntimeException(e) |
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.
TooGenericExceptionThrown: RuntimeException is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.
When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
val hash: ByteArray? = digest.digest(value.toByteArray()) | ||
Base64.encodeToString(hash, Base64.DEFAULT) | ||
} catch (e: NoSuchAlgorithmException) { | ||
throw RuntimeException(e) |
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.
TooGenericExceptionThrown: RuntimeException is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.
When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
094e7ca
to
8cfc6b7
Compare
jsonObject.put("userId", userId) | ||
jsonObject.put("kid", uuid) | ||
} catch (e: JSONException) { | ||
throw RuntimeException(e) |
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.
TooGenericExceptionThrown: RuntimeException is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.
When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
4dcfd17
to
079d921
Compare
} catch (e: NoSuchAlgorithmException) { | ||
throw RuntimeException(e) | ||
} | ||
catch (e: Exception) { |
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.
TooGenericExceptionCaught: Caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.
When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
7d76290
to
3720dc3
Compare
54a850d
to
3c38c07
Compare
forgerock-auth/src/main/java/org/forgerock/android/auth/callback/DeviceBindingCallback.kt
Show resolved
Hide resolved
listener: FRListener<Void>, | ||
keyAware: KeyAwareInterface = KeyAware(userId), | ||
encryptedPreference: PreferenceUtil = PreferenceUtil(), | ||
executorService: ScheduledExecutorService? = Executors.newSingleThreadScheduledExecutor(), |
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.
Instead of using ExecutorService, will it be easier if we just use some calculation with currenttime and timeout?
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.
good idea
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.
done, made it even easier
forgerock-auth/src/main/java/org/forgerock/android/auth/callback/DeviceBindingCallback.kt
Outdated
Show resolved
Hide resolved
forgerock-auth/src/main/java/org/forgerock/android/auth/callback/DeviceBindingCallback.kt
Outdated
Show resolved
Hide resolved
forgerock-auth/src/main/java/org/forgerock/android/auth/callback/DeviceBindingCallback.kt
Outdated
Show resolved
Hide resolved
forgerock-auth/src/main/java/org/forgerock/android/auth/devicebind/BiometricUtil.kt
Outdated
Show resolved
Hide resolved
341147f
to
3db7b89
Compare
forgerock-auth/src/main/java/org/forgerock/android/auth/callback/DeviceBindingCallback.kt
Outdated
Show resolved
Hide resolved
forgerock-auth/src/main/java/org/forgerock/android/auth/callback/DeviceBindingCallback.kt
Outdated
Show resolved
Hide resolved
forgerock-auth/src/main/java/org/forgerock/android/auth/callback/DeviceBindingCallback.kt
Outdated
Show resolved
Hide resolved
jsonObject.put("kid", uuid) | ||
jsonObject.put("authType", authenticationType.serializedValue) | ||
} catch (e: JSONException) { | ||
throw RuntimeException(e) |
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.
TooGenericExceptionThrown: RuntimeException is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.
When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
05ecf86
to
6b5b1ac
Compare
forgerock-auth/src/main/java/org/forgerock/android/auth/devicebind/DeviceBindAuthenticators.kt
Outdated
Show resolved
Hide resolved
6b5b1ac
to
729ff99
Compare
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.
Overall everything looks good to me!
My only ask is to please add java docs to public interfaces. Currently, the docs are not consistent - e.g. in some places function parameters include description of the params, and return values, etc... and in others these a missing...
forgerock-auth/src/main/java/org/forgerock/android/auth/DeviceIdentifier.java
Show resolved
Hide resolved
forgerock-auth/src/main/java/org/forgerock/android/auth/DeviceIdentifier.java
Show resolved
Hide resolved
forgerock-auth/src/main/java/org/forgerock/android/auth/callback/AbstractCallback.java
Show resolved
Hide resolved
forgerock-auth/src/main/java/org/forgerock/android/auth/callback/AbstractCallback.java
Show resolved
Hide resolved
forgerock-auth/src/main/java/org/forgerock/android/auth/devicebind/BiometricBindingHandler.kt
Show resolved
Hide resolved
forgerock-auth/src/main/java/org/forgerock/android/auth/devicebind/BiometricBindingHandler.kt
Show resolved
Hide resolved
forgerock-auth/src/main/java/org/forgerock/android/auth/devicebind/BiometricBindingHandler.kt
Show resolved
Hide resolved
729ff99
to
93758f3
Compare
Device Binding / Verifier feature - SDKS-1747, SDKS-2022
} | ||
} | ||
} | ||
catch (e: Exception) { |
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.
TooGenericExceptionCaught: Caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
ℹ️ Learn about @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Command | Usage |
---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
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.
👍🏻
JIRA Ticket
Please link jira ticket here
Description
Briefly describe the change and any information that would help speedup the review and testing.
Definition of Done Checklist: