Skip to content
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

Add Restore Credential Support for Shrine app #104

Open
wants to merge 1 commit into
base: shrine-compose
Choose a base branch
from

Conversation

neelanshsahai
Copy link

Change-Id: I476116b1f3400c452783d2f172bd7adbca5fcadb

Change-Id: I476116b1f3400c452783d2f172bd7adbca5fcadb
Copy link

@danjkim danjkim left a comment

Choose a reason for hiding this comment

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

can the backup integration also be included to the test app?

@neelanshsahai neelanshsahai requested a review from cy245 January 9, 2025 14:50
@neelanshsahai neelanshsahai marked this pull request as ready for review February 10, 2025 10:32
restoreCredentialRequest,
)
} catch (e: Exception) {
e.printStackTrace()
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this?

@@ -17,16 +17,19 @@ package com.example.android.authentication.shrine

import android.annotation.SuppressLint
import android.content.Context
import androidx.credentials.ClearCredentialStateRequest
Copy link
Contributor

Choose a reason for hiding this comment

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

@neelanshsahai can we update the README to tell about RestoreCredentials functionality? @cy245 FYI

context: Context,
) {
val passkeysEligibility = PasskeysEligibility.isPasskeySupported(context)
if (passkeysEligibility.isEligible) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if not eligible, are we throwing any error?


suspend fun getRestoreKey(
authenticationJson: JSONObject,
activity: Context,
Copy link
Contributor

Choose a reason for hiding this comment

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

keep either activ ity or context which is more reasonable?

creationResult = jsonObject,
)
}
getPasskey = { jsonObject ->
Copy link
Contributor

Choose a reason for hiding this comment

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

is it getPasskeyRequest or reponse or just a result, a bit confusing? Can we name accordingly?

context = activityContext,
)
}
createPassword = { username: String, password: String ->
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants