-
Notifications
You must be signed in to change notification settings - Fork 73
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
Question: Can the User Pool also support the USER_PASSWORD_AUTH flow for classic signin/signup? #138
Comments
Thanks for that! The flow you wrote down should work, and should be supported by the library already. Well to be more exact, it is supported by the backend and front end library code ( We've been playing with the idea of adding username+password auth to the React component too, as another option besides magic link and paskey, but haven't done so yet. |
Hey @ottokruse. Thanks for the reply. We're actually trying to continue using as much of the Amplify UI React screens as possible, rather than rewriting them for integration with this library. Is that what you mean by "prefab React components"? Promisingly, the two libraries seem to working together well so far: the But I now have an issue with Here's the config: Passwordless.configureFromAmplify(
Amplify.configure({
Auth: {
region: "eu-west-1",
userPoolId: "eu-west-1_lBAOg7CvJ",
userPoolWebClientId: "51uiujj9qmhu82dmphp3ila3rn", // Client ID for both CUSTOM_AUTH and password flows
authenticationFlowType: "USER_PASSWORD_AUTH", // legacy but will definitely look at switching to USER_SRP_AUTH
},
})
).with({
fido2: {
baseUrl: "https://xpzygh93g8.execute-api.eu-west-1.amazonaws.com/v1/",
authenticatorSelection: {
authenticatorAttachment: "platform",
userVerification: "required",
},
rp: {
id: "localhost",
name: "localhost",
},
},
proxyApiHeaders: {
"Access-Control-Allow-Origin": "*",
},
debug: console.debug,
});
Auth.configure(); |
Yes indeed. Not sure though how you're going to be able to add a button "Sign in with passkey" to the prefab Amplify UI, but I haven't looked into it maybe it is possible––would love to know!
Nice to hear that this works :) We did intend it.
|
I'm currently just injecting the button into the const components = {
SignIn: {
Header() {
return (
<>
<p>Sign in with biometrics:</p>
<button onClick={() => authenticateWithFido2()} />
<p>or with password:</p>
</>
)
},
},
};
<Authenticator components={components}>...</Authenticator>
Thank you 🙏 I'll redeploy the stack and report back... |
Amazing! Once you have it all working I want to add an example like that as example here in this repo. Awesome stuff |
It works :) I can sign in with biometrics (or password), and add fido2 credentials after should the account need it. Amazing. I'll tidy up the code and post it in this chat. Thanks again - this is huge for us and our clients! |
Cool! 🎉 Looking forward to hearing/seeing more |
@harrygreen how has it been going? |
Not bad thanks @ottokruse. We're in the process of converting only the output we need - lambdas, api gateway, dynamo - into our Terraform setup. |
Great news |
Hey @ottokruse, quick question. If a user deletes their passkey from their device (an accepted scenario), dynamoDB doesn't know about it. So, I tried to let the user re-add their device - but I get the browser error:
What would you recommend doing in this scenario? Call |
This happens because the credential ID is the same as one that already exists in DynamoDB and is sent to the client as part of the registration ceremony in
The browser then throws that error if the user tries to register a credential with an ID that is in that list. Catch the error and offer the user the option of removing the existing credential from the Relying Party (DynamoDB)? "It looks like you've registered this device with us before. We must first remove it from our records, before you can register it again." My colleague suggests: if you have a mix of resident/non-resident keys, send (A resident key is a discoverable credential, that supports usernameless sign-in. These are often called passkeys but that term is also used more and more for non-discoverable credentials) So if you (we) do what my colleague suggests, we wouldn't have the error in the first place. Users can then always reregister their discoverable credentials and it will overwrite the record both on their device and on the server. That's a great idea but AFAIK it is not easy to detect what credential is a resident key (passkey) on RP side (yet), so I can't suggest a path there. |
Was just suggested to use the credProps extension to see on RP side if a credential is a resident key |
Thanks @ottokruse, that worked. I listen for We've gone live with the implementation 👍, with mixed results - we're seeing problems with tokens/sessions in the wild. Reading the Amplify JS docs https://docs.amplify.aws/javascript/prev/build-a-backend/auth/switch-auth/#customauth-flow states tl;dr |
Hey mate. About Check this search: https://github.com/search?q=%22Cannot+retrieve+a+new+session.+Please+authenticate.%22&type=code Looks like Amplify thinks there is no refresh token? maybe upgrading to Amplify V6 helps you.
Yes, you shouldn't have to do anything special for it, besides loading both (they should not conflict). But maybe you are already doing that, otherwise I don't understand why you'd see an Amplify error? Would be great to dig in why this is happening exactly |
Thanks. The
Yep, it's working. I've just got Amplify's const fido2options = await requestUsernamelessSignInChallenge();
const fido2credential = await fido2getCredential({
...fido2options,
relyingPartyId: "localhost",
});
if (!fido2credential.userHandleB64) {
throw new Error("No discoverable credentials available");
}
let username = new TextDecoder().decode(
bufferFromBase64Url(fido2credential.userHandleB64)
);
if (username.startsWith("s|")) {
throw new Error("Username is required for initiating sign-in");
}
username = username.replace(/^u\|/, "");
const user = await Auth.signIn(username, "");
if (user?.challengeName === "CUSTOM_CHALLENGE") {
try {
await Auth.sendCustomChallengeAnswer(
user,
JSON.stringify(fido2credential),
{
signInMethod: "FIDO2",
}
);
} catch (err) {
...
}
} I'm working on the create credentials part now. |
Was hoping you wouldn't have to do that but yes you can, and that's probably the most seamless way to integrate with Amplify. Although it remains guessing why you get the error you did.
Should just have 1 ID, 1 Access and 1 Refresh token though? (these are shared between this lib and Amplify) |
I'm trying to find the cause. |
@harrygreen Were you able to resolve your issue? Would love to hear how things turned out? We are having similar issues with Amplify in a native iOS app when the app wakes up in the background from a BLE service discovery (which is hard to test and debug). |
Hey @king7532, sorry for the delay. Yes, we have the solution in production for months now. AmplifyUI libs entirely handles the sessions/tokens as it does normally. This code handles everything else: the calls to the API for lambda triggers and DynamoDB, and invokes the passkey creation and verification. It would be amazing if Amplify and Cognito could provide this as an e2e solution. Having to write API gateway, DynamoDB, lambdas, is expensive and error-prone. Imagine if we just had a simple toggle in Cognito to expose endpoints to authenticate with passkeys. And AmplifyUI interface with that 👌 |
Thanks for that update, and hold tight for the re:Invent announcements, who knows ;)
Have any code to share on how you made this work? |
Holding tight for re:Invent.. awesome. So, the setup on page load is: import { Amplify } from "@aws-amplify/core";
import { Passwordless } from "../lib/fido2";
const amplifyConfigureResponse = Amplify.configure({
Auth: {
region: "[region]",
userPoolId: "[userPoolId]",
userPoolWebClientId: "[clientId]",
authenticationFlowType: "USER_SRP_AUTH",
},
});
Passwordless.configureFromAmplify(amplifyConfigureResponse).with({
fido2: {
baseUrl: "[api hostname]",
rp: {
id: location.hostname,
name: location.hostname,
},
},
}); Then then call the 3 relevant methods from https://github.com/aws-samples/amazon-cognito-passwordless-auth/blob/main/client/fido2.ts:
That's all we've needed so far - so no magic link, SMS, managing passkeys. After users login via the normal email/password flow, we then prompt them to add a passkey with Next time they load the app while logged out, we check the localStorage token for the passkey credential, and show the button to login with passkey in addition to email/password form. Hitting that button fires this: import { Auth } from "@aws-amplify/auth";
import {
fido2getCredential,
requestUsernamelessSignInChallenge,
} from "../../lib/fido2/fido2";
import { bufferFromBase64Url } from "../../lib/fido2/util";
async function attemptLoginWithBiometrics() {
const fido2options = await requestUsernamelessSignInChallenge();
const fido2credential = await fido2getCredential(fido2options);
if (!fido2credential.userHandleB64) {
throw new Error("No discoverable credentials available");
}
let username = new TextDecoder().decode(
bufferFromBase64Url(fido2credential.userHandleB64)
);
if (username.startsWith("s|")) {
throw new Error("Username is required for initiating sign-in");
}
username = username.replace(/^u\|/, "");
const user = await Auth.signIn(username, "");
if (user?.challengeName === "CUSTOM_CHALLENGE") {
await Auth.sendCustomChallengeAnswer(
user,
JSON.stringify(fido2credential),{ signInMethod: "FIDO2" }
);
await Auth.currentSession();
}
} As for the infra work, we converted/reverse-engineered the CDK into Terraform. That was a bit of a beast.. |
Nice!! Thanks for that! |
And thank you @ottokruse et al for the library! Without it we would've gone down the federated/SSO route. |
Hey. The library is fantastic. Perhaps this is more related to Cognito than specifically to this library.
We've successfully deployed the CDK setup and now want to allow a standard password auth flow for signin and signup. Password will be a fallback option for signin, and a default option for signup.
USER_PASSWORD_AUTH
in the user pool.fido2CreateCredential()
.Are there any problems with this? If not, I'm happy to help with a PR to expose these options.
The text was updated successfully, but these errors were encountered: