SDKS-5026: Device Authorization Grant Feature Testing#202
Open
vibhorgoswami wants to merge 2 commits into
Open
SDKS-5026: Device Authorization Grant Feature Testing#202vibhorgoswami wants to merge 2 commits into
vibhorgoswami wants to merge 2 commits into
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
vibhorgoswami
temporarily deployed
to
github-pages
GitHub Actions
Inactive
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## SDKS-4784 #202 +/- ##
===============================================
- Coverage 42.16% 42.15% -0.02%
Complexity 1287 1287
===============================================
Files 315 315
Lines 9605 9605
Branches 1447 1447
===============================================
- Hits 4050 4049 -1
Misses 4999 4999
- Partials 556 557 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
vibhorgoswami
temporarily deployed
to
github-pages
GitHub Actions
Inactive
vibhorgoswami
force-pushed
the
SDKS-5026
branch
2 times, most recently
from
d6c9a7b to
bb07373
Compare
vibhorgoswami
temporarily deployed
to
github-pages
GitHub Actions
Inactive
vibhorgoswami
temporarily deployed
to
github-pages
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/config/EnvViewModel.kt (1)
346-364:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInitialize Device Auth client during ViewModel bootstrap
daAppliedis loaded and assigned (Line 360), but no SDK instance is built in the same init path. This can leaveoidcDeviceClientout of sync with applied config until a manual selection happens.Suggested fix
withContext(Dispatchers.Main) { customJourneyConfigs = jCustom customDaVinciConfigs = dvCustom customWebConfigs = wCustom customDeviceAuthConfigs = daCustom appliedJourneyConfig = jApplied appliedDaVinciConfig = dvApplied appliedWebConfig = wApplied appliedDeviceAuthConfig = daApplied buildJourneyInstance(jApplied) buildDaVinciInstance(dvApplied) buildWebInstance(wApplied) + buildDeviceAuthClient(daApplied) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/config/EnvViewModel.kt` around lines 346 - 364, The device auth applied config is loaded into appliedDeviceAuthConfig but the SDK client isn't initialized during ViewModel bootstrap; call the method that builds the Device Auth SDK instance (e.g., buildDeviceAuthInstance) with daApplied inside the same withContext block after setting appliedDeviceAuthConfig so oidcDeviceClient is initialized and stays in sync with the applied config.
🧹 Nitpick comments (1)
samples/pingsampleapp/src/main/res/values/strings.xml (1)
25-26: 💤 Low valueConsider using consistent terminology in title and subtitle.
The title uses the full term "Device Authorization Grant" while the subtitle abbreviates to "Device Auth". For improved clarity and consistency, consider using the full term in the subtitle as well.
📝 Suggested refinement
- <string name="text_device_authorization_grant_subtitle">Start or verify Device Auth</string> + <string name="text_device_authorization_grant_subtitle">Start or verify Device Authorization</string>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@samples/pingsampleapp/src/main/res/values/strings.xml` around lines 25 - 26, Update the subtitle string to use the full term to match the title: change the value of the resource named text_device_authorization_grant_subtitle so it reads "Start or verify Device Authorization Grant" (matching the wording of text_device_authorization_grant_title) to keep terminology consistent across the UI.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authgrant/ApproveDeviceScreen.kt`:
- Around line 207-211: The onClick handler for the Approve with Browser Button
calls context.startActivity(Intent(Intent.ACTION_VIEW,
initialVerificationUri.toUri())) without guarding for ActivityNotFoundException;
wrap the launch in a safe check (use
context.packageManager?.resolveActivity(intent, 0) to ensure an app can handle
the intent or run startActivity inside a try/catch for
ActivityNotFoundException) and only call
onApproveWithBrowser(initialVerificationUri) after a successful launch; on
failure, log or show a user-facing message instead of dismissing UI so the flow
doesn't crash.
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authgrant/DeviceAuthorizationGrantViewModel.kt`:
- Around line 98-127: In the DeviceFlowStatus.Expired,
DeviceFlowStatus.AccessDenied and is DeviceFlowStatus.Failure branches update
the _uiState to also clear any stale device-auth artifacts: set userCode = ""
(or null if your UI state uses nullable), verificationUri = "" (or null), and
isSuccess = false along with the existing
hasStarted/statusMessage/errorMessage/isLoading changes so previous codes/URIs
and success flags are not retained after terminal error states; modify the calls
to _uiState.update in those branches (the ones shown handling Expired,
AccessDenied and Failure) to include these fields.
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/config/EnvViewModel.kt`:
- Around line 470-472: The code compares appliedDeviceAuthConfig.display to
deleted.display and then injects a hard-coded DeviceAuthConfigState; instead,
compare a stable identifier (e.g., appliedDeviceAuthConfig.id or
appliedDeviceAuthConfig.issuer) to deleted.id/issuer and reuse an existing
preset from your presets list before falling back; update the branch around
appliedDeviceAuthConfig, deleted and selectDeviceAuthConfig to search your
presets for a matching config (by id or issuer) and call
selectDeviceAuthConfig(matchingPreset) if found, otherwise clear the selection
or select a safe default from presets rather than instantiating a new hard-coded
DeviceAuthConfigState.
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/logout/Logout.kt`:
- Around line 159-173: The aggregate session predicates that drive the “Logout
All” enabled state and the “No Active Sessions” message do not include the
device-auth session (state.oidcDeviceClient); update the aggregate checks (the
functions/expressions that compute whether any session is active and whether
“logout all” should be enabled—e.g., the code in the composable/logic that reads
state.oidcUserSession, state.oidcBackchannel, state.samlSession, etc.) to also
consider state.oidcDeviceClient, and ensure the same updated predicate is used
where the “No Active Sessions” label and the Logout All button enabled/disabled
logic are computed (update the related ViewModel/composable methods such as
listLogoutOptions(), the logout-all handler predicate, and the UI
visibility/enablement checks to include oidcDeviceClient).
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/logout/LogoutViewModel.kt`:
- Around line 62-67: Wrap the logout calls in try/catch/finally so failures
don't prevent onCompleted() or subsequent logouts: in logoutOidcDeviceClient,
call oidcDeviceClient?.user()?.logout() inside a try block, log or handle the
exception in catch, and call onCompleted() in finally; do the same pattern for
the other logout helpers referenced in this file (e.g., the logoutAll sequence
and any methods around lines 69-77) so each logout is resilient and errors don't
stop later calls.
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/token/Token.kt`:
- Around line 115-122: The Tab composable currently hardcodes the label "Auth
Grant" (seen where Tab is created with selected = tokenState.selectedTab ==
TokenType.AUTH_GRANT and onClick calling
tokenViewModel.selectTab(TokenType.AUTH_GRANT) /
tokenViewModel.loadAllTokens()); extract this and other user-facing strings in
the Token composable (also around lines 153-206) into resources by adding
entries like auth_grant, access_token, clear, refresh, revoke to strings.xml and
replace hardcoded Text("Auth Grant") (and other hardcoded Text(...) usages) with
Text(stringResource(R.string.<name>)) to enable i18n consistency.
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/userprofile/UserProfileViewModel.kt`:
- Around line 180-196: In authGrantUserInfo, when oidcDeviceClient?.user() is
null the function currently returns without clearing previous values; update the
logic in authGrantUserInfo to call state.update and set authGrantUser = null and
authGrantError = null (or appropriate cleared values) when
oidcDeviceClient?.user() returns null so stale profile data is removed; locate
this in the authGrantUserInfo function and ensure both the null-branch and
existing Result.Failure/Result.Success branches consistently update state via
state.update.
---
Outside diff comments:
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/config/EnvViewModel.kt`:
- Around line 346-364: The device auth applied config is loaded into
appliedDeviceAuthConfig but the SDK client isn't initialized during ViewModel
bootstrap; call the method that builds the Device Auth SDK instance (e.g.,
buildDeviceAuthInstance) with daApplied inside the same withContext block after
setting appliedDeviceAuthConfig so oidcDeviceClient is initialized and stays in
sync with the applied config.
---
Nitpick comments:
In `@samples/pingsampleapp/src/main/res/values/strings.xml`:
- Around line 25-26: Update the subtitle string to use the full term to match
the title: change the value of the resource named
text_device_authorization_grant_subtitle so it reads "Start or verify Device
Authorization Grant" (matching the wording of
text_device_authorization_grant_title) to keep terminology consistent across the
UI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 12e2be9e-a913-490c-b230-974b82dd3767
📒 Files selected for processing (19)
gradle/libs.versions.tomlsamples/pingsampleapp/build.gradle.ktssamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authgrant/ApproveDeviceScreen.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authgrant/DeviceAuthorizationGrantScreen.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/authgrant/DeviceAuthorizationGrantViewModel.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/config/Env.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/config/EnvViewModel.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/davinci/DaVinci.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/davinci/DaVinciViewModel.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/home/HomeApp.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/logout/Logout.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/logout/LogoutViewModel.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/navigation/Navigation.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/token/Token.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/token/TokenState.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/token/TokenViewModel.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/userprofile/UserProfile.ktsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/userprofile/UserProfileViewModel.ktsamples/pingsampleapp/src/main/res/values/strings.xml
Comment on lines
+115
to
+122
| Tab( | ||
| selected = tokenState.selectedTab == TokenType.AUTH_GRANT, | ||
| onClick = { | ||
| tokenViewModel.selectTab(TokenType.AUTH_GRANT) | ||
| tokenViewModel.loadAllTokens() | ||
| }, | ||
| text = { Text("Auth Grant") } | ||
| ) |
Contributor
There was a problem hiding this comment.
Externalize new/edited UI labels to resources.
New/edited user-facing labels are hardcoded in the composable. Please move them to strings.xml and use stringResource(...) for i18n consistency.
Proposed fix
+import androidx.compose.ui.res.stringResource
@@
- text = { Text("Auth Grant") }
+ text = { Text(stringResource(R.string.auth_grant)) }
@@
- text = { Text("Refresh") },
+ text = { Text(stringResource(R.string.refresh)) },
@@
- text = { Text("Revoke") },
+ text = { Text(stringResource(R.string.revoke)) },
@@
- Text(text = "AccessToken")
+ Text(text = stringResource(R.string.access_token))
@@
- Text(text = "Clear")
+ Text(text = stringResource(R.string.clear))
@@
- Text(text = "Refresh")
+ Text(text = stringResource(R.string.refresh))
@@
- Text(text = "Revoke")
+ Text(text = stringResource(R.string.revoke))<!-- samples/pingsampleapp/src/main/res/values/strings.xml -->
<string name="auth_grant">Auth Grant</string>
<string name="access_token">Access token</string>
<string name="clear">Clear</string>
<string name="refresh">Refresh</string>
<string name="revoke">Revoke</string>Also applies to: 153-206
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/token/Token.kt`
around lines 115 - 122, The Tab composable currently hardcodes the label "Auth
Grant" (seen where Tab is created with selected = tokenState.selectedTab ==
TokenType.AUTH_GRANT and onClick calling
tokenViewModel.selectTab(TokenType.AUTH_GRANT) /
tokenViewModel.loadAllTokens()); extract this and other user-facing strings in
the Token composable (also around lines 153-206) into resources by adding
entries like auth_grant, access_token, clear, refresh, revoke to strings.xml and
replace hardcoded Text("Auth Grant") (and other hardcoded Text(...) usages) with
Text(stringResource(R.string.<name>)) to enable i18n consistency.
Comment on lines
+180
to
+196
| fun authGrantUserInfo() { | ||
| viewModelScope.launch { | ||
| oidcDeviceClient?.user()?.let { user -> | ||
| when (val result = user.userinfo(false)) { | ||
| is Result.Failure -> | ||
| state.update { s -> | ||
| s.copy(authGrantUser = null, authGrantError = result.value) | ||
| } | ||
|
|
||
| is Result.Success -> { | ||
| state.update { s -> | ||
| s.copy(authGrantUser = result.value, authGrantError = null) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Clear auth-grant state when there is no active device-client user.
When oidcDeviceClient?.user() is null, the function exits without resetting authGrantUser/authGrantError, so stale profile data can remain visible.
🛠️ Suggested fix
fun authGrantUserInfo() {
viewModelScope.launch {
- oidcDeviceClient?.user()?.let { user ->
- when (val result = user.userinfo(false)) {
- is Result.Failure ->
- state.update { s ->
- s.copy(authGrantUser = null, authGrantError = result.value)
- }
-
- is Result.Success -> {
- state.update { s ->
- s.copy(authGrantUser = result.value, authGrantError = null)
- }
- }
- }
- }
+ val user = oidcDeviceClient?.user()
+ if (user == null) {
+ state.update { s ->
+ s.copy(
+ authGrantUser = null,
+ authGrantError = null,
+ showRawAuthGrantUserInfo = false,
+ )
+ }
+ return@launch
+ }
+ when (val result = user.userinfo(false)) {
+ is Result.Failure ->
+ state.update { s ->
+ s.copy(authGrantUser = null, authGrantError = result.value)
+ }
+ is Result.Success ->
+ state.update { s ->
+ s.copy(authGrantUser = result.value, authGrantError = null)
+ }
+ }
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/userprofile/UserProfileViewModel.kt`
around lines 180 - 196, In authGrantUserInfo, when oidcDeviceClient?.user() is
null the function currently returns without clearing previous values; update the
logic in authGrantUserInfo to call state.update and set authGrantUser = null and
authGrantError = null (or appropriate cleared values) when
oidcDeviceClient?.user() returns null so stale profile data is removed; locate
this in the authGrantUserInfo function and ensure both the null-branch and
existing Result.Failure/Result.Success branches consistently update state via
state.update.
vibhorgoswami
temporarily deployed
to
github-pages
GitHub Actions
Inactive
witrisna
force-pushed
the
SDKS-4784
branch
2 times, most recently
from
14b1925 to
f8980b3
Compare
vibhorgoswami
temporarily deployed
to
github-pages
GitHub Actions
Inactive
vibhorgoswami
temporarily deployed
to
github-pages
GitHub Actions
Inactive
witrisna
requested changes
May 26, 2026
…upport - Implement `OidcDeviceClient` to manage the device authorization flow, including requesting device codes and polling the token endpoint. - Add `DeviceFlowStatus` sealed class to represent the states of the device flow: `Started`, `Polling`, `Success`, `Expired`, `AccessDenied`, and `Failure`. - Update `Oidc` module to detect the presence of `VERIFICATION_URI_COMPLETE` in the shared context to trigger device-code completion instead of the standard browser-redirect flow. - Modify `Journey` and `DaVinci` success logic to skip standard OIDC token exchange and instead perform device code verification when a `user_code` is present. - Implement `populateDeviceFlowVerificationRequest` to construct verification URLs supporting both standard and PingOne-specific tenant path formats. - Update `OpenIdConfiguration` to include `device_authorization_endpoint` and allow manual configuration via a new `openId` DSL block in `OidcClientConfig`. - Support passing custom parameters in `Journey.start` via an updated `Option` class. - Add comprehensive unit tests for device flow polling logic, error handling (e.g., `slow_down`, `expired_token`), and integration within DaVinci and Journey workflows.
vibhorgoswami
temporarily deployed
to
github-pages
GitHub Actions
Inactive
vibhorgoswami
temporarily deployed
to
github-pages
GitHub Actions
Inactive
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
2 participants
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.
JIRA Ticket
SDKS-5026
Description
Introduces the test feature for Device Authorization Grant. Shows the option for QR code/User Code/Verification URL to authenticate the device.
Summary by CodeRabbit