-
Notifications
You must be signed in to change notification settings - Fork 161
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
Custom IDP with Amplify and Auth at Edge #236
Comments
Great question! Very interesting to read what you're up to. So, matching I think it's easiest to also handle the redirect from Cognito with Amplify (not So make sure to use as redirectUri the unprotected piece of your site where you have Amplify running. Don't use Let Amplify automatically handle the redirect as it will, get JWTs and set them in cookies, that should work with Auth@Edge too from that moment on. That's the compatibility we intent: the cookies with the JWTs are recognized by Amplify as well as Auth@Edge, irregardless of whether they were set by Amplify or Auth@Edge. Haven't tried this though so it's unproven. Let me know what you run into. |
Well mate... Thanks for your quick reply. It worked (for the few test i just did). Well i still need to do lot more test to see how it behave with the rest of the stack (refresh token and all). On top of that i would want to reduce the number of cookie amplify is returning, will try to modify a bit the behavior on that sens. And need to try if i can redirect directly to my home instead of my login to avoid useless load + redirect after auth. Will get back to you on Monday with my differents findings. Thanks again ! you already made my weekend ❤️ |
nb : I was already checking how to inject the auth-at-edge state in the amplify call. And then in the Edge get it back and transform it back in the correct format -_- So i'm avoiding quite a lot with this |
Okay let's see! You're welcome and I hope this is a good path forward for you 👍🏻👍🏻 |
Hello @ottokruse ! The thing is that duplicating the already numberous cookies is problematic (too heavy request, waf rules etc.) My solution is to force the domain directly in the cookieSetting That works. But again weird. Did i missunderstood some configuration or those process aren't compatible with amplify (that force the dot for the cookie domain). Another thing i'm struggling with is the current signOut process. It seems it only cleans the ID Token. So in case you signOut with a user then reconnect with another one etc. You duplicate all the others amplify cookies because the cookie got the username in his name. So now trying to update the signOut process to clean everything. Would love to be able to rename amplify cookie but not possible i would say... |
Hi @AzaxSyndrom
Actually it is better to not use the leading dot, because that makes cookies readable by subdomains too. To get Amplify to work with that and prevent the double cookies that you see, use a single space Amplify.configure({
Auth: {
...,
cookieStorage: {
domain: " ", // Use a single space " " for host-only cookies
...
},
},
}); Read the comment here for more details: https://github.com/aws-samples/cloudfront-authorization-at-edge/blob/996a07f6bee12b4b62503ac763d5465ee0a06a1c/src/cfn-custom-resources/react-app/index.ts#L64C23-L64C23 |
About the signOut only clearing ID token, it should clear all cookies actually so we need to dig in. The reason this probably happens is because you cannot explicitly delete cookies, only overwrite them with an empty value and expiry set (then browser understands it must discard it). However, overwrite like that only works if you use the exact same cookie settings as you did when you initially set the cookies. So e.g. exact same path, domain, etc. Have a look if that is the case for you? Note that this PR did fix an issue like this: #207 So make sure you are on a version of Auth@Edge that includes that fix (v2.1.2 onwards) |
Hi |
Nice! 🎉 |
Hello there
after searching for hours the different issues (closed & open) on both Amplify project and here i couldn't find a similar solution.
Question
Did someone actually manage to make auth-at-Edge stack work with amplify integration and an custom IDP through Cognito ?
Quick problem overview
Amplify modify the state during a custom IDP signIn hence blocking the parse-auth process.
Context
We are using this stack for our applications with the default Cognito UI and it work great (both Cognito user-pool & Azure IDP connections).
We would want to use a custom Login page with amplify to replace the hosted UI (same as shared here issue49)
We created a spa with amplify & the auth module accessible on /login and modified the check-auth to redirect on our custom login page. Setup the stack for cookie compatibility amplify.
The solution works with amplify when login through Cognito user-pool. Even though the parse-auth function is completely skip during the authentification process from amplify (Auth.signIn) (maybe i'm already doing something wrong here).
But when using the Auth.federatedSignIn({ customProvider: 'my-providerName-setup-inCognito'}); the state generated from the check-auth is not send to oauth2/authorize request. Amplify generate it's own state.
It does have the option to pass a customState attribute to the federatedSignIn methods. But even with that, it still modify it.
So when the authentification flow from IDP finish and that we end up on the parse-auth, the lambda refuse the connexion because the state cannot be parse as intended here
parsedState = JSON.parse( Buffer.from(common.urlSafe.parse(state), "base64").toString() );
Did someone managed a similar solution ?
Possible solutions
1 - Not using amplify and do all the IDP flow manually in the custom login page (still weird for a compliant amplify stack)
2 - Override amplify class to get and set the state correctly -> cumbersome & not easy to maintain later on
3 - Pass the state in a header during check-auth et set it again in parse-auth -> Isn't that posing a security risk ? (like losing the whole interest of the state)
It's a solution i take by heart and would really wanna be able to achieve !
Thank you for the reading ❤️
The text was updated successfully, but these errors were encountered: